[Mlir-commits] [mlir] [mlir][OpenMP] Don't allow firstprivate for simd (PR #146734)
Tom Eccles
llvmlistbot at llvm.org
Thu Jul 3 04:10:00 PDT 2025
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/146734
>From 57e389c2a0cac02f29f606f3ed9761858be4d9ff Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Wed, 2 Jul 2025 15:48:50 +0000
Subject: [PATCH 1/3] [mlir][OpenMP] Don't allow firstprivate for simd
This is not allowed by the openmp standard.
---
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 23 ++++++++++++++++++-
mlir/test/Target/LLVMIR/openmp-todo.mlir | 23 +++++++++++++++++++
2 files changed, 45 insertions(+), 1 deletion(-)
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
+}
>From 683dc84ff16d4b8eb09181f20822d4bb62ca64e0 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 3 Jul 2025 10:46:49 +0000
Subject: [PATCH 2/3] Revert "[mlir][OpenMP] Don't allow firstprivate for simd"
This reverts commit 57e389c2a0cac02f29f606f3ed9761858be4d9ff.
---
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 23 +------------------
mlir/test/Target/LLVMIR/openmp-todo.mlir | 23 -------------------
2 files changed, 1 insertion(+), 45 deletions(-)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index b307de09fbcc4..7b07243c5f843 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -326,25 +326,6 @@ 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");
@@ -460,7 +441,6 @@ static LogicalResult checkImplementationStatus(Operation &op) {
checkReduction(op, result);
})
.Case([&](omp::SimdOp op) {
- checkFirstprivate(op, result);
checkLinear(op, result);
checkReduction(op, result);
})
@@ -2914,8 +2894,7 @@ convertOmpSimd(Operation &opInst, llvm::IRBuilderBase &builder,
.failed())
return failure();
- // No call to copyFirstPrivateVars because firstprivate is not allowed on
- // SIMD.
+ // TODO: no call to copyFirstPrivateVars?
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 e86a91dab873d..29725a02c075a 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -503,26 +503,3 @@ 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
-}
>From 971daec68cf21ded2b5abf6153a8de66edf15948 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 3 Jul 2025 11:08:31 +0000
Subject: [PATCH 3/3] Disallow SIMD FIRSTPRIVATE in the MLIR verifier
---
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 19 +++++++++++
mlir/test/Dialect/OpenMP/invalid.mlir | 33 ++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index e94d570b57122..ffc84781f77ff 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -15,11 +15,13 @@
#include "mlir/Dialect/Func/IR/FuncOps.h"
#include "mlir/Dialect/LLVMIR/LLVMTypes.h"
#include "mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.h"
+#include "mlir/Dialect/OpenMP/OpenMPClauseOperands.h"
#include "mlir/IR/Attributes.h"
#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/DialectImplementation.h"
#include "mlir/IR/OpImplementation.h"
#include "mlir/IR/OperationSupport.h"
+#include "mlir/IR/SymbolTable.h"
#include "mlir/Interfaces/FoldInterfaces.h"
#include "llvm/ADT/ArrayRef.h"
@@ -2640,6 +2642,23 @@ LogicalResult SimdOp::verify() {
return emitError()
<< "'omp.composite' attribute present in non-composite wrapper";
+ // Firstprivate is not allowed for SIMD in the standard. Check that none of
+ // the private decls are for firstprivate.
+ std::optional<ArrayAttr> privateSyms = getPrivateSyms();
+ if (privateSyms) {
+ for (const Attribute &sym : *privateSyms) {
+ auto symRef = cast<SymbolRefAttr>(sym);
+ omp::PrivateClauseOp privatizer =
+ SymbolTable::lookupNearestSymbolFrom<omp::PrivateClauseOp>(
+ getOperation(), symRef);
+ if (!privatizer)
+ return emitError() << "Cannot find privatizer '" << symRef << "'";
+ if (privatizer.getDataSharingType() ==
+ DataSharingClauseType::FirstPrivate)
+ return emitError() << "FIRSTPRIVATE cannot be used with SIMD";
+ }
+ }
+
return success();
}
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 060b3cd2455a0..7608ad57c7967 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -480,6 +480,39 @@ func.func @omp_simd_pretty_simdlen_safelen(%lb : index, %ub : index, %step : ind
// -----
+func.func @omp_simd_bad_privatizer(%lb : index, %ub : index, %step : index) {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr
+ // expected-error @below {{Cannot find privatizer '@not_defined'}}
+ omp.simd private(@not_defined %1 -> %arg0 : !llvm.ptr) {
+ omp.loop_nest (%arg2) : index = (%lb) to (%ub) inclusive step (%step) {
+ omp.yield
+ }
+ }
+}
+
+// -----
+
+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)
+}
+func.func @omp_simd_firstprivate(%lb : index, %ub : index, %step : index) {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr
+ // expected-error @below {{FIRSTPRIVATE cannot be used with SIMD}}
+ omp.simd private(@_QFEp_firstprivate_i32 %1 -> %arg0 : !llvm.ptr) {
+ omp.loop_nest (%arg2) : index = (%lb) to (%ub) inclusive step (%step) {
+ omp.yield
+ }
+ }
+ llvm.return
+}
+
+// -----
+
// expected-error @below {{op expects alloc region to yield a value of the reduction type}}
omp.declare_reduction @add_f32 : f32
alloc {
More information about the Mlir-commits
mailing list