-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[MLIR][OpenMP] Add omp.private
op
#80955
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1594,6 +1594,73 @@ LogicalResult DataBoundsOp::verify() { | |
return success(); | ||
} | ||
|
||
LogicalResult PrivateClauseOp::verify() { | ||
Type symType = getType(); | ||
|
||
auto verifyTerminator = [&](Operation *terminator) -> LogicalResult { | ||
if (!terminator->hasSuccessors() && !llvm::isa<YieldOp>(terminator)) | ||
return mlir::emitError(terminator->getLoc()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the mlir prefix here and in several places below required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because I don't want this to get resolved to |
||
<< "expected exit block terminator to be an `omp.yield` op."; | ||
|
||
YieldOp yieldOp = llvm::cast<YieldOp>(terminator); | ||
TypeRange yieldedTypes = yieldOp.getResults().getTypes(); | ||
|
||
if (yieldedTypes.size() == 1 && yieldedTypes.front() == symType) | ||
return success(); | ||
|
||
auto error = mlir::emitError(yieldOp.getLoc()) | ||
<< "Invalid yielded value. Expected type: " << symType | ||
<< ", got: "; | ||
|
||
if (yieldedTypes.empty()) | ||
error << "None"; | ||
else | ||
error << yieldedTypes; | ||
|
||
return error; | ||
}; | ||
|
||
auto verifyRegion = [&](Region ®ion, unsigned expectedNumArgs, | ||
StringRef regionName) -> LogicalResult { | ||
assert(!region.empty()); | ||
|
||
if (region.getNumArguments() != expectedNumArgs) | ||
return mlir::emitError(region.getLoc()) | ||
<< "`" << regionName << "`: " | ||
<< "expected " << expectedNumArgs | ||
<< " region arguments, got: " << region.getNumArguments(); | ||
|
||
for (Block &block : region) { | ||
// MLIR will verify the absence of the terminator for us. | ||
if (!block.mightHaveTerminator()) | ||
continue; | ||
|
||
if (failed(verifyTerminator(block.getTerminator()))) | ||
return failure(); | ||
} | ||
|
||
return success(); | ||
}; | ||
|
||
if (failed(verifyRegion(getAllocRegion(), /*expectedNumArgs=*/1, "alloc"))) | ||
return failure(); | ||
|
||
DataSharingClauseType dsType = getDataSharingType(); | ||
|
||
if (dsType == DataSharingClauseType::Private && !getCopyRegion().empty()) | ||
return emitError("`private` clauses require only an `alloc` region."); | ||
|
||
if (dsType == DataSharingClauseType::FirstPrivate && getCopyRegion().empty()) | ||
return emitError( | ||
"`firstprivate` clauses require both `alloc` and `copy` regions."); | ||
|
||
if (dsType == DataSharingClauseType::FirstPrivate && | ||
failed(verifyRegion(getCopyRegion(), /*expectedNumArgs=*/2, "copy"))) | ||
return failure(); | ||
|
||
return success(); | ||
} | ||
|
||
#define GET_ATTRDEF_CLASSES | ||
#include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc" | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// RUN: mlir-opt -verify-diagnostics %s | mlir-opt | FileCheck %s | ||
|
||
// CHECK: omp.private {type = private} @x.privatizer : !llvm.ptr alloc { | ||
omp.private {type = private} @x.privatizer : !llvm.ptr alloc { | ||
// CHECK: ^bb0(%arg0: {{.*}}): | ||
^bb0(%arg0: !llvm.ptr): | ||
omp.yield(%arg0 : !llvm.ptr) | ||
} | ||
|
||
// CHECK: omp.private {type = firstprivate} @y.privatizer : !llvm.ptr alloc { | ||
omp.private {type = firstprivate} @y.privatizer : !llvm.ptr alloc { | ||
// CHECK: ^bb0(%arg0: {{.*}}): | ||
^bb0(%arg0: !llvm.ptr): | ||
omp.yield(%arg0 : !llvm.ptr) | ||
// CHECK: } copy { | ||
} copy { | ||
// CHECK: ^bb0(%arg0: {{.*}}, %arg1: {{.*}}): | ||
^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr): | ||
omp.yield(%arg0 : !llvm.ptr) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are the types different for alloc_region and copy_region?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the
alloc_region
is mandatory for bothprivate
andfirstprivate
while thecopy_region
is mandatory only forfirstprivate
. Therefore, thealloc_region
has to have at least one block while thecopy_region
might be empty.