[flang][OpenMP] Map device pointers on host device as well #145562
Conversation
Given a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if `x` was used in the body. The cause of the crash is that the MLIR->LLVM codegen tries to look up the translated value of x, but one had not been mapped. Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created. Fixes llvm#145558
llvmbot
added
mlir:llvm
mlir
flang
|
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-flang-fir-hlfir Author: Krzysztof Parzyszek (kparzysz) ChangesGiven a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created. Fixes #145558 Full diff: https://github.com/llvm/llvm-project/pull/145562.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/target-data-if-false.f90 b/flang/test/Lower/OpenMP/target-data-if-false.f90
new file mode 100644
index 0000000000000..ac0f675ddc806
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-data-if-false.f90
@@ -0,0 +1,25 @@
+!RUN: %flang_fc1 -emit-llvm -fopenmp %openmp_flags -fopenmp-version=52 %s -o - | FileCheck %s
+
+!Check that this doesn't crash.
+
+!CHECK-LABEL: define void @f00_()
+!CHECK: call i1 @_FortranAioOutputDerivedType
+
+subroutine f00
+ use iso_c_binding
+ type(c_ptr) :: x
+
+!$omp target data use_device_ptr(x) if(.false.)
+ print *, x
+!$omp end target data
+end
+
+!CHECK-LABEL: define void @f01_()
+!CHECK: call i1 @_FortranAioOutputInteger32
+subroutine f01
+ integer :: x
+
+!$omp target data use_device_addr(x) if(.false.)
+ print *, x
+!$omp end target data
+end
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 90ce06a0345c0..d7986dbf451fc 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4549,6 +4549,18 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
}
break;
case BodyGenTy::DupNoPriv:
+ if (info.DevicePtrInfoMap.empty()) {
+ // For host device we still need to do the mapping for codegen,
+ // otherwise it may try to lookup a missing value.
+ if (!ompBuilder->Config.IsTargetDevice.value_or(false)) {
+ mapUseDevice(llvm::OpenMPIRBuilder::DeviceInfoTy::Address,
+ blockArgIface.getUseDeviceAddrBlockArgs(),
+ useDeviceAddrVars, mapData);
+ mapUseDevice(llvm::OpenMPIRBuilder::DeviceInfoTy::Pointer,
+ blockArgIface.getUseDevicePtrBlockArgs(),
+ useDevicePtrVars, mapData);
+ }
+ }
// We must always restoreIP regardless of doing anything the caller
// does not restore it, leading to incorrect (no) branch generation.
builder.restoreIP(codeGenIP);
|
|
@llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesGiven a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created. Fixes #145558 Full diff: https://github.com/llvm/llvm-project/pull/145562.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/target-data-if-false.f90 b/flang/test/Lower/OpenMP/target-data-if-false.f90
new file mode 100644
index 0000000000000..ac0f675ddc806
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-data-if-false.f90
@@ -0,0 +1,25 @@
+!RUN: %flang_fc1 -emit-llvm -fopenmp %openmp_flags -fopenmp-version=52 %s -o - | FileCheck %s
+
+!Check that this doesn't crash.
+
+!CHECK-LABEL: define void @f00_()
+!CHECK: call i1 @_FortranAioOutputDerivedType
+
+subroutine f00
+ use iso_c_binding
+ type(c_ptr) :: x
+
+!$omp target data use_device_ptr(x) if(.false.)
+ print *, x
+!$omp end target data
+end
+
+!CHECK-LABEL: define void @f01_()
+!CHECK: call i1 @_FortranAioOutputInteger32
+subroutine f01
+ integer :: x
+
+!$omp target data use_device_addr(x) if(.false.)
+ print *, x
+!$omp end target data
+end
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 90ce06a0345c0..d7986dbf451fc 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4549,6 +4549,18 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
}
break;
case BodyGenTy::DupNoPriv:
+ if (info.DevicePtrInfoMap.empty()) {
+ // For host device we still need to do the mapping for codegen,
+ // otherwise it may try to lookup a missing value.
+ if (!ompBuilder->Config.IsTargetDevice.value_or(false)) {
+ mapUseDevice(llvm::OpenMPIRBuilder::DeviceInfoTy::Address,
+ blockArgIface.getUseDeviceAddrBlockArgs(),
+ useDeviceAddrVars, mapData);
+ mapUseDevice(llvm::OpenMPIRBuilder::DeviceInfoTy::Pointer,
+ blockArgIface.getUseDevicePtrBlockArgs(),
+ useDevicePtrVars, mapData);
+ }
+ }
// We must always restoreIP regardless of doing anything the caller
// does not restore it, leading to incorrect (no) branch generation.
builder.restoreIP(codeGenIP);
|
|
@llvm/pr-subscribers-mlir Author: Krzysztof Parzyszek (kparzysz) ChangesGiven a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created. Fixes #145558 Full diff: https://github.com/llvm/llvm-project/pull/145562.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/target-data-if-false.f90 b/flang/test/Lower/OpenMP/target-data-if-false.f90
new file mode 100644
index 0000000000000..ac0f675ddc806
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-data-if-false.f90
@@ -0,0 +1,25 @@
+!RUN: %flang_fc1 -emit-llvm -fopenmp %openmp_flags -fopenmp-version=52 %s -o - | FileCheck %s
+
+!Check that this doesn't crash.
+
+!CHECK-LABEL: define void @f00_()
+!CHECK: call i1 @_FortranAioOutputDerivedType
+
+subroutine f00
+ use iso_c_binding
+ type(c_ptr) :: x
+
+!$omp target data use_device_ptr(x) if(.false.)
+ print *, x
+!$omp end target data
+end
+
+!CHECK-LABEL: define void @f01_()
+!CHECK: call i1 @_FortranAioOutputInteger32
+subroutine f01
+ integer :: x
+
+!$omp target data use_device_addr(x) if(.false.)
+ print *, x
+!$omp end target data
+end
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 90ce06a0345c0..d7986dbf451fc 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4549,6 +4549,18 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
}
break;
case BodyGenTy::DupNoPriv:
+ if (info.DevicePtrInfoMap.empty()) {
+ // For host device we still need to do the mapping for codegen,
+ // otherwise it may try to lookup a missing value.
+ if (!ompBuilder->Config.IsTargetDevice.value_or(false)) {
+ mapUseDevice(llvm::OpenMPIRBuilder::DeviceInfoTy::Address,
+ blockArgIface.getUseDeviceAddrBlockArgs(),
+ useDeviceAddrVars, mapData);
+ mapUseDevice(llvm::OpenMPIRBuilder::DeviceInfoTy::Pointer,
+ blockArgIface.getUseDevicePtrBlockArgs(),
+ useDevicePtrVars, mapData);
+ }
+ }
// We must always restoreIP regardless of doing anything the caller
// does not restore it, leading to incorrect (no) branch generation.
builder.restoreIP(codeGenIP);
|
| blockArgIface.getUseDevicePtrBlockArgs(), | ||
| useDevicePtrVars, mapData); | ||
| } | ||
| } | ||
| // We must always restoreIP regardless of doing anything the caller | ||
| // does not restore it, leading to incorrect (no) branch generation. | ||
| builder.restoreIP(codeGenIP); |
There was a problem hiding this comment.
Nit: Sorry unrelated to your change, minor code cleanup.
This restoreIP call seems redundant as we have one at the entry of bodyGenCB, can we please remove it and move the comment to the restoreIP call above.
Thanks.
There was a problem hiding this comment.
The one at the beginning of bodyGenCB?
There was a problem hiding this comment.
Thanks, the new revision is what I wanted :)
Given a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if `x` was used in the body. The cause of the crash is that the MLIR->LLVM codegen tries to look up the translated value of x, but one had not been mapped. Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created. Fixes llvm#145558
Given a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if `x` was used in the body. The cause of the crash is that the MLIR->LLVM codegen tries to look up the translated value of x, but one had not been mapped. Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created. Fixes llvm#145558
Given a TARGET DATA construct with USE_DEVICE_PTR(x) and IF(FALSE), the compiler will crash if
xwas used in the body. The cause of the crash is that the MLIR->LLVM codegen tries to look up the translated value of x, but one had not been mapped.Given an IF clause, the translation will generate an if-then-else construct, with the "else" block corresponding to the false condition, i.e. the host device playing the role of the target device. In that block, still process the USE_DEVICE_ADDR/USE_DEVICE_PTR clauses, which will cause the translation mappings to be created.
Fixes #145558