[Flang][OpenMP] Fix update operation not found issue #92165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 llvm#91929
|
@llvm/pr-subscribers-mlir-openmp @llvm/pr-subscribers-mlir-llvm Author: Kiran Chandramohan (kiranchandramohan) ChangesIf 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 Full diff: https://github.com/llvm/llvm-project/pull/92165.diff 1 Files Affected:
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index a7294632d6667..ed9fb44cf08ed 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,
|
|
@llvm/pr-subscribers-mlir Author: Kiran Chandramohan (kiranchandramohan) ChangesIf 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 Full diff: https://github.com/llvm/llvm-project/pull/92165.diff 1 Files Affected:
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index a7294632d6667..ed9fb44cf08ed 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,
|
NimishMishra
approved these changes
May 15, 2024
Dinistro
reviewed
May 15, 2024
Yes. Done. |
Dinistro
approved these changes
May 15, 2024
mub-at-arm
pushed a commit
to mub-at-arm/llvm-project
that referenced
this pull request
May 16, 2024
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 llvm#91929
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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