[mlir][acc] Add support for data clause modifiers #144806
Merged
Conversation
This file contains hidden or 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
The OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made: - Ensure that modifier builders which do not take modifier are still available. - All data clause enum values are left in place until a complete transition is made to the new modifiers.
|
@llvm/pr-subscribers-mlir-openacc Author: Razvan Lupusoru (razvanlupusoru) ChangesThe OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made:
Patch is 20.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144806.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 34312655115a1..8cbdf710cfa6e 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -199,6 +199,41 @@ def OpenACC_DataClauseEnum : I64EnumAttr<"DataClause",
def OpenACC_DataClauseAttr : EnumAttr<OpenACC_Dialect, OpenACC_DataClauseEnum,
"data_clause">;
+// Data clause modifiers:
+// * readonly: Added in OpenACC 2.7 to copyin and cache.
+// * zero: Added in OpenACC 3.0 for create and copyout.
+// * always, alwaysin, alwaysout: Added in OpenACC 3.4 for
+// copy, copyin, and copyout clauses.
+// * capture: Added in OpenACC 3.4 for copy, copyin, copyout and create clauses.
+def OpenACC_DataClauseModifierNone : I32BitEnumAttrCaseNone<"none">;
+// All of the modifiers below are bit flags - so the value noted is `1 << bit`.
+// Thus the `zero` modifier is `1 << 0` = 1, `readonly` is `1 << 1` = 2, etc.
+def OpenACC_DataClauseModifierZero : I32BitEnumAttrCaseBit<"zero", 0>;
+def OpenACC_DataClauseModifierReadonly : I32BitEnumAttrCaseBit<"readonly", 1>;
+def OpenACC_DataClauseModifierAlwaysIn : I32BitEnumAttrCaseBit<"alwaysin", 2>;
+def OpenACC_DataClauseModifierAlwaysOut : I32BitEnumAttrCaseBit<"alwaysout", 3>;
+def OpenACC_DataClauseModifierAlways : I32BitEnumAttrCaseGroup<"always",
+ [OpenACC_DataClauseModifierAlwaysIn, OpenACC_DataClauseModifierAlwaysOut]>;
+def OpenACC_DataClauseModifierCapture : I32BitEnumAttrCaseBit<"capture", 4>;
+
+def OpenACC_DataClauseModifierEnum : I32BitEnumAttr<
+ "DataClauseModifier",
+ "Captures data clause modifiers",
+ [
+ OpenACC_DataClauseModifierNone, OpenACC_DataClauseModifierZero,
+ OpenACC_DataClauseModifierReadonly, OpenACC_DataClauseModifierAlwaysIn,
+ OpenACC_DataClauseModifierAlwaysOut, OpenACC_DataClauseModifierAlways,
+ OpenACC_DataClauseModifierCapture]> {
+ let separator = ",";
+ let cppNamespace = "::mlir::acc";
+ let genSpecializedAttr = 0;
+ let printBitEnumPrimaryGroups = 1;
+}
+
+def OpenACC_DataClauseModifierAttr : EnumAttr<OpenACC_Dialect,
+ OpenACC_DataClauseModifierEnum,
+ "data_clause_modifier">;
+
class OpenACC_Attr<string name, string attrMnemonic,
list<Trait> traits = [],
string baseCppClass = "::mlir::Attribute">
@@ -477,6 +512,8 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
DefaultValuedAttr<OpenACC_DataClauseAttr, clause>:$dataClause,
DefaultValuedAttr<BoolAttr, "true">:$structured,
DefaultValuedAttr<BoolAttr, "false">:$implicit,
+ DefaultValuedAttr<OpenACC_DataClauseModifierAttr,
+ "mlir::acc::DataClauseModifier::none">:$modifiers,
OptionalAttr<StrAttr>:$name));
let description = !strconcat(extraDescription, [{
@@ -506,6 +543,7 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
counters (2.6.7).
- `implicit`: Whether this is an implicitly generated operation, such as copies
done to satisfy "Variables with Implicitly Determined Data Attributes" in 2.6.2.
+ - `modifiers`: Keeps track of the data clause modifiers (eg zero, readonly, etc)
- `name`: Holds the name of variable as specified in user clause (including bounds).
The async values attached to the data entry operation imply that the data
@@ -584,7 +622,8 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
/*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$var,
"bool":$structured, "bool":$implicit,
@@ -601,9 +640,23 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
/*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>];
+ }]>,
+ OpBuilder<(ins "::mlir::Type":$accVarType, "::mlir::Value":$var,
+ "::mlir::Type":$varType, "::mlir::Value":$varPtrPtr,
+ "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVarType, var, varType, varPtrPtr, bounds,
+ asyncOperands, asyncOperandsDeviceType, asyncOnly, dataClause,
+ structured, implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
+ ];
}
//===----------------------------------------------------------------------===//
@@ -817,9 +870,7 @@ def OpenACC_CacheOp : OpenACC_DataEntryOp<"cache",
let extraClassDeclaration = extraClassDeclarationBase # [{
/// Check if this is a cache with readonly modifier.
- bool isCacheReadonly() {
- return getDataClause() == acc::DataClause::acc_cache_readonly;
- }
+ bool isCacheReadonly();
}];
}
@@ -840,6 +891,8 @@ class OpenACC_DataExitOp<string mnemonic, string clause, string extraDescription
DefaultValuedAttr<OpenACC_DataClauseAttr,clause>:$dataClause,
DefaultValuedAttr<BoolAttr, "true">:$structured,
DefaultValuedAttr<BoolAttr, "false">:$implicit,
+ DefaultValuedAttr<OpenACC_DataClauseModifierAttr,
+ "mlir::acc::DataClauseModifier::none">:$modifiers,
OptionalAttr<StrAttr>:$name));
let description = !strconcat(extraDescription, [{
@@ -861,6 +914,7 @@ class OpenACC_DataExitOp<string mnemonic, string clause, string extraDescription
counters (2.6.7).
- `implicit`: Whether this is an implicitly generated operation, such as copies
done to satisfy "Variables with Implicitly Determined Data Attributes" in 2.6.2.
+ - `modifiers`: Keeps track of the data clause modifiers (eg zero, always, etc)
- `name`: Holds the name of variable as specified in user clause (including bounds).
The async values attached to the data exit operation imply that the data
@@ -944,7 +998,8 @@ class OpenACC_DataExitOpWithVarPtr<string mnemonic, string clause>
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$accVar,
"::mlir::Value":$var,
@@ -961,9 +1016,22 @@ class OpenACC_DataExitOpWithVarPtr<string mnemonic, string clause>
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>];
+ }]>,
+ OpBuilder<(ins "::mlir::Value":$accVar, "::mlir::Value":$var,
+ "::mlir::Type":$varType, "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVar, var, varType, bounds,
+ asyncOperands, asyncOperandsDeviceType, asyncOnly, dataClause,
+ structured, implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
+ ];
code extraClassDeclarationDataExit = [{
mlir::TypedValue<mlir::acc::PointerLikeType> getVarPtr() {
@@ -998,7 +1066,8 @@ class OpenACC_DataExitOpNoVarPtr<string mnemonic, string clause> :
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$accVar,
"bool":$structured, "bool":$implicit,
@@ -1009,9 +1078,20 @@ class OpenACC_DataExitOpNoVarPtr<string mnemonic, string clause> :
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>
+ }]>,
+ OpBuilder<(ins "::mlir::Value":$accVar, "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVar, bounds, asyncOperands,
+ asyncOperandsDeviceType, asyncOnly, dataClause, structured,
+ implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
];
code extraClassDeclarationDataExit = [{
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 0dfead98b7e73..dd56a4108991d 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -321,6 +321,24 @@ static LogicalResult checkVarAndAccVar(Op op) {
return success();
}
+template <typename Op>
+static LogicalResult checkNoModifier(Op op) {
+ if (op.getModifiers() != acc::DataClauseModifier::none)
+ return op.emitError("no data clause modifiers are allowed");
+ return success();
+}
+
+template <typename Op>
+static LogicalResult
+checkValidModifier(Op op, acc::DataClauseModifier validModifiers) {
+ if (acc::bitEnumContainsAny(op.getModifiers(), ~validModifiers))
+ return op.emitError(
+ "invalid data clause modifiers: " +
+ acc::stringifyDataClauseModifier(op.getModifiers() & ~validModifiers));
+
+ return success();
+}
+
static ParseResult parseVar(mlir::OpAsmParser &parser,
OpAsmParser::UnresolvedOperand &var) {
// Either `var` or `varPtr` keyword is required.
@@ -447,6 +465,8 @@ LogicalResult acc::PrivateOp::verify() {
"data clause associated with private operation must match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -459,6 +479,8 @@ LogicalResult acc::FirstprivateOp::verify() {
"match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -471,6 +493,8 @@ LogicalResult acc::ReductionOp::verify() {
"match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -485,6 +509,8 @@ LogicalResult acc::DevicePtrOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -499,6 +525,8 @@ LogicalResult acc::PresentOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -518,11 +546,17 @@ LogicalResult acc::CopyinOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::readonly |
+ acc::DataClauseModifier::always |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CopyinOp::isCopyinReadonly() {
- return getDataClause() == acc::DataClause::acc_copyin_readonly;
+ return getDataClause() == acc::DataClause::acc_copyin_readonly ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::readonly);
}
//===----------------------------------------------------------------------===//
@@ -541,13 +575,19 @@ LogicalResult acc::CreateOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::alwaysout |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CreateOp::isCreateZero() {
// The zero modifier is encoded in the data clause.
return getDataClause() == acc::DataClause::acc_create_zero ||
- getDataClause() == acc::DataClause::acc_copyout_zero;
+ getDataClause() == acc::DataClause::acc_copyout_zero ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::zero);
}
//===----------------------------------------------------------------------===//
@@ -561,6 +601,8 @@ LogicalResult acc::NoCreateOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -575,6 +617,8 @@ LogicalResult acc::AttachOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -590,6 +634,8 @@ LogicalResult acc::DeclareDeviceResidentOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -605,6 +651,8 @@ LogicalResult acc::DeclareLinkOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -626,11 +674,16 @@ LogicalResult acc::CopyoutOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::always |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CopyoutOp::isCopyoutZero() {
- return getDataClause() == acc::DataClause::acc_copyout_zero;
+ return getDataClause() == acc::DataClause::acc_copyout_zero ||
+ acc::bitEnumContainsAny(getModifiers(), acc::DataClauseModifier::zero);
}
//===----------------------------------------------------------------------===//
@@ -652,6 +705,13 @@ LogicalResult acc::DeleteOp::verify() {
" or specify original clause this operation was decomposed from");
if (!getAccVar())
return emitError("must have device pointer");
+ // This op is the exit part of copyin and create - thus allow all modifiers
+ // allowed on either case.
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::readonly |
+ acc::DataClauseModifier::alwaysin |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
@@ -667,6 +727,8 @@ LogicalResult acc::DetachOp::verify() {
" or specify original clause this operation was decomposed from");
if (!getAccVar())
return emitError("must have device pointer");
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -686,6 +748,8 @@ LogicalResult acc::UpdateHostOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -702,6 +766,8 @@ LogicalResult acc::UpdateDeviceOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -718,6 +784,8 @@ LogicalResult acc::UseDeviceOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -735,9 +803,17 @@ LogicalResult acc::CacheOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::readonly)))
+ return failure();
return success();
}
+bool acc::CacheOp::isCacheReadonly() {
+ return getDataClause() == acc::DataClause::acc_cache_readonly ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::readonly);
+}
+
template <typename StructureOp>
static ParseResult parseRegions(OpAsmParser &parser, OperationState &state,
unsigned nRegions = 1) {
diff --git a/mlir/test/Dialect/OpenACC/invalid.mlir b/mlir/test/Dialect/OpenACC/invalid.mlir
index 8f6e961a06163..d85ad2ff80d80 100644
--- a/mlir/test/Dialect/OpenACC/invalid.mlir
+++ b/mlir/test/Dialect/OpenACC/invalid.mlir
@@ -819,3 +819,15 @@ func.func @acc_loop_container() {
} attributes { collapse = [3], collapseDeviceType = [#acc.device_type<none>], independent = [#acc.device_type<none>]}
return
}
+
+// -----
+
+%value = memref.alloc() : memref<f32>
+// expected-error @below {{no data clause modifiers are allowed}}
+%0 = acc.private varPtr(%value : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier zero>}
+
+// -----
+
+%value = memref.alloc() : memref<f32>
+// expected-error @below {{invalid data clause modifiers: alwaysin}}
+%0 = acc.create varPtr(%value : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier zero,capture,always>}
diff --git a/mlir/test/Dialect/OpenACC/ops.mlir b/mlir/test/Dialect/OpenACC/ops.mlir
index 97278f869534b..c1d8276d904bb 100644
--- a/mlir/test/Dialect/OpenACC/ops.mlir
+++ b/mlir/test/Dialect/OpenACC/ops.mlir
@@ -921,6 +921,21 @@ func.func @testdataop(%a: memref<f32>, %b: memref<f32>, %c: memref<f32>) -> () {
// -----
+func.func @testdataopmodifiers(%a: memref<f32>, %b: memref<f32>, %c: memref<f32>) -> () {
+ %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier capture,zero>}
+ %1 = acc.copyin varPtr(%b : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier readonly,capture,always>}
+ acc.data dataOperands(%0, %1 : memref<f32>, memref<f32>) {
+ }
+ acc.copyout accPtr(%0 : memref<f32>) to varPtr(%a : memref<f32...
[truncated]
|
|
@llvm/pr-subscribers-openacc Author: Razvan Lupusoru (razvanlupusoru) ChangesThe OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made:
Patch is 20.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144806.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 34312655115a1..8cbdf710cfa6e 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -199,6 +199,41 @@ def OpenACC_DataClauseEnum : I64EnumAttr<"DataClause",
def OpenACC_DataClauseAttr : EnumAttr<OpenACC_Dialect, OpenACC_DataClauseEnum,
"data_clause">;
+// Data clause modifiers:
+// * readonly: Added in OpenACC 2.7 to copyin and cache.
+// * zero: Added in OpenACC 3.0 for create and copyout.
+// * always, alwaysin, alwaysout: Added in OpenACC 3.4 for
+// copy, copyin, and copyout clauses.
+// * capture: Added in OpenACC 3.4 for copy, copyin, copyout and create clauses.
+def OpenACC_DataClauseModifierNone : I32BitEnumAttrCaseNone<"none">;
+// All of the modifiers below are bit flags - so the value noted is `1 << bit`.
+// Thus the `zero` modifier is `1 << 0` = 1, `readonly` is `1 << 1` = 2, etc.
+def OpenACC_DataClauseModifierZero : I32BitEnumAttrCaseBit<"zero", 0>;
+def OpenACC_DataClauseModifierReadonly : I32BitEnumAttrCaseBit<"readonly", 1>;
+def OpenACC_DataClauseModifierAlwaysIn : I32BitEnumAttrCaseBit<"alwaysin", 2>;
+def OpenACC_DataClauseModifierAlwaysOut : I32BitEnumAttrCaseBit<"alwaysout", 3>;
+def OpenACC_DataClauseModifierAlways : I32BitEnumAttrCaseGroup<"always",
+ [OpenACC_DataClauseModifierAlwaysIn, OpenACC_DataClauseModifierAlwaysOut]>;
+def OpenACC_DataClauseModifierCapture : I32BitEnumAttrCaseBit<"capture", 4>;
+
+def OpenACC_DataClauseModifierEnum : I32BitEnumAttr<
+ "DataClauseModifier",
+ "Captures data clause modifiers",
+ [
+ OpenACC_DataClauseModifierNone, OpenACC_DataClauseModifierZero,
+ OpenACC_DataClauseModifierReadonly, OpenACC_DataClauseModifierAlwaysIn,
+ OpenACC_DataClauseModifierAlwaysOut, OpenACC_DataClauseModifierAlways,
+ OpenACC_DataClauseModifierCapture]> {
+ let separator = ",";
+ let cppNamespace = "::mlir::acc";
+ let genSpecializedAttr = 0;
+ let printBitEnumPrimaryGroups = 1;
+}
+
+def OpenACC_DataClauseModifierAttr : EnumAttr<OpenACC_Dialect,
+ OpenACC_DataClauseModifierEnum,
+ "data_clause_modifier">;
+
class OpenACC_Attr<string name, string attrMnemonic,
list<Trait> traits = [],
string baseCppClass = "::mlir::Attribute">
@@ -477,6 +512,8 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
DefaultValuedAttr<OpenACC_DataClauseAttr, clause>:$dataClause,
DefaultValuedAttr<BoolAttr, "true">:$structured,
DefaultValuedAttr<BoolAttr, "false">:$implicit,
+ DefaultValuedAttr<OpenACC_DataClauseModifierAttr,
+ "mlir::acc::DataClauseModifier::none">:$modifiers,
OptionalAttr<StrAttr>:$name));
let description = !strconcat(extraDescription, [{
@@ -506,6 +543,7 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
counters (2.6.7).
- `implicit`: Whether this is an implicitly generated operation, such as copies
done to satisfy "Variables with Implicitly Determined Data Attributes" in 2.6.2.
+ - `modifiers`: Keeps track of the data clause modifiers (eg zero, readonly, etc)
- `name`: Holds the name of variable as specified in user clause (including bounds).
The async values attached to the data entry operation imply that the data
@@ -584,7 +622,8 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
/*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$var,
"bool":$structured, "bool":$implicit,
@@ -601,9 +640,23 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
/*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>];
+ }]>,
+ OpBuilder<(ins "::mlir::Type":$accVarType, "::mlir::Value":$var,
+ "::mlir::Type":$varType, "::mlir::Value":$varPtrPtr,
+ "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVarType, var, varType, varPtrPtr, bounds,
+ asyncOperands, asyncOperandsDeviceType, asyncOnly, dataClause,
+ structured, implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
+ ];
}
//===----------------------------------------------------------------------===//
@@ -817,9 +870,7 @@ def OpenACC_CacheOp : OpenACC_DataEntryOp<"cache",
let extraClassDeclaration = extraClassDeclarationBase # [{
/// Check if this is a cache with readonly modifier.
- bool isCacheReadonly() {
- return getDataClause() == acc::DataClause::acc_cache_readonly;
- }
+ bool isCacheReadonly();
}];
}
@@ -840,6 +891,8 @@ class OpenACC_DataExitOp<string mnemonic, string clause, string extraDescription
DefaultValuedAttr<OpenACC_DataClauseAttr,clause>:$dataClause,
DefaultValuedAttr<BoolAttr, "true">:$structured,
DefaultValuedAttr<BoolAttr, "false">:$implicit,
+ DefaultValuedAttr<OpenACC_DataClauseModifierAttr,
+ "mlir::acc::DataClauseModifier::none">:$modifiers,
OptionalAttr<StrAttr>:$name));
let description = !strconcat(extraDescription, [{
@@ -861,6 +914,7 @@ class OpenACC_DataExitOp<string mnemonic, string clause, string extraDescription
counters (2.6.7).
- `implicit`: Whether this is an implicitly generated operation, such as copies
done to satisfy "Variables with Implicitly Determined Data Attributes" in 2.6.2.
+ - `modifiers`: Keeps track of the data clause modifiers (eg zero, always, etc)
- `name`: Holds the name of variable as specified in user clause (including bounds).
The async values attached to the data exit operation imply that the data
@@ -944,7 +998,8 @@ class OpenACC_DataExitOpWithVarPtr<string mnemonic, string clause>
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$accVar,
"::mlir::Value":$var,
@@ -961,9 +1016,22 @@ class OpenACC_DataExitOpWithVarPtr<string mnemonic, string clause>
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>];
+ }]>,
+ OpBuilder<(ins "::mlir::Value":$accVar, "::mlir::Value":$var,
+ "::mlir::Type":$varType, "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVar, var, varType, bounds,
+ asyncOperands, asyncOperandsDeviceType, asyncOnly, dataClause,
+ structured, implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
+ ];
code extraClassDeclarationDataExit = [{
mlir::TypedValue<mlir::acc::PointerLikeType> getVarPtr() {
@@ -998,7 +1066,8 @@ class OpenACC_DataExitOpNoVarPtr<string mnemonic, string clause> :
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$accVar,
"bool":$structured, "bool":$implicit,
@@ -1009,9 +1078,20 @@ class OpenACC_DataExitOpNoVarPtr<string mnemonic, string clause> :
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>
+ }]>,
+ OpBuilder<(ins "::mlir::Value":$accVar, "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVar, bounds, asyncOperands,
+ asyncOperandsDeviceType, asyncOnly, dataClause, structured,
+ implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
];
code extraClassDeclarationDataExit = [{
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 0dfead98b7e73..dd56a4108991d 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -321,6 +321,24 @@ static LogicalResult checkVarAndAccVar(Op op) {
return success();
}
+template <typename Op>
+static LogicalResult checkNoModifier(Op op) {
+ if (op.getModifiers() != acc::DataClauseModifier::none)
+ return op.emitError("no data clause modifiers are allowed");
+ return success();
+}
+
+template <typename Op>
+static LogicalResult
+checkValidModifier(Op op, acc::DataClauseModifier validModifiers) {
+ if (acc::bitEnumContainsAny(op.getModifiers(), ~validModifiers))
+ return op.emitError(
+ "invalid data clause modifiers: " +
+ acc::stringifyDataClauseModifier(op.getModifiers() & ~validModifiers));
+
+ return success();
+}
+
static ParseResult parseVar(mlir::OpAsmParser &parser,
OpAsmParser::UnresolvedOperand &var) {
// Either `var` or `varPtr` keyword is required.
@@ -447,6 +465,8 @@ LogicalResult acc::PrivateOp::verify() {
"data clause associated with private operation must match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -459,6 +479,8 @@ LogicalResult acc::FirstprivateOp::verify() {
"match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -471,6 +493,8 @@ LogicalResult acc::ReductionOp::verify() {
"match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -485,6 +509,8 @@ LogicalResult acc::DevicePtrOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -499,6 +525,8 @@ LogicalResult acc::PresentOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -518,11 +546,17 @@ LogicalResult acc::CopyinOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::readonly |
+ acc::DataClauseModifier::always |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CopyinOp::isCopyinReadonly() {
- return getDataClause() == acc::DataClause::acc_copyin_readonly;
+ return getDataClause() == acc::DataClause::acc_copyin_readonly ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::readonly);
}
//===----------------------------------------------------------------------===//
@@ -541,13 +575,19 @@ LogicalResult acc::CreateOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::alwaysout |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CreateOp::isCreateZero() {
// The zero modifier is encoded in the data clause.
return getDataClause() == acc::DataClause::acc_create_zero ||
- getDataClause() == acc::DataClause::acc_copyout_zero;
+ getDataClause() == acc::DataClause::acc_copyout_zero ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::zero);
}
//===----------------------------------------------------------------------===//
@@ -561,6 +601,8 @@ LogicalResult acc::NoCreateOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -575,6 +617,8 @@ LogicalResult acc::AttachOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -590,6 +634,8 @@ LogicalResult acc::DeclareDeviceResidentOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -605,6 +651,8 @@ LogicalResult acc::DeclareLinkOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -626,11 +674,16 @@ LogicalResult acc::CopyoutOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::always |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CopyoutOp::isCopyoutZero() {
- return getDataClause() == acc::DataClause::acc_copyout_zero;
+ return getDataClause() == acc::DataClause::acc_copyout_zero ||
+ acc::bitEnumContainsAny(getModifiers(), acc::DataClauseModifier::zero);
}
//===----------------------------------------------------------------------===//
@@ -652,6 +705,13 @@ LogicalResult acc::DeleteOp::verify() {
" or specify original clause this operation was decomposed from");
if (!getAccVar())
return emitError("must have device pointer");
+ // This op is the exit part of copyin and create - thus allow all modifiers
+ // allowed on either case.
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::readonly |
+ acc::DataClauseModifier::alwaysin |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
@@ -667,6 +727,8 @@ LogicalResult acc::DetachOp::verify() {
" or specify original clause this operation was decomposed from");
if (!getAccVar())
return emitError("must have device pointer");
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -686,6 +748,8 @@ LogicalResult acc::UpdateHostOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -702,6 +766,8 @@ LogicalResult acc::UpdateDeviceOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -718,6 +784,8 @@ LogicalResult acc::UseDeviceOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -735,9 +803,17 @@ LogicalResult acc::CacheOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::readonly)))
+ return failure();
return success();
}
+bool acc::CacheOp::isCacheReadonly() {
+ return getDataClause() == acc::DataClause::acc_cache_readonly ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::readonly);
+}
+
template <typename StructureOp>
static ParseResult parseRegions(OpAsmParser &parser, OperationState &state,
unsigned nRegions = 1) {
diff --git a/mlir/test/Dialect/OpenACC/invalid.mlir b/mlir/test/Dialect/OpenACC/invalid.mlir
index 8f6e961a06163..d85ad2ff80d80 100644
--- a/mlir/test/Dialect/OpenACC/invalid.mlir
+++ b/mlir/test/Dialect/OpenACC/invalid.mlir
@@ -819,3 +819,15 @@ func.func @acc_loop_container() {
} attributes { collapse = [3], collapseDeviceType = [#acc.device_type<none>], independent = [#acc.device_type<none>]}
return
}
+
+// -----
+
+%value = memref.alloc() : memref<f32>
+// expected-error @below {{no data clause modifiers are allowed}}
+%0 = acc.private varPtr(%value : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier zero>}
+
+// -----
+
+%value = memref.alloc() : memref<f32>
+// expected-error @below {{invalid data clause modifiers: alwaysin}}
+%0 = acc.create varPtr(%value : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier zero,capture,always>}
diff --git a/mlir/test/Dialect/OpenACC/ops.mlir b/mlir/test/Dialect/OpenACC/ops.mlir
index 97278f869534b..c1d8276d904bb 100644
--- a/mlir/test/Dialect/OpenACC/ops.mlir
+++ b/mlir/test/Dialect/OpenACC/ops.mlir
@@ -921,6 +921,21 @@ func.func @testdataop(%a: memref<f32>, %b: memref<f32>, %c: memref<f32>) -> () {
// -----
+func.func @testdataopmodifiers(%a: memref<f32>, %b: memref<f32>, %c: memref<f32>) -> () {
+ %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier capture,zero>}
+ %1 = acc.copyin varPtr(%b : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier readonly,capture,always>}
+ acc.data dataOperands(%0, %1 : memref<f32>, memref<f32>) {
+ }
+ acc.copyout accPtr(%0 : memref<f32>) to varPtr(%a : memref<f32...
[truncated]
|
|
@llvm/pr-subscribers-mlir Author: Razvan Lupusoru (razvanlupusoru) ChangesThe OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made:
Patch is 20.82 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144806.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 34312655115a1..8cbdf710cfa6e 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -199,6 +199,41 @@ def OpenACC_DataClauseEnum : I64EnumAttr<"DataClause",
def OpenACC_DataClauseAttr : EnumAttr<OpenACC_Dialect, OpenACC_DataClauseEnum,
"data_clause">;
+// Data clause modifiers:
+// * readonly: Added in OpenACC 2.7 to copyin and cache.
+// * zero: Added in OpenACC 3.0 for create and copyout.
+// * always, alwaysin, alwaysout: Added in OpenACC 3.4 for
+// copy, copyin, and copyout clauses.
+// * capture: Added in OpenACC 3.4 for copy, copyin, copyout and create clauses.
+def OpenACC_DataClauseModifierNone : I32BitEnumAttrCaseNone<"none">;
+// All of the modifiers below are bit flags - so the value noted is `1 << bit`.
+// Thus the `zero` modifier is `1 << 0` = 1, `readonly` is `1 << 1` = 2, etc.
+def OpenACC_DataClauseModifierZero : I32BitEnumAttrCaseBit<"zero", 0>;
+def OpenACC_DataClauseModifierReadonly : I32BitEnumAttrCaseBit<"readonly", 1>;
+def OpenACC_DataClauseModifierAlwaysIn : I32BitEnumAttrCaseBit<"alwaysin", 2>;
+def OpenACC_DataClauseModifierAlwaysOut : I32BitEnumAttrCaseBit<"alwaysout", 3>;
+def OpenACC_DataClauseModifierAlways : I32BitEnumAttrCaseGroup<"always",
+ [OpenACC_DataClauseModifierAlwaysIn, OpenACC_DataClauseModifierAlwaysOut]>;
+def OpenACC_DataClauseModifierCapture : I32BitEnumAttrCaseBit<"capture", 4>;
+
+def OpenACC_DataClauseModifierEnum : I32BitEnumAttr<
+ "DataClauseModifier",
+ "Captures data clause modifiers",
+ [
+ OpenACC_DataClauseModifierNone, OpenACC_DataClauseModifierZero,
+ OpenACC_DataClauseModifierReadonly, OpenACC_DataClauseModifierAlwaysIn,
+ OpenACC_DataClauseModifierAlwaysOut, OpenACC_DataClauseModifierAlways,
+ OpenACC_DataClauseModifierCapture]> {
+ let separator = ",";
+ let cppNamespace = "::mlir::acc";
+ let genSpecializedAttr = 0;
+ let printBitEnumPrimaryGroups = 1;
+}
+
+def OpenACC_DataClauseModifierAttr : EnumAttr<OpenACC_Dialect,
+ OpenACC_DataClauseModifierEnum,
+ "data_clause_modifier">;
+
class OpenACC_Attr<string name, string attrMnemonic,
list<Trait> traits = [],
string baseCppClass = "::mlir::Attribute">
@@ -477,6 +512,8 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
DefaultValuedAttr<OpenACC_DataClauseAttr, clause>:$dataClause,
DefaultValuedAttr<BoolAttr, "true">:$structured,
DefaultValuedAttr<BoolAttr, "false">:$implicit,
+ DefaultValuedAttr<OpenACC_DataClauseModifierAttr,
+ "mlir::acc::DataClauseModifier::none">:$modifiers,
OptionalAttr<StrAttr>:$name));
let description = !strconcat(extraDescription, [{
@@ -506,6 +543,7 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
counters (2.6.7).
- `implicit`: Whether this is an implicitly generated operation, such as copies
done to satisfy "Variables with Implicitly Determined Data Attributes" in 2.6.2.
+ - `modifiers`: Keeps track of the data clause modifiers (eg zero, readonly, etc)
- `name`: Holds the name of variable as specified in user clause (including bounds).
The async values attached to the data entry operation imply that the data
@@ -584,7 +622,8 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
/*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$var,
"bool":$structured, "bool":$implicit,
@@ -601,9 +640,23 @@ class OpenACC_DataEntryOp<string mnemonic, string clause, string extraDescriptio
/*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>];
+ }]>,
+ OpBuilder<(ins "::mlir::Type":$accVarType, "::mlir::Value":$var,
+ "::mlir::Type":$varType, "::mlir::Value":$varPtrPtr,
+ "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVarType, var, varType, varPtrPtr, bounds,
+ asyncOperands, asyncOperandsDeviceType, asyncOnly, dataClause,
+ structured, implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
+ ];
}
//===----------------------------------------------------------------------===//
@@ -817,9 +870,7 @@ def OpenACC_CacheOp : OpenACC_DataEntryOp<"cache",
let extraClassDeclaration = extraClassDeclarationBase # [{
/// Check if this is a cache with readonly modifier.
- bool isCacheReadonly() {
- return getDataClause() == acc::DataClause::acc_cache_readonly;
- }
+ bool isCacheReadonly();
}];
}
@@ -840,6 +891,8 @@ class OpenACC_DataExitOp<string mnemonic, string clause, string extraDescription
DefaultValuedAttr<OpenACC_DataClauseAttr,clause>:$dataClause,
DefaultValuedAttr<BoolAttr, "true">:$structured,
DefaultValuedAttr<BoolAttr, "false">:$implicit,
+ DefaultValuedAttr<OpenACC_DataClauseModifierAttr,
+ "mlir::acc::DataClauseModifier::none">:$modifiers,
OptionalAttr<StrAttr>:$name));
let description = !strconcat(extraDescription, [{
@@ -861,6 +914,7 @@ class OpenACC_DataExitOp<string mnemonic, string clause, string extraDescription
counters (2.6.7).
- `implicit`: Whether this is an implicitly generated operation, such as copies
done to satisfy "Variables with Implicitly Determined Data Attributes" in 2.6.2.
+ - `modifiers`: Keeps track of the data clause modifiers (eg zero, always, etc)
- `name`: Holds the name of variable as specified in user clause (including bounds).
The async values attached to the data exit operation imply that the data
@@ -944,7 +998,8 @@ class OpenACC_DataExitOpWithVarPtr<string mnemonic, string clause>
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$accVar,
"::mlir::Value":$var,
@@ -961,9 +1016,22 @@ class OpenACC_DataExitOpWithVarPtr<string mnemonic, string clause>
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>];
+ }]>,
+ OpBuilder<(ins "::mlir::Value":$accVar, "::mlir::Value":$var,
+ "::mlir::Type":$varType, "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVar, var, varType, bounds,
+ asyncOperands, asyncOperandsDeviceType, asyncOnly, dataClause,
+ structured, implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
+ ];
code extraClassDeclarationDataExit = [{
mlir::TypedValue<mlir::acc::PointerLikeType> getVarPtr() {
@@ -998,7 +1066,8 @@ class OpenACC_DataExitOpNoVarPtr<string mnemonic, string clause> :
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit), /*name=*/nullptr);
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
+ /*name=*/nullptr);
}]>,
OpBuilder<(ins "::mlir::Value":$accVar,
"bool":$structured, "bool":$implicit,
@@ -1009,9 +1078,20 @@ class OpenACC_DataExitOpNoVarPtr<string mnemonic, string clause> :
bounds, /*asyncOperands=*/{}, /*asyncOperandsDeviceType=*/nullptr,
/*asyncOnly=*/nullptr, /*dataClause=*/nullptr,
/*structured=*/$_builder.getBoolAttr(structured),
- /*implicit=*/$_builder.getBoolAttr(implicit),
+ /*implicit=*/$_builder.getBoolAttr(implicit), /*modifiers=*/nullptr,
/*name=*/$_builder.getStringAttr(name));
- }]>
+ }]>,
+ OpBuilder<(ins "::mlir::Value":$accVar, "::mlir::ValueRange":$bounds,
+ "::mlir::ValueRange":$asyncOperands,
+ "::mlir::ArrayAttr":$asyncOperandsDeviceType,
+ "::mlir::ArrayAttr":$asyncOnly,
+ "::mlir::acc::DataClause":$dataClause, "bool":$structured,
+ "bool":$implicit, "::mlir::StringAttr":$name),
+ [{
+ build($_builder, $_state, accVar, bounds, asyncOperands,
+ asyncOperandsDeviceType, asyncOnly, dataClause, structured,
+ implicit, ::mlir::acc::DataClauseModifier::none, name);
+ }]>,
];
code extraClassDeclarationDataExit = [{
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 0dfead98b7e73..dd56a4108991d 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -321,6 +321,24 @@ static LogicalResult checkVarAndAccVar(Op op) {
return success();
}
+template <typename Op>
+static LogicalResult checkNoModifier(Op op) {
+ if (op.getModifiers() != acc::DataClauseModifier::none)
+ return op.emitError("no data clause modifiers are allowed");
+ return success();
+}
+
+template <typename Op>
+static LogicalResult
+checkValidModifier(Op op, acc::DataClauseModifier validModifiers) {
+ if (acc::bitEnumContainsAny(op.getModifiers(), ~validModifiers))
+ return op.emitError(
+ "invalid data clause modifiers: " +
+ acc::stringifyDataClauseModifier(op.getModifiers() & ~validModifiers));
+
+ return success();
+}
+
static ParseResult parseVar(mlir::OpAsmParser &parser,
OpAsmParser::UnresolvedOperand &var) {
// Either `var` or `varPtr` keyword is required.
@@ -447,6 +465,8 @@ LogicalResult acc::PrivateOp::verify() {
"data clause associated with private operation must match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -459,6 +479,8 @@ LogicalResult acc::FirstprivateOp::verify() {
"match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -471,6 +493,8 @@ LogicalResult acc::ReductionOp::verify() {
"match its intent");
if (failed(checkVarAndVarType(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -485,6 +509,8 @@ LogicalResult acc::DevicePtrOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -499,6 +525,8 @@ LogicalResult acc::PresentOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -518,11 +546,17 @@ LogicalResult acc::CopyinOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::readonly |
+ acc::DataClauseModifier::always |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CopyinOp::isCopyinReadonly() {
- return getDataClause() == acc::DataClause::acc_copyin_readonly;
+ return getDataClause() == acc::DataClause::acc_copyin_readonly ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::readonly);
}
//===----------------------------------------------------------------------===//
@@ -541,13 +575,19 @@ LogicalResult acc::CreateOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::alwaysout |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CreateOp::isCreateZero() {
// The zero modifier is encoded in the data clause.
return getDataClause() == acc::DataClause::acc_create_zero ||
- getDataClause() == acc::DataClause::acc_copyout_zero;
+ getDataClause() == acc::DataClause::acc_copyout_zero ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::zero);
}
//===----------------------------------------------------------------------===//
@@ -561,6 +601,8 @@ LogicalResult acc::NoCreateOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -575,6 +617,8 @@ LogicalResult acc::AttachOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -590,6 +634,8 @@ LogicalResult acc::DeclareDeviceResidentOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -605,6 +651,8 @@ LogicalResult acc::DeclareLinkOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -626,11 +674,16 @@ LogicalResult acc::CopyoutOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::always |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
bool acc::CopyoutOp::isCopyoutZero() {
- return getDataClause() == acc::DataClause::acc_copyout_zero;
+ return getDataClause() == acc::DataClause::acc_copyout_zero ||
+ acc::bitEnumContainsAny(getModifiers(), acc::DataClauseModifier::zero);
}
//===----------------------------------------------------------------------===//
@@ -652,6 +705,13 @@ LogicalResult acc::DeleteOp::verify() {
" or specify original clause this operation was decomposed from");
if (!getAccVar())
return emitError("must have device pointer");
+ // This op is the exit part of copyin and create - thus allow all modifiers
+ // allowed on either case.
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::zero |
+ acc::DataClauseModifier::readonly |
+ acc::DataClauseModifier::alwaysin |
+ acc::DataClauseModifier::capture)))
+ return failure();
return success();
}
@@ -667,6 +727,8 @@ LogicalResult acc::DetachOp::verify() {
" or specify original clause this operation was decomposed from");
if (!getAccVar())
return emitError("must have device pointer");
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -686,6 +748,8 @@ LogicalResult acc::UpdateHostOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -702,6 +766,8 @@ LogicalResult acc::UpdateDeviceOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -718,6 +784,8 @@ LogicalResult acc::UseDeviceOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkNoModifier(*this)))
+ return failure();
return success();
}
@@ -735,9 +803,17 @@ LogicalResult acc::CacheOp::verify() {
return failure();
if (failed(checkVarAndAccVar(*this)))
return failure();
+ if (failed(checkValidModifier(*this, acc::DataClauseModifier::readonly)))
+ return failure();
return success();
}
+bool acc::CacheOp::isCacheReadonly() {
+ return getDataClause() == acc::DataClause::acc_cache_readonly ||
+ acc::bitEnumContainsAny(getModifiers(),
+ acc::DataClauseModifier::readonly);
+}
+
template <typename StructureOp>
static ParseResult parseRegions(OpAsmParser &parser, OperationState &state,
unsigned nRegions = 1) {
diff --git a/mlir/test/Dialect/OpenACC/invalid.mlir b/mlir/test/Dialect/OpenACC/invalid.mlir
index 8f6e961a06163..d85ad2ff80d80 100644
--- a/mlir/test/Dialect/OpenACC/invalid.mlir
+++ b/mlir/test/Dialect/OpenACC/invalid.mlir
@@ -819,3 +819,15 @@ func.func @acc_loop_container() {
} attributes { collapse = [3], collapseDeviceType = [#acc.device_type<none>], independent = [#acc.device_type<none>]}
return
}
+
+// -----
+
+%value = memref.alloc() : memref<f32>
+// expected-error @below {{no data clause modifiers are allowed}}
+%0 = acc.private varPtr(%value : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier zero>}
+
+// -----
+
+%value = memref.alloc() : memref<f32>
+// expected-error @below {{invalid data clause modifiers: alwaysin}}
+%0 = acc.create varPtr(%value : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier zero,capture,always>}
diff --git a/mlir/test/Dialect/OpenACC/ops.mlir b/mlir/test/Dialect/OpenACC/ops.mlir
index 97278f869534b..c1d8276d904bb 100644
--- a/mlir/test/Dialect/OpenACC/ops.mlir
+++ b/mlir/test/Dialect/OpenACC/ops.mlir
@@ -921,6 +921,21 @@ func.func @testdataop(%a: memref<f32>, %b: memref<f32>, %c: memref<f32>) -> () {
// -----
+func.func @testdataopmodifiers(%a: memref<f32>, %b: memref<f32>, %c: memref<f32>) -> () {
+ %0 = acc.create varPtr(%a : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier capture,zero>}
+ %1 = acc.copyin varPtr(%b : memref<f32>) -> memref<f32> {modifiers = #acc<data_clause_modifier readonly,capture,always>}
+ acc.data dataOperands(%0, %1 : memref<f32>, memref<f32>) {
+ }
+ acc.copyout accPtr(%0 : memref<f32>) to varPtr(%a : memref<f32...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
erichkeane
reviewed
Jun 24, 2025
DrSergei
pushed a commit
to DrSergei/llvm-project
that referenced
this pull request
Jun 24, 2025
The OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made: - Ensure that modifier builders which do not take modifier are still available. - All data clause enum values are left in place until a complete transition is made to the new modifiers.
erichkeane
reviewed
Jun 25, 2025
| template <typename Op> | ||
| static LogicalResult | ||
| checkValidModifier(Op op, acc::DataClauseModifier validModifiers) { | ||
| if (acc::bitEnumContainsAny(op.getModifiers(), ~validModifiers)) |
There was a problem hiding this comment.
ONE thing I noticed, is that this doesn't check for anything but the list of modifiers. We MIGHT want to check that no bits > the largest one are set. Else we end up failing during printing if op.getModifiers is 1<<5.
There was a problem hiding this comment.
Here is the implementation of ~ from OpenACCOpsEnums.h.inc which gets generated from the .td file:
inline constexpr DataClauseModifier operator~(DataClauseModifier bits) {
// Ensure only bits that can be present in the enum are set
return static_cast<DataClauseModifier>(~static_cast<uint32_t>(bits) & static_cast<uint32_t>(31u));
}
Note that it includes a & 31 to avoid the problem you are describing.
Does this answer your concern or did I misunderstand your point here?
There was a problem hiding this comment.
Yeah, that is part of the problem. We shouldn't be silently accepting those bits IMO, we should be failing verification there. Else it gets caught by:
clang: /local/home/ekeane/llvm-project/build/tools/mlir/include/mlir/Dialect/OpenACC/OpenACCOpsEnums.cpp.inc:239: std::string mlir::acc::stringifyDataClauseModifier(DataClauseModifier): Assertion `31u == (31u | val) && "invalid bits set in bit enum"' failed.
I would expect to fail verification rather than asserting like this. In this case, I'd assigned 1<<5 to one of these (basically just as a 'part' of my implementation, before I got the always->alwaysin | alwaysout part done), and was surprised to see an assert rather than a verification failure.
Stack trace in case it is helpful.
#0 0x0000625529419ee1 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /local/home/ekeane/llvm-project/llvm/lib/Support/Unix/Signals.inc:804:11
#1 0x000062552941a46b PrintStackTraceSignalHandler(void*) /local/home/ekeane/llvm-project/llvm/lib/Support/Unix/Signals.inc:888:1
#2 0x00006255294183d6 llvm::sys::RunSignalHandlers() /local/home/ekeane/llvm-project/llvm/lib/Support/Signals.cpp:105:5
#3 0x000062552941ac2d SignalHandler(int, siginfo_t*, void*) /local/home/ekeane/llvm-project/llvm/lib/Support/Unix/Signals.inc:418:7
#4 0x00007f69d1045330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330)
#5 0x00007f69d109eb2c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
#6 0x00007f69d109eb2c __pthread_kill_internal ./nptl/pthread_kill.c:78:10
#7 0x00007f69d109eb2c pthread_kill ./nptl/pthread_kill.c:89:10
#8 0x00007f69d104527e raise ./signal/../sysdeps/posix/raise.c:27:6
#9 0x00007f69d10288ff abort ./stdlib/abort.c:81:7
#10 0x00007f69d102881b _nl_load_domain ./intl/loadmsgcat.c:1177:9
#11 0x00007f69d103b517 (/lib/x86_64-linux-gnu/libc.so.6+0x3b517)
#12 0x000062552b5a9428 mlir::acc::stringifyDataClauseModifier[abi:cxx11](mlir::acc::DataClauseModifier) /local/home/ekeane/llvm-project/build/tools/mlir/include/mlir/Dialect/OpenACC/OpenACCOpsEnums.cpp.inc:241:11
#13 0x000062552b6edf16 mlir::acc::DataClauseModifierAttr::print(mlir::AsmPrinter&) const /local/home/ekeane/llvm-project/build/tools/mlir/include/mlir/Dialect/OpenACC/OpenACCOpsAttributes.cpp.inc:420:17
#14 0x000062552b6fd939 _ZZL25generatedAttributePrinterN4mlir9AttributeERNS_10AsmPrinterEENK3$_3clINS_3acc22DataClauseModifierAttrEEEDaT_ /local/home/ekeane/llvm-project/build/tools/mlir/include/mlir/Dialect/OpenACC/OpenACCOpsAttributes.cpp.inc:100:14
#15 0x000062552b6fd25f llvm::TypeSwitch<mlir::Attribute, llvm::LogicalResult>& llvm::TypeSwitch<mlir::Attribute, llvm::LogicalResult>::Case<mlir::acc::DataClauseModifierAttr, generatedAttributePrinter(mlir::Attribute, mlir::AsmPrinter&)::$_3>(generatedAttributePrinter(mlir::Attribute, mlir::AsmPrinter&)::$_3&&) /local/home/ekeane/llvm-project/llvm/include/llvm/ADT/TypeSwitch.h:102:22
#16 0x000062552b6f1691 generatedAttributePrinter(mlir::Attribute, mlir::AsmPrinter&) /local/home/ekeane/llvm-project/build/tools/mlir/include/mlir/Dialect/OpenACC/OpenACCOpsAttributes.cpp.inc:97:6
#17 0x000062552b6f15f9 mlir::acc::OpenACCDialect::printAttribute(mlir::Attribute, mlir::DialectAsmPrinter&) const /local/home/ekeane/llvm-project/build/tools/mlir/include/mlir/Dialect/OpenACC/OpenACCOpsAttributes.cpp.inc:1131:25
erichkeane
added a commit
to erichkeane/llvm-project
that referenced
this pull request
Jun 25, 2025
Some of the 'data' clauses can have a 'modifier-list' which specifies one of a few keywords from a list. This patch adds support for lowering them following llvm#144806. We have to keep a separate enum from MLIR, since we have to keep 'always' around for semantic reasons, whereas the dialect doesn't differentiate these. This patch ensures we get these right for the only applicable clause so far, which is 'copy'.
erichkeane
added a commit
that referenced
this pull request
Jun 26, 2025
Some of the 'data' clauses can have a 'modifier-list' which specifies one of a few keywords from a list. This patch adds support for lowering them following #144806. We have to keep a separate enum from MLIR, since we have to keep 'always' around for semantic reasons, whereas the dialect doesn't differentiate these. This patch ensures we get these right for the only applicable clause so far, which is 'copy'.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
The OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made: - Ensure that modifier builders which do not take modifier are still available. - All data clause enum values are left in place until a complete transition is made to the new modifiers.
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
Some of the 'data' clauses can have a 'modifier-list' which specifies one of a few keywords from a list. This patch adds support for lowering them following llvm#144806. We have to keep a separate enum from MLIR, since we have to keep 'always' around for semantic reasons, whereas the dialect doesn't differentiate these. This patch ensures we get these right for the only applicable clause so far, which is 'copy'.
erichkeane
added a commit
to erichkeane/llvm-project
that referenced
this pull request
Jun 26, 2025
…ombined
This patch does the lowering of copyin (represented as a
acc.copyin/acc.delete), copyout (acc.create/acc.copyin), and create
(acc.create/acc.delete).
Additionally, it found a few problems with llvm#144806, so it fixes those as
well.
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
The OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations. In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made: - Ensure that modifier builders which do not take modifier are still available. - All data clause enum values are left in place until a complete transition is made to the new modifiers.
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
Some of the 'data' clauses can have a 'modifier-list' which specifies one of a few keywords from a list. This patch adds support for lowering them following llvm#144806. We have to keep a separate enum from MLIR, since we have to keep 'always' around for semantic reasons, whereas the dialect doesn't differentiate these. This patch ensures we get these right for the only applicable clause so far, which is 'copy'.
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
llvm#145976) …ombined This patch does the lowering of copyin (represented as a acc.copyin/acc.delete), copyout (acc.create/acc.copyin), and create (acc.create/acc.delete). Additionally, it found a few problems with llvm#144806, so it fixes those as well.
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.
The OpenACC data clause operations have been updated to support the OpenACC 3.4 data clause modifiers. This includes ensuring verifiers check that only supported ones are used on relevant operations.
In order to support a seamless update from encoding the modifiers in the data clause to this attribute, the following considerations were made: