Skip to content

Commit 89ee3ae

Browse files
[Flang][OpenMP] Fix update operation not found issue (#92165)
If there is only one non-terminator operation in the update region then the update operation can be found and we can try to generate an atomicrmw instruction. Otherwise use the cmpxchg loop. Fixes #91929
1 parent 7c95629 commit 89ee3ae

File tree

2 files changed

+45
-21
lines changed

2 files changed

+45
-21
lines changed

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,31 +1623,33 @@ convertOmpAtomicUpdate(omp::AtomicUpdateOp &opInst,
16231623

16241624
// Convert values and types.
16251625
auto &innerOpList = opInst.getRegion().front().getOperations();
1626-
bool isRegionArgUsed{false}, isXBinopExpr{false};
1626+
bool isXBinopExpr{false};
16271627
llvm::AtomicRMWInst::BinOp binop;
16281628
mlir::Value mlirExpr;
1629-
// Find the binary update operation that uses the region argument
1630-
// and get the expression to update
1631-
for (Operation &innerOp : innerOpList) {
1632-
if (innerOp.getNumOperands() == 2) {
1633-
binop = convertBinOpToAtomic(innerOp);
1634-
if (!llvm::is_contained(innerOp.getOperands(),
1635-
opInst.getRegion().getArgument(0)))
1636-
continue;
1637-
isRegionArgUsed = true;
1638-
isXBinopExpr = innerOp.getNumOperands() > 0 &&
1639-
innerOp.getOperand(0) == opInst.getRegion().getArgument(0);
1640-
mlirExpr = (isXBinopExpr ? innerOp.getOperand(1) : innerOp.getOperand(0));
1641-
break;
1629+
llvm::Value *llvmExpr = nullptr;
1630+
llvm::Value *llvmX = nullptr;
1631+
llvm::Type *llvmXElementType = nullptr;
1632+
if (innerOpList.size() == 2) {
1633+
// The two operations here are the update and the terminator.
1634+
// Since we can identify the update operation, there is a possibility
1635+
// that we can generate the atomicrmw instruction.
1636+
mlir::Operation &innerOp = *opInst.getRegion().front().begin();
1637+
if (!llvm::is_contained(innerOp.getOperands(),
1638+
opInst.getRegion().getArgument(0))) {
1639+
return opInst.emitError("no atomic update operation with region argument"
1640+
" as operand found inside atomic.update region");
16421641
}
1642+
binop = convertBinOpToAtomic(innerOp);
1643+
isXBinopExpr = innerOp.getOperand(0) == opInst.getRegion().getArgument(0);
1644+
mlirExpr = (isXBinopExpr ? innerOp.getOperand(1) : innerOp.getOperand(0));
1645+
llvmExpr = moduleTranslation.lookupValue(mlirExpr);
1646+
} else {
1647+
// Since the update region includes more than one operation
1648+
// we will resort to generating a cmpxchg loop.
1649+
binop = llvm::AtomicRMWInst::BinOp::BAD_BINOP;
16431650
}
1644-
if (!isRegionArgUsed)
1645-
return opInst.emitError("no atomic update operation with region argument"
1646-
" as operand found inside atomic.update region");
1647-
1648-
llvm::Value *llvmExpr = moduleTranslation.lookupValue(mlirExpr);
1649-
llvm::Value *llvmX = moduleTranslation.lookupValue(opInst.getX());
1650-
llvm::Type *llvmXElementType = moduleTranslation.convertType(
1651+
llvmX = moduleTranslation.lookupValue(opInst.getX());
1652+
llvmXElementType = moduleTranslation.convertType(
16511653
opInst.getRegion().getArgument(0).getType());
16521654
llvm::OpenMPIRBuilder::AtomicOpValue llvmAtomicX = {llvmX, llvmXElementType,
16531655
/*isSigned=*/false,

mlir/test/Target/LLVMIR/openmp-llvm.mlir

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,28 @@ llvm.func @omp_atomic_update_intrinsic(%x:!llvm.ptr, %expr: i32) {
14671467

14681468
// -----
14691469

1470+
// CHECK-LABEL: @atomic_update_cmpxchg
1471+
// CHECK-SAME: (ptr %[[X:.*]], ptr %[[EXPR:.*]]) {
1472+
// CHECK: %[[AT_LOAD_VAL:.*]] = load atomic i32, ptr %[[X]] monotonic, align 4
1473+
// CHECK: %[[LOAD_VAL_PHI:.*]] = phi i32 [ %[[AT_LOAD_VAL]], %entry ], [ %[[LOAD_VAL:.*]], %.atomic.cont ]
1474+
// CHECK: %[[VAL_SUCCESS:.*]] = cmpxchg ptr %[[X]], i32 %[[LOAD_VAL_PHI]], i32 %{{.*}} monotonic monotonic, align 4
1475+
// CHECK: %[[LOAD_VAL]] = extractvalue { i32, i1 } %[[VAL_SUCCESS]], 0
1476+
// CHECK: br i1 %{{.*}}, label %.atomic.exit, label %.atomic.cont
1477+
1478+
llvm.func @atomic_update_cmpxchg(%arg0: !llvm.ptr, %arg1: !llvm.ptr) {
1479+
%0 = llvm.load %arg1 : !llvm.ptr -> f32
1480+
omp.atomic.update %arg0 : !llvm.ptr {
1481+
^bb0(%arg2: i32):
1482+
%1 = llvm.sitofp %arg2 : i32 to f32
1483+
%2 = llvm.fadd %1, %0 : f32
1484+
%3 = llvm.fptosi %2 : f32 to i32
1485+
omp.yield(%3 : i32)
1486+
}
1487+
llvm.return
1488+
}
1489+
1490+
// -----
1491+
14701492
// CHECK-LABEL: @omp_atomic_capture_prefix_update
14711493
// CHECK-SAME: (ptr %[[x:.*]], ptr %[[v:.*]], i32 %[[expr:.*]], ptr %[[xf:.*]], ptr %[[vf:.*]], float %[[exprf:.*]])
14721494
llvm.func @omp_atomic_capture_prefix_update(

0 commit comments

Comments
 (0)