[Mlir-commits] [mlir] [mlir][OpenMP] Don't allow firstprivate for simd (PR #146734)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-openmp

Author: Tom Eccles (tblah)

<details>
<summary>Changes</summary>

This is not allowed by the openmp standard.

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


2 Files Affected:

- (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+22-1) 
- (modified) mlir/test/Target/LLVMIR/openmp-todo.mlir (+23) 


``````````diff
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 7b07243c5f843..b307de09fbcc4 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -326,6 +326,25 @@ static LogicalResult checkImplementationStatus(Operation &op) {
     if (op.getDistScheduleChunkSize())
       result = todo("dist_schedule with chunk_size");
   };
+  auto checkFirstprivate = [&todo](auto op, LogicalResult &result) {
+    // Firstprivate is not listed as supported by the simd operation in
+    // OpenMP 6.0. This is here to catch it because it is allowed at the dialect
+    // level.
+    if constexpr (std::is_same_v<std::decay_t<decltype(op)>, omp::SimdOp>) {
+      std::optional<ArrayAttr> privateSyms = op.getPrivateSyms();
+      if (!privateSyms)
+        return;
+      for (const Attribute &sym : *privateSyms) {
+        omp::PrivateClauseOp privatizer =
+            findPrivatizer(op, cast<SymbolRefAttr>(sym));
+        if (privatizer.getDataSharingType() ==
+            omp::DataSharingClauseType::FirstPrivate) {
+          result = todo("firstprivate");
+          break;
+        }
+      }
+    }
+  };
   auto checkHint = [](auto op, LogicalResult &) {
     if (op.getHint())
       op.emitWarning("hint clause discarded");
@@ -441,6 +460,7 @@ static LogicalResult checkImplementationStatus(Operation &op) {
         checkReduction(op, result);
       })
       .Case([&](omp::SimdOp op) {
+        checkFirstprivate(op, result);
         checkLinear(op, result);
         checkReduction(op, result);
       })
@@ -2894,7 +2914,8 @@ convertOmpSimd(Operation &opInst, llvm::IRBuilderBase &builder,
           .failed())
     return failure();
 
-  // TODO: no call to copyFirstPrivateVars?
+  // No call to copyFirstPrivateVars because firstprivate is not allowed on
+  // SIMD.
 
   assert(afterAllocas.get()->getSinglePredecessor());
   if (failed(initReductionVars(simdOp, reductionArgs, builder,
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index 29725a02c075a..e86a91dab873d 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -503,3 +503,26 @@ llvm.func @wsloop_order(%lb : i32, %ub : i32, %step : i32) {
   }
   llvm.return
 }
+
+// -----
+
+omp.private {type = firstprivate} @_QFEp_firstprivate_i32 : i32 copy {
+^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+  %0 = llvm.load %arg0 : !llvm.ptr -> i32
+  llvm.store %0, %arg1 : i32, !llvm.ptr
+  omp.yield(%arg1 : !llvm.ptr)
+}
+llvm.func @_QQmain() {
+  %0 = llvm.mlir.constant(1 : i64) : i64
+  %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr
+  %3 = llvm.mlir.constant(10 : i32) : i32
+  %4 = llvm.mlir.constant(1 : i32) : i32
+  // expected-error at below {{not yet implemented: Unhandled clause firstprivate in omp.simd operation}}
+  // expected-error at below {{LLVM Translation failed for operation: omp.simd}}
+  omp.simd private(@_QFEp_firstprivate_i32 %1 -> %arg0 : !llvm.ptr) {
+    omp.loop_nest (%arg2) : i32 = (%4) to (%3) inclusive step (%4) {
+      omp.yield
+    }
+  }
+  llvm.return
+}

``````````

</details>


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