[Mlir-commits] [llvm] [mlir] [OpenMP][OMPIRBuilder] Support complex types in atomic update/capture (PR #191490)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Apr 13 22:50:52 PDT 2026
https://github.com/chichunchen updated https://github.com/llvm/llvm-project/pull/191490
>From 4b00dd5722213277e749c723e96e778040cab176 Mon Sep 17 00:00:00 2001
From: "Chi Chun, Chen" <chichun.chen at hpe.com>
Date: Fri, 10 Apr 2026 02:32:49 -0500
Subject: [PATCH 1/2] [OpenMP][OMPIRBuilder] Support complex types in atomic
update/capture
Route struct-typed values through the libcall path in
`emitAtomicUpdate`.
Previously, the libcall path was gated on `RMWOp == BAD_BINOP`, so
atomic capture swap patterns (`v = x; x = expr`) for complex values
lowered as structs fell through to the cmpxchg path. That path called
`getScalarSizeInBits()` on a struct type, produced 0, and triggered an
assertion in `IntegerType::get()`.
Remove the `BAD_BINOP` restriction so struct types always use the
libcall path. This is safe because the libcall path does not use
`RMWOp` and already handles arbitrary type sizes correctly.
Also fix `LoadSize` in the libcall path to use `XElemTy` rather than
the pointer type, which previously gave the wrong size for larger
complex types such as `complex(8)`.
Fixes https://github.com/llvm/llvm-project/issues/191317
Assisted with copilot and GPT-5.4
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 13 +++++--------
mlir/test/Target/LLVMIR/openmp-llvm.mlir | 22 ++++++++++++++++++++++
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 7b82e7abbf957..228e2ac581bcb 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -10475,7 +10475,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createAtomicUpdate(
"OMP Atomic expects a pointer to target memory");
Type *XElemTy = X.ElemTy;
assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
- XElemTy->isPointerTy()) &&
+ XElemTy->isPointerTy() || XElemTy->isStructTy()) &&
"OMP atomic update expected a scalar type");
assert((RMWOp != AtomicRMWInst::Max) && (RMWOp != AtomicRMWInst::Min) &&
(RMWOp != AtomicRMWInst::UMax) && (RMWOp != AtomicRMWInst::UMin) &&
@@ -10535,8 +10535,7 @@ Expected<std::pair<Value *, Value *>> OpenMPIRBuilder::emitAtomicUpdate(
AtomicOrdering AO, AtomicRMWInst::BinOp RMWOp,
AtomicUpdateCallbackTy &UpdateOp, bool VolatileX, bool IsXBinopExpr,
bool IsIgnoreDenormalMode, bool IsFineGrainedMemory, bool IsRemoteMemory) {
- // TODO: handle the case where XElemTy is not byte-sized or not a power of 2
- // or a complex datatype.
+ // TODO: handle the case where XElemTy is not byte-sized or not a power of 2.
bool emitRMWOp = false;
switch (RMWOp) {
case AtomicRMWInst::Add:
@@ -10578,14 +10577,12 @@ Expected<std::pair<Value *, Value *>> OpenMPIRBuilder::emitAtomicUpdate(
Res.second = Res.first;
else
Res.second = emitRMWOpAsInstruction(Res.first, Expr, RMWOp);
- } else if (RMWOp == llvm::AtomicRMWInst::BinOp::BAD_BINOP &&
- XElemTy->isStructTy()) {
+ } else if (XElemTy->isStructTy()) {
LoadInst *OldVal =
Builder.CreateLoad(XElemTy, X, X->getName() + ".atomic.load");
OldVal->setAtomic(AO);
const DataLayout &LoadDL = OldVal->getModule()->getDataLayout();
- unsigned LoadSize =
- LoadDL.getTypeStoreSize(OldVal->getPointerOperand()->getType());
+ unsigned LoadSize = LoadDL.getTypeStoreSize(XElemTy);
OpenMPIRBuilder::AtomicInfo atomicInfo(
&Builder, XElemTy, LoadSize * 8, LoadSize * 8, OldVal->getAlign(),
@@ -10713,7 +10710,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createAtomicCapture(
"OMP Atomic expects a pointer to target memory");
Type *XElemTy = X.ElemTy;
assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
- XElemTy->isPointerTy()) &&
+ XElemTy->isPointerTy() || XElemTy->isStructTy()) &&
"OMP atomic capture expected a scalar type");
assert((RMWOp != AtomicRMWInst::Max) && (RMWOp != AtomicRMWInst::Min) &&
"OpenMP atomic does not support LT or GT operations");
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index c5cdecd091770..795297cd94d42 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -1766,6 +1766,28 @@ llvm.func @_QPomp_atomic_capture_complex() {
// -----
+// CHECK-LABEL: define void @omp_atomic_capture_complex_swap
+llvm.func @omp_atomic_capture_complex_swap(%x_arg: !llvm.ptr, %v_arg: !llvm.ptr) {
+ // CHECK: %[[ATOMIC_TEMP_LOAD:.*]] = alloca { float, float }, align 8
+ // CHECK: %[[X_NEW_VAL:.*]] = alloca { float, float }, align 8
+ // CHECK: call void @__atomic_load(i64 8, ptr %{{.*}}, ptr %[[ATOMIC_TEMP_LOAD]], i32 0)
+ // CHECK: %[[PHI:.*]] = phi { float, float }
+ // CHECK: store { float, float } { float 1.000000e+00, float 1.000000e+00 }, ptr %[[X_NEW_VAL]], align 4
+ // CHECK: call i1 @__atomic_compare_exchange(i64 8, ptr %{{.*}}, ptr %[[ATOMIC_TEMP_LOAD]], ptr %[[X_NEW_VAL]], i32 2, i32 2)
+ // CHECK: store { float, float } %[[PHI]], ptr %{{.*}}, align 4
+ %0 = llvm.mlir.constant(1.000000e+00 : f32) : f32
+ %1 = llvm.mlir.undef : !llvm.struct<(f32, f32)>
+ %2 = llvm.insertvalue %0, %1[0] : !llvm.struct<(f32, f32)>
+ %3 = llvm.insertvalue %0, %2[1] : !llvm.struct<(f32, f32)>
+ omp.atomic.capture {
+ omp.atomic.read %v_arg = %x_arg : !llvm.ptr, !llvm.ptr, !llvm.struct<(f32, f32)>
+ omp.atomic.write %x_arg = %3 : !llvm.ptr, !llvm.struct<(f32, f32)>
+ }
+ llvm.return
+}
+
+// -----
+
// CHECK-LABEL: define void @omp_atomic_read_complex() {
llvm.func @omp_atomic_read_complex(){
>From 2db12cc7ecc667e3ce1628f54fd3b4a4b5177436 Mon Sep 17 00:00:00 2001
From: "Chi Chun, Chen" <chichun.chen at hpe.com>
Date: Tue, 14 Apr 2026 00:38:32 -0500
Subject: [PATCH 2/2] Add test for fallback path and update assert message
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 4 +-
mlir/test/Target/LLVMIR/openmp-llvm.mlir | 65 +++++++++++++++++++++++
2 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 228e2ac581bcb..d3e1d3315cad8 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -10476,7 +10476,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createAtomicUpdate(
Type *XElemTy = X.ElemTy;
assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
XElemTy->isPointerTy() || XElemTy->isStructTy()) &&
- "OMP atomic update expected a scalar type");
+ "OMP atomic update expected a scalar or struct type");
assert((RMWOp != AtomicRMWInst::Max) && (RMWOp != AtomicRMWInst::Min) &&
(RMWOp != AtomicRMWInst::UMax) && (RMWOp != AtomicRMWInst::UMin) &&
"OpenMP atomic does not support LT or GT operations");
@@ -10711,7 +10711,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createAtomicCapture(
Type *XElemTy = X.ElemTy;
assert((XElemTy->isFloatingPointTy() || XElemTy->isIntegerTy() ||
XElemTy->isPointerTy() || XElemTy->isStructTy()) &&
- "OMP atomic capture expected a scalar type");
+ "OMP atomic capture expected a scalar or struct type");
assert((RMWOp != AtomicRMWInst::Max) && (RMWOp != AtomicRMWInst::Min) &&
"OpenMP atomic does not support LT or GT operations");
});
diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
index 795297cd94d42..d093fea3c5cb7 100644
--- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -1766,6 +1766,49 @@ llvm.func @_QPomp_atomic_capture_complex() {
// -----
+// Non-struct types with BAD_BINOP (multi-step) still use the cmpxchg fallback path.
+//
+// CHECK-LABEL: @atomic_update_float_multi_step
+// CHECK-SAME: (ptr %[[x:.*]], float %[[a:.*]], float %[[b:.*]])
+// CHECK: %[[load:.*]] = load atomic i32, ptr %[[x]] monotonic
+// CHECK: %[[phi:.*]] = phi i32
+// CHECK: %[[fltCast:.*]] = bitcast i32 %[[phi]] to float
+// CHECK: %[[t:.*]] = fmul float %[[fltCast]], %[[a]]
+// CHECK: %[[r:.*]] = fadd float %[[t]], %[[b]]
+// CHECK: cmpxchg ptr %[[x]], i32 %[[phi]], i32 %{{.*}} monotonic monotonic
+llvm.func @atomic_update_float_multi_step(%x: !llvm.ptr, %a: f32, %b: f32) {
+ omp.atomic.update %x : !llvm.ptr {
+ ^bb0(%xval: f32):
+ %t = llvm.fmul %xval, %a : f32
+ %r = llvm.fadd %t, %b : f32
+ omp.yield(%r : f32)
+ }
+ llvm.return
+}
+
+// -----
+
+// Non-struct types with BAD_BINOP (unrecognized) ops still
+// use the cmpxchg fallback path.
+//
+// CHECK-LABEL: @atomic_update_float_intrinsic
+// CHECK-SAME: (ptr %[[x:.*]], float %[[expr:.*]])
+// CHECK: %[[load:.*]] = load atomic i32, ptr %[[x]] monotonic
+// CHECK: %[[phi:.*]] = phi i32
+// CHECK: %[[fltCast:.*]] = bitcast i32 %[[phi]] to float
+// CHECK: %[[res:.*]] = call float @llvm.maxnum.f32(float %[[fltCast]], float %[[expr]])
+// CHECK: cmpxchg ptr %[[x]], i32 %[[phi]], i32 %{{.*}} monotonic monotonic
+llvm.func @atomic_update_float_intrinsic(%x: !llvm.ptr, %expr: f32) {
+ omp.atomic.update %x : !llvm.ptr {
+ ^bb0(%xval: f32):
+ %newval = "llvm.intr.maxnum"(%xval, %expr) : (f32, f32) -> f32
+ omp.yield(%newval : f32)
+ }
+ llvm.return
+}
+
+// -----
+
// CHECK-LABEL: define void @omp_atomic_capture_complex_swap
llvm.func @omp_atomic_capture_complex_swap(%x_arg: !llvm.ptr, %v_arg: !llvm.ptr) {
// CHECK: %[[ATOMIC_TEMP_LOAD:.*]] = alloca { float, float }, align 8
@@ -1788,6 +1831,28 @@ llvm.func @omp_atomic_capture_complex_swap(%x_arg: !llvm.ptr, %v_arg: !llvm.ptr)
// -----
+// CHECK-LABEL: define void @omp_atomic_capture_complex8_swap
+llvm.func @omp_atomic_capture_complex8_swap(%x_arg: !llvm.ptr, %v_arg: !llvm.ptr) {
+ // CHECK: %[[ATOMIC_TEMP_LOAD:.*]] = alloca { double, double }, align 8
+ // CHECK: %[[X_NEW_VAL:.*]] = alloca { double, double }, align 8
+ // CHECK: call void @__atomic_load(i64 16, ptr %{{.*}}, ptr %[[ATOMIC_TEMP_LOAD]], i32 0)
+ // CHECK: %[[PHI:.*]] = phi { double, double }
+ // CHECK: store { double, double } { double 1.000000e+00, double 1.000000e+00 }, ptr %[[X_NEW_VAL]], align 8
+ // CHECK: call i1 @__atomic_compare_exchange(i64 16, ptr %{{.*}}, ptr %[[ATOMIC_TEMP_LOAD]], ptr %[[X_NEW_VAL]], i32 2, i32 2)
+ // CHECK: store { double, double } %[[PHI]], ptr %{{.*}}, align 8
+ %0 = llvm.mlir.constant(1.000000e+00 : f64) : f64
+ %1 = llvm.mlir.undef : !llvm.struct<(f64, f64)>
+ %2 = llvm.insertvalue %0, %1[0] : !llvm.struct<(f64, f64)>
+ %3 = llvm.insertvalue %0, %2[1] : !llvm.struct<(f64, f64)>
+ omp.atomic.capture {
+ omp.atomic.read %v_arg = %x_arg : !llvm.ptr, !llvm.ptr, !llvm.struct<(f64, f64)>
+ omp.atomic.write %x_arg = %3 : !llvm.ptr, !llvm.struct<(f64, f64)>
+ }
+ llvm.return
+}
+
+// -----
+
// CHECK-LABEL: define void @omp_atomic_read_complex() {
llvm.func @omp_atomic_read_complex(){
More information about the Mlir-commits
mailing list