Skip to content

[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

Merged
merged 1 commit into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
93 changes: 92 additions & 1 deletion mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,97 @@ def DeclareTargetAttr : OpenMP_Attr<"DeclareTarget", "declaretarget"> {
let assemblyFormat = "`<` struct(params) `>`";
}

//===----------------------------------------------------------------------===//
// 2.19.4 Data-Sharing Attribute Clauses
//===----------------------------------------------------------------------===//

def DataSharingTypePrivate : I32EnumAttrCase<"Private", 0, "private">;
def DataSharingTypeFirstPrivate : I32EnumAttrCase<"FirstPrivate", 1, "firstprivate">;

def DataSharingClauseType : I32EnumAttr<
"DataSharingClauseType",
"Type of a data-sharing clause",
[DataSharingTypePrivate, DataSharingTypeFirstPrivate]> {
let genSpecializedAttr = 0;
let cppNamespace = "::mlir::omp";
}

def DataSharingClauseTypeAttr : EnumAttr<
OpenMP_Dialect, DataSharingClauseType, "data_sharing_type"> {
let assemblyFormat = "`{` `type` `=` $value `}`";
}

def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove]> {
let summary = "Provides declaration of [first]private logic.";
let description = [{
This operation provides a declaration of how to implement the
[first]privatization of a variable. The dialect users should provide
information about how to create an instance of the type in the alloc region
and how to initialize the copy from the original item in the copy region.

Examples:
---------
* `private(x)` would be emitted as:
```mlir
omp.private {type = private} @x.privatizer : !fir.ref<i32> alloc {
^bb0(%arg0: !fir.ref<i32>):
%0 = ... allocate proper memory for the private clone ...
omp.yield(%0 : !fir.ref<i32>)
}
```

* `firstprivate(x)` would be emitted as:
```mlir
omp.private {type = firstprivate} @x.privatizer : !fir.ref<i32> alloc {
^bb0(%arg0: !fir.ref<i32>):
%0 = ... allocate proper memory for the private clone ...
omp.yield(%0 : !fir.ref<i32>)
} copy {
^bb0(%arg0: !fir.ref<i32>, %arg1: !fir.ref<i32>):
// %arg0 is the original host variable. Same as for `alloc`.
// %arg1 represents the memory allocated in `alloc`.
... copy from host to the privatized clone ....
omp.yield(%arg1 : !fir.ref<i32>)
}
```

There are no restrictions on the body except for:
- The `alloc` region has a single argument.
- The `copy` region has 2 arguments.
- Both regions are terminated by `omp.yield` ops.
The above restrictions and other obvious restrictions (e.g. verifying the
type of yielded values) are verified by the custom op verifier. The actual
contents of the blocks inside both regions are not verified.

Instances of this op would then be used by ops that model directives that
accept data-sharing attribute clauses.

The $sym_name attribute provides a symbol by which the privatizer op can be
referenced by other dialect ops.

The $type attribute is the type of the value being privatized.

The $data_sharing_type attribute specifies whether privatizer corresponds
to a `private` or a `firstprivate` clause.
}];

let arguments = (ins SymbolNameAttr:$sym_name,
TypeAttrOf<AnyType>:$type,
DataSharingClauseTypeAttr:$data_sharing_type);

let regions = (region MinSizedRegion<1>:$alloc_region,
AnyRegion:$copy_region);
Comment on lines +214 to +215
Copy link
Contributor

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?

Copy link
Member Author

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 both private and firstprivate while the copy_region is mandatory only for firstprivate. Therefore, the alloc_region has to have at least one block while the copy_region might be empty.


let assemblyFormat = [{
$data_sharing_type $sym_name `:` $type
`alloc` $alloc_region
(`copy` $copy_region^)?
attr-dict
}];

let hasVerifier = 1;
}

//===----------------------------------------------------------------------===//
// 2.6 parallel Construct
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -612,7 +703,7 @@ def SimdLoopOp : OpenMP_Op<"simdloop", [AttrSizedOperandSegments,
def YieldOp : OpenMP_Op<"yield",
[Pure, ReturnLike, Terminator,
ParentOneOf<["WsLoopOp", "ReductionDeclareOp",
"AtomicUpdateOp", "SimdLoopOp"]>]> {
"AtomicUpdateOp", "SimdLoopOp", "PrivateClauseOp"]>]> {
let summary = "loop yield and termination operation";
let description = [{
"omp.yield" yields SSA values from the OpenMP dialect op region and
Expand Down
67 changes: 67 additions & 0 deletions mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the mlir prefix here and in several places below required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because I don't want this to get resolved to this->emitError(..) in order to provide more accurate location information when needed.

<< "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 &region, 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"

Expand Down
63 changes: 63 additions & 0 deletions mlir/test/Dialect/OpenMP/invalid.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -1738,3 +1738,66 @@ func.func @omp_distribute(%data_var : memref<i32>) -> () {
"omp.terminator"() : () -> ()
}) : (memref<i32>) -> ()
}

// -----

omp.private {type = private} @x.privatizer : i32 alloc {
^bb0(%arg0: i32):
%0 = arith.constant 0.0 : f32
// expected-error @below {{Invalid yielded value. Expected type: 'i32', got: 'f32'}}
omp.yield(%0 : f32)
}

// -----

omp.private {type = private} @x.privatizer : i32 alloc {
^bb0(%arg0: i32):
// expected-error @below {{Invalid yielded value. Expected type: 'i32', got: None}}
omp.yield
}

// -----

omp.private {type = private} @x.privatizer : i32 alloc {
^bb0(%arg0: i32):
// expected-error @below {{expected exit block terminator to be an `omp.yield` op.}}
omp.terminator
}

// -----

// expected-error @below {{`alloc`: expected 1 region arguments, got: 2}}
omp.private {type = private} @x.privatizer : f32 alloc {
^bb0(%arg0: f32, %arg1: f32):
omp.yield(%arg0 : f32)
}

// -----

// expected-error @below {{`copy`: expected 2 region arguments, got: 1}}
omp.private {type = firstprivate} @x.privatizer : f32 alloc {
^bb0(%arg0: f32):
omp.yield(%arg0 : f32)
} copy {
^bb0(%arg0: f32):
omp.yield(%arg0 : f32)
}

// -----

// expected-error @below {{`private` clauses require only an `alloc` region.}}
omp.private {type = private} @x.privatizer : f32 alloc {
^bb0(%arg0: f32):
omp.yield(%arg0 : f32)
} copy {
^bb0(%arg0: f32, %arg1 : f32):
omp.yield(%arg0 : f32)
}

// -----

// expected-error @below {{`firstprivate` clauses require both `alloc` and `copy` regions.}}
omp.private {type = firstprivate} @x.privatizer : f32 alloc {
^bb0(%arg0: f32):
omp.yield(%arg0 : f32)
}
21 changes: 21 additions & 0 deletions mlir/test/Dialect/OpenMP/roundtrip.mlir
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)
}