[Mlir-commits] [mlir] [Flang][OpenMP] Declare Target fixes for USM and declare target to (PR #200248)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-openmp

Author: agozillon

<details>
<summary>Changes</summary>

Currently declare target enter would fall through the if statement even though it's a synonym of declare target to, so fix this via adding a simpler and more readable check for link clause instead.

USM declare target to works a little differently to regular link cases, where the type is actually converted into a pointer, rather than a global of the original typing. So, add a caveat where we convert the type to a pointer if requires usm has been triggered. This gets the correct behaviour in USM mode on USM devices.

---
Full diff: https://github.com/llvm/llvm-project/pull/200248.diff


2 Files Affected:

- (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+10-4) 
- (added) mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-device-2.mlir (+50) 


``````````diff
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index f0511bb4be7dd..8435855bfca13 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -8171,15 +8171,21 @@ convertDeclareTargetAttr(Operation *op, mlir::omp::DeclareTargetAttr attribute,
           /*GlobalInitializer*/ nullptr, /*VariableLinkage*/ nullptr,
           gVal->getType(), gVal);
 
+      bool requiresUSM = ompBuilder->Config.hasRequiresUnifiedSharedMemory();
       if (ompBuilder->Config.isTargetDevice() &&
-          (attribute.getCaptureClause().getValue() !=
-               mlir::omp::DeclareTargetCaptureClause::to ||
-           ompBuilder->Config.hasRequiresUnifiedSharedMemory())) {
+          (attribute.getCaptureClause().getValue() ==
+               mlir::omp::DeclareTargetCaptureClause::link ||
+           requiresUSM)) {
+        llvm::Type *ptrTy = gVal->getType();
+        // For USM the global type becomes a pointer handle, as opposed to the
+        // globals original type.
+        if (requiresUSM)
+          ptrTy = llvm::PointerType::get(llvmModule->getContext(), 0);
         ompBuilder->getAddrOfDeclareTargetVar(
             captureClause, deviceClause, isDeclaration, isExternallyVisible,
             ompBuilder->getTargetEntryUniqueInfo(fileInfoCallBack, vfs),
             mangledName, generatedRefs, /*OpenMPSimd*/ false, targetTriple,
-            gVal->getType(), /*GlobalInitializer*/ nullptr,
+            ptrTy, /*GlobalInitializer*/ nullptr,
             /*VariableLinkage*/ nullptr);
       }
     }
diff --git a/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-device-2.mlir b/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-device-2.mlir
new file mode 100644
index 0000000000000..6969f3030cba8
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-device-2.mlir
@@ -0,0 +1,50 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// This test checks that we correctly generate ref pointers with the correct type in USM mode
+// for link and to clauses. And verifies we continue to make the correct replacement accesses
+// within the target region.
+
+module attributes {llvm.target_triple = "amdgcn-amd-amdhsa", omp.is_target_device = true, omp.requires = #omp<clause_requires unified_shared_memory>} {
+  // CHECK-DAG: @_QMtest_0Evar_to_usm_decl_tgt_ref_ptr = weak global ptr null, align 8
+  llvm.mlir.global external @_QMtest_0Evar_to_usm() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : i32 {
+    %0 = llvm.mlir.constant(1 : i32) : i32
+    llvm.return %0 : i32
+  }
+
+  // CHECK-DAG: @_QMtest_0Evar_enter_usm_decl_tgt_ref_ptr = weak global ptr null, align 8
+  llvm.mlir.global external @_QMtest_0Evar_enter_usm() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (enter)>} : i32 {
+    %0 = llvm.mlir.constant(2 : i32) : i32
+    llvm.return %0 : i32
+  }
+
+  // CHECK-DAG: @_QMtest_0Evar_link_usm_decl_tgt_ref_ptr = weak global ptr null, align 8
+  llvm.mlir.global external @_QMtest_0Evar_link_usm() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : i32 {
+    %0 = llvm.mlir.constant(3 : i32) : i32
+    llvm.return %0 : i32
+  }
+
+  llvm.func @test_usm_declare_target() attributes {} {
+    %0 = llvm.mlir.addressof @_QMtest_0Evar_to_usm : !llvm.ptr
+    %1 = llvm.mlir.addressof @_QMtest_0Evar_enter_usm : !llvm.ptr
+    %2 = llvm.mlir.addressof @_QMtest_0Evar_link_usm : !llvm.ptr
+    // CHECK-DAG: %[[TO_VAR:.*]] = load ptr, ptr @_QMtest_0Evar_to_usm_decl_tgt_ref_ptr, align 8
+    // CHECK-DAG: store i32 10, ptr %[[TO_VAR]], align 4
+    // CHECK-DAG: %[[ENTER_VAR:.*]] = load ptr, ptr @_QMtest_0Evar_enter_usm_decl_tgt_ref_ptr, align 8
+    // CHECK-DAG: store i32 20, ptr %[[ENTER_VAR]], align 4
+    // CHECK-DAG: %[[LINK_VAR:.*]] = load ptr, ptr @_QMtest_0Evar_link_usm_decl_tgt_ref_ptr, align 8
+    // CHECK-DAG: store i32 30, ptr %[[LINK_VAR]], align 4
+    %map0 = omp.map.info var_ptr(%0 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+    %map1 = omp.map.info var_ptr(%1 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+    %map2 = omp.map.info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+    omp.target map_entries(%map0 -> %arg0, %map1 -> %arg1, %map2 -> %arg2 : !llvm.ptr, !llvm.ptr, !llvm.ptr) {
+      %c10 = llvm.mlir.constant(10 : i32) : i32
+      %c20 = llvm.mlir.constant(20 : i32) : i32
+      %c30 = llvm.mlir.constant(30 : i32) : i32
+      llvm.store %c10, %arg0 : i32, !llvm.ptr
+      llvm.store %c20, %arg1 : i32, !llvm.ptr
+      llvm.store %c30, %arg2 : i32, !llvm.ptr
+      omp.terminator
+    }
+    llvm.return
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/200248