diff options
author | Kiran Chandramohan <kiran.chandramohan@arm.com> | 2024-05-16 08:04:40 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-16 08:04:40 +0100 |
commit | 89ee3ae2bd1d5705a3e775e3f30bf0ec6d8d863a (patch) | |
tree | e3d55cc1a2ae49d1bb30374404b1f73d27daf9d0 | |
parent | 7c956293d856224dd6a1b633820ef53009f7ef1c (diff) | |
download | llvm-89ee3ae2bd1d5705a3e775e3f30bf0ec6d8d863a.tar.gz |
[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
-rw-r--r-- | mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp | 44 | ||||
-rw-r--r-- | mlir/test/Target/LLVMIR/openmp-llvm.mlir | 22 |
2 files changed, 45 insertions, 21 deletions
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index a7294632d666..ed9fb44cf08e 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -1623,31 +1623,33 @@ convertOmpAtomicUpdate(omp::AtomicUpdateOp &opInst, // Convert values and types. auto &innerOpList = opInst.getRegion().front().getOperations(); - bool isRegionArgUsed{false}, isXBinopExpr{false}; + bool isXBinopExpr{false}; llvm::AtomicRMWInst::BinOp binop; mlir::Value mlirExpr; - // Find the binary update operation that uses the region argument - // and get the expression to update - for (Operation &innerOp : innerOpList) { - if (innerOp.getNumOperands() == 2) { - binop = convertBinOpToAtomic(innerOp); - if (!llvm::is_contained(innerOp.getOperands(), - opInst.getRegion().getArgument(0))) - continue; - isRegionArgUsed = true; - isXBinopExpr = innerOp.getNumOperands() > 0 && - innerOp.getOperand(0) == opInst.getRegion().getArgument(0); - mlirExpr = (isXBinopExpr ? innerOp.getOperand(1) : innerOp.getOperand(0)); - break; + llvm::Value *llvmExpr = nullptr; + llvm::Value *llvmX = nullptr; + llvm::Type *llvmXElementType = nullptr; + if (innerOpList.size() == 2) { + // The two operations here are the update and the terminator. + // Since we can identify the update operation, there is a possibility + // that we can generate the atomicrmw instruction. + mlir::Operation &innerOp = *opInst.getRegion().front().begin(); + if (!llvm::is_contained(innerOp.getOperands(), + opInst.getRegion().getArgument(0))) { + return opInst.emitError("no atomic update operation with region argument" + " as operand found inside atomic.update region"); } + binop = convertBinOpToAtomic(innerOp); + isXBinopExpr = innerOp.getOperand(0) == opInst.getRegion().getArgument(0); + mlirExpr = (isXBinopExpr ? innerOp.getOperand(1) : innerOp.getOperand(0)); + llvmExpr = moduleTranslation.lookupValue(mlirExpr); + } else { + // Since the update region includes more than one operation + // we will resort to generating a cmpxchg loop. + binop = llvm::AtomicRMWInst::BinOp::BAD_BINOP; } - if (!isRegionArgUsed) - return opInst.emitError("no atomic update operation with region argument" - " as operand found inside atomic.update region"); - - llvm::Value *llvmExpr = moduleTranslation.lookupValue(mlirExpr); - llvm::Value *llvmX = moduleTranslation.lookupValue(opInst.getX()); - llvm::Type *llvmXElementType = moduleTranslation.convertType( + llvmX = moduleTranslation.lookupValue(opInst.getX()); + llvmXElementType = moduleTranslation.convertType( opInst.getRegion().getArgument(0).getType()); llvm::OpenMPIRBuilder::AtomicOpValue llvmAtomicX = {llvmX, llvmXElementType, /*isSigned=*/false, diff --git a/mlir/test/Target/LLVMIR/openmp-llvm.mlir b/mlir/test/Target/LLVMIR/openmp-llvm.mlir index ad40ca26bec9..8654899efefd 100644 --- a/mlir/test/Target/LLVMIR/openmp-llvm.mlir +++ b/mlir/test/Target/LLVMIR/openmp-llvm.mlir @@ -1467,6 +1467,28 @@ llvm.func @omp_atomic_update_intrinsic(%x:!llvm.ptr, %expr: i32) { // ----- +// CHECK-LABEL: @atomic_update_cmpxchg +// CHECK-SAME: (ptr %[[X:.*]], ptr %[[EXPR:.*]]) { +// CHECK: %[[AT_LOAD_VAL:.*]] = load atomic i32, ptr %[[X]] monotonic, align 4 +// CHECK: %[[LOAD_VAL_PHI:.*]] = phi i32 [ %[[AT_LOAD_VAL]], %entry ], [ %[[LOAD_VAL:.*]], %.atomic.cont ] +// CHECK: %[[VAL_SUCCESS:.*]] = cmpxchg ptr %[[X]], i32 %[[LOAD_VAL_PHI]], i32 %{{.*}} monotonic monotonic, align 4 +// CHECK: %[[LOAD_VAL]] = extractvalue { i32, i1 } %[[VAL_SUCCESS]], 0 +// CHECK: br i1 %{{.*}}, label %.atomic.exit, label %.atomic.cont + +llvm.func @atomic_update_cmpxchg(%arg0: !llvm.ptr, %arg1: !llvm.ptr) { + %0 = llvm.load %arg1 : !llvm.ptr -> f32 + omp.atomic.update %arg0 : !llvm.ptr { + ^bb0(%arg2: i32): + %1 = llvm.sitofp %arg2 : i32 to f32 + %2 = llvm.fadd %1, %0 : f32 + %3 = llvm.fptosi %2 : f32 to i32 + omp.yield(%3 : i32) + } + llvm.return +} + +// ----- + // CHECK-LABEL: @omp_atomic_capture_prefix_update // CHECK-SAME: (ptr %[[x:.*]], ptr %[[v:.*]], i32 %[[expr:.*]], ptr %[[xf:.*]], ptr %[[vf:.*]], float %[[exprf:.*]]) llvm.func @omp_atomic_capture_prefix_update( |