Skip to content

[Flang][OpenMP] Fix update operation not found issue #92165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 16, 2024
Merged

[Flang][OpenMP] Fix update operation not found issue #92165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b036c9
[Flang][OpenMP] Fix update operation not found issue
kiranchandramohan May 14, 2024
cd8b717
[MLIR][OpenMP] Test for atomic update fix
kiranchandramohan May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 23 additions & 21 deletions mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 22 additions & 0 deletions mlir/test/Target/LLVMIR/openmp-llvm.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down