[OpenMP][IRBuilder] Handle target ... nowait when codegen targets host
#124720
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes llvm#124578 Handles the `nowait` clause for `omp.target` ops when the actual target is the host (i.e. there is no target device). Rather than only checking for the `HasNoWait` boolean, we also check for the presence/absence of a `DeviceID` value. We only emit the target task if both are present.
llvmbot
added
mlir:llvm
mlir
flang:openmp
clang:openmp
|
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-mlir Author: Kareem Ergawy (ergawy) ChangesFixes #124578 Handles the Full diff: https://github.com/llvm/llvm-project/pull/124720.diff 2 Files Affected:
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 8cc3a99d92023d..0692101d56eb76 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -7258,10 +7258,12 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
// If `HasNoWait == true`, we call @__kmpc_omp_target_task_alloc to provide
// the DeviceID to the deferred task and also since
// @__kmpc_omp_target_task_alloc creates an untied/async task.
+ bool NeedsTargetTask = HasNoWait && DeviceID;
Function *TaskAllocFn =
- !HasNoWait ? getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_task_alloc)
- : getOrCreateRuntimeFunctionPtr(
- OMPRTL___kmpc_omp_target_task_alloc);
+ !NeedsTargetTask
+ ? getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_task_alloc)
+ : getOrCreateRuntimeFunctionPtr(
+ OMPRTL___kmpc_omp_target_task_alloc);
// Arguments - `loc_ref` (Ident) and `gtid` (ThreadID)
// call.
@@ -7310,8 +7312,10 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
/*sizeof_task=*/TaskSize, /*sizeof_shared=*/SharedsSize,
/*task_func=*/ProxyFn};
- if (HasNoWait)
+ if (NeedsTargetTask) {
+ assert(DeviceID && "Expected non-empty device ID.");
TaskAllocArgs.push_back(DeviceID);
+ }
TaskData = Builder.CreateCall(TaskAllocFn, TaskAllocArgs);
@@ -7333,7 +7337,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
// ---------------------------------------------------------------
// The above means that the lack of a nowait on the target construct
// translates to '#pragma omp task if(0)'
- if (!HasNoWait) {
+ if (!NeedsTargetTask) {
if (DepArray) {
Function *TaskWaitFn =
getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_wait_deps);
diff --git a/mlir/test/Target/LLVMIR/omptarget-nowait-host-only.mlir b/mlir/test/Target/LLVMIR/omptarget-nowait-host-only.mlir
new file mode 100644
index 00000000000000..6b634226a35686
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-nowait-host-only.mlir
@@ -0,0 +1,29 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Tests `target ... nowait` when code gen targets the host rather than a
+// device.
+
+module attributes {omp.is_target_device = false} {
+ llvm.func @omp_target_nowait_() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x f32 {bindc_name = "x"} : (i64) -> !llvm.ptr
+ %3 = omp.map.info var_ptr(%1 : !llvm.ptr, f32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = "x"}
+ omp.target nowait map_entries(%3 -> %arg0 : !llvm.ptr) {
+ %4 = llvm.mlir.constant(5.000000e+00 : f32) : f32
+ llvm.store %4, %arg0 : f32, !llvm.ptr
+ omp.terminator
+ }
+ llvm.return
+ }
+}
+
+// CHECK: define void @omp_target_nowait_()
+// CHECK-NOT: define {{.*}} @
+// CHECK-NOT: call ptr @__kmpc_omp_target_task_alloc({{.*}})
+// Verify that we directly emit a call to the "target" region's body from the
+// parent function of the the `omp.target` op.
+// CHECK: call void @__omp_offloading_[[DEV:.*]]_[[FIL:.*]]_omp_target_nowait__l[[LINE:.*]](ptr {{.*}})
+// CHECK-NEXT: ret void
+
+// CHECK: define internal void @__omp_offloading_[[DEV]]_[[FIL]]_omp_target_nowait__l[[LINE]](ptr %[[ADDR_X:.*]])
+// CHECK: store float 5{{.*}}, ptr %[[ADDR_X]], align 4
|
|
@llvm/pr-subscribers-flang-openmp Author: Kareem Ergawy (ergawy) ChangesFixes #124578 Handles the Full diff: https://github.com/llvm/llvm-project/pull/124720.diff 2 Files Affected:
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 8cc3a99d92023d..0692101d56eb76 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -7258,10 +7258,12 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
// If `HasNoWait == true`, we call @__kmpc_omp_target_task_alloc to provide
// the DeviceID to the deferred task and also since
// @__kmpc_omp_target_task_alloc creates an untied/async task.
+ bool NeedsTargetTask = HasNoWait && DeviceID;
Function *TaskAllocFn =
- !HasNoWait ? getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_task_alloc)
- : getOrCreateRuntimeFunctionPtr(
- OMPRTL___kmpc_omp_target_task_alloc);
+ !NeedsTargetTask
+ ? getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_task_alloc)
+ : getOrCreateRuntimeFunctionPtr(
+ OMPRTL___kmpc_omp_target_task_alloc);
// Arguments - `loc_ref` (Ident) and `gtid` (ThreadID)
// call.
@@ -7310,8 +7312,10 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
/*sizeof_task=*/TaskSize, /*sizeof_shared=*/SharedsSize,
/*task_func=*/ProxyFn};
- if (HasNoWait)
+ if (NeedsTargetTask) {
+ assert(DeviceID && "Expected non-empty device ID.");
TaskAllocArgs.push_back(DeviceID);
+ }
TaskData = Builder.CreateCall(TaskAllocFn, TaskAllocArgs);
@@ -7333,7 +7337,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
// ---------------------------------------------------------------
// The above means that the lack of a nowait on the target construct
// translates to '#pragma omp task if(0)'
- if (!HasNoWait) {
+ if (!NeedsTargetTask) {
if (DepArray) {
Function *TaskWaitFn =
getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_omp_wait_deps);
diff --git a/mlir/test/Target/LLVMIR/omptarget-nowait-host-only.mlir b/mlir/test/Target/LLVMIR/omptarget-nowait-host-only.mlir
new file mode 100644
index 00000000000000..6b634226a35686
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-nowait-host-only.mlir
@@ -0,0 +1,29 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Tests `target ... nowait` when code gen targets the host rather than a
+// device.
+
+module attributes {omp.is_target_device = false} {
+ llvm.func @omp_target_nowait_() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x f32 {bindc_name = "x"} : (i64) -> !llvm.ptr
+ %3 = omp.map.info var_ptr(%1 : !llvm.ptr, f32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = "x"}
+ omp.target nowait map_entries(%3 -> %arg0 : !llvm.ptr) {
+ %4 = llvm.mlir.constant(5.000000e+00 : f32) : f32
+ llvm.store %4, %arg0 : f32, !llvm.ptr
+ omp.terminator
+ }
+ llvm.return
+ }
+}
+
+// CHECK: define void @omp_target_nowait_()
+// CHECK-NOT: define {{.*}} @
+// CHECK-NOT: call ptr @__kmpc_omp_target_task_alloc({{.*}})
+// Verify that we directly emit a call to the "target" region's body from the
+// parent function of the the `omp.target` op.
+// CHECK: call void @__omp_offloading_[[DEV:.*]]_[[FIL:.*]]_omp_target_nowait__l[[LINE:.*]](ptr {{.*}})
+// CHECK-NEXT: ret void
+
+// CHECK: define internal void @__omp_offloading_[[DEV]]_[[FIL]]_omp_target_nowait__l[[LINE]](ptr %[[ADDR_X:.*]])
+// CHECK: store float 5{{.*}}, ptr %[[ADDR_X]], align 4
|
abidh
approved these changes
Jan 28, 2025
|
Thanks for the quick reviews guys! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/9504 Here is the relevant piece of the build log for the reference |
|
The buildbot failure seems completely unrelated. Please let me know if I missed something. |
Yes, there is an issue open for it: #124485. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12574 Here is the relevant piece of the build log for the reference |
Looking into this one ... |
|
Ran |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #124578
Handles the
nowaitclause foromp.targetops when the actual target is the host (i.e. there is no target device). Rather than only checking for theHasNoWaitboolean, we also check for the presence/absence of aDeviceIDvalue. We only emit the target task if both are present.