[mlir][acc] Refactor and expand OpenACC utils #115119
Open
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
This MR accomplishes the following: - OpenACC utilities are moved from the dialect to their own library to be consistent with the way Utils are done in other dialects. - Adds matching setters for several of the utilities that already had getters. - Ensure that OpenACCUtils depends on OpenACCDialect (and not vice versa) by inlining some of the used utilities in the dialect. - Adds unit testing for the utilities
|
@llvm/pr-subscribers-mlir-openacc @llvm/pr-subscribers-openacc Author: Razvan Lupusoru (razvanlupusoru) ChangesThis MR accomplishes the following:
Patch is 49.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/115119.diff 10 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
index cda07d6a913649..4d399f2e0ed19c 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
@@ -83,59 +83,6 @@ namespace acc {
/// combined and the final mapping value would be 5 (4 | 1).
enum OpenACCExecMapping { NONE = 0, VECTOR = 1, WORKER = 2, GANG = 4 };
-/// Used to obtain the `varPtr` from a data clause operation.
-/// Returns empty value if not a data clause operation or is a data exit
-/// operation with no `varPtr`.
-mlir::Value getVarPtr(mlir::Operation *accDataClauseOp);
-
-/// Used to obtain the `accPtr` from a data clause operation.
-/// When a data entry operation, it obtains its result `accPtr` value.
-/// If a data exit operation, it obtains its operand `accPtr` value.
-/// Returns empty value if not a data clause operation.
-mlir::Value getAccPtr(mlir::Operation *accDataClauseOp);
-
-/// Used to obtain the `varPtrPtr` from a data clause operation.
-/// Returns empty value if not a data clause operation.
-mlir::Value getVarPtrPtr(mlir::Operation *accDataClauseOp);
-
-/// Used to obtain `bounds` from an acc data clause operation.
-/// Returns an empty vector if there are no bounds.
-mlir::SmallVector<mlir::Value> getBounds(mlir::Operation *accDataClauseOp);
-
-/// Used to obtain `async` operands from an acc data clause operation.
-/// Returns an empty vector if there are no such operands.
-mlir::SmallVector<mlir::Value>
-getAsyncOperands(mlir::Operation *accDataClauseOp);
-
-/// Returns an array of acc:DeviceTypeAttr attributes attached to
-/// an acc data clause operation, that correspond to the device types
-/// associated with the async clauses with an async-value.
-mlir::ArrayAttr getAsyncOperandsDeviceType(mlir::Operation *accDataClauseOp);
-
-/// Returns an array of acc:DeviceTypeAttr attributes attached to
-/// an acc data clause operation, that correspond to the device types
-/// associated with the async clauses without an async-value.
-mlir::ArrayAttr getAsyncOnly(mlir::Operation *accDataClauseOp);
-
-/// Used to obtain the `name` from an acc operation.
-std::optional<llvm::StringRef> getVarName(mlir::Operation *accOp);
-
-/// Used to obtain the `dataClause` from a data entry operation.
-/// Returns empty optional if not a data entry operation.
-std::optional<mlir::acc::DataClause>
-getDataClause(mlir::Operation *accDataEntryOp);
-
-/// Used to find out whether data operation is implicit.
-/// Returns false if not a data operation or if it is a data operation without
-/// implicit flag.
-bool getImplicitFlag(mlir::Operation *accDataEntryOp);
-
-/// Used to get an immutable range iterating over the data operands.
-mlir::ValueRange getDataOperands(mlir::Operation *accOp);
-
-/// Used to get a mutable range iterating over the data operands.
-mlir::MutableOperandRange getMutableDataOperands(mlir::Operation *accOp);
-
/// Used to obtain the attribute name for declare.
static constexpr StringLiteral getDeclareAttrName() {
return StringLiteral("acc.declare");
diff --git a/mlir/include/mlir/Dialect/OpenACC/Utils/OpenACCUtils.h b/mlir/include/mlir/Dialect/OpenACC/Utils/OpenACCUtils.h
new file mode 100644
index 00000000000000..e1d01aa2512c0c
--- /dev/null
+++ b/mlir/include/mlir/Dialect/OpenACC/Utils/OpenACCUtils.h
@@ -0,0 +1,117 @@
+//===- OpenACCUtils.h - OpenACC Utilities -----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_DIALECT_OPENACC_UTILS_OPENACCUTILS_H_
+#define MLIR_DIALECT_OPENACC_UTILS_OPENACCUTILS_H_
+
+#include "mlir/Dialect/OpenACC/OpenACC.h"
+
+namespace mlir {
+namespace acc {
+/// Used to obtain the `varPtr` from a data clause operation.
+/// Returns empty value if not a data clause operation or is a data exit
+/// operation with no `varPtr`.
+mlir::Value getVarPtr(mlir::Operation *accDataClauseOp);
+
+/// Used to set the `varPtr` of a data clause operation.
+/// Returns true if it was set successfully and false if this is not a data
+/// clause operation.
+bool setVarPtr(mlir::Operation *accDataClauseOp, mlir::Value varPtr);
+
+/// Used to obtain the `accPtr` from a data clause operation.
+/// When a data entry operation, it obtains its result `accPtr` value.
+/// If a data exit operation, it obtains its operand `accPtr` value.
+/// Returns empty value if not a data clause operation.
+mlir::Value getAccPtr(mlir::Operation *accDataClauseOp);
+
+/// Used to set the `accPtr` for a data exit operation.
+/// Returns true if it was set successfully and false if is not a data exit
+/// operation (data entry operations have their result as `accPtr` which
+/// cannot be changed).
+bool setAccPtr(mlir::Operation *accDataClauseOp, mlir::Value accPtr);
+
+/// Used to obtain the `varPtrPtr` from a data clause operation.
+/// Returns empty value if not a data clause operation.
+mlir::Value getVarPtrPtr(mlir::Operation *accDataClauseOp);
+
+/// Used to set the `varPtrPtr` for a data clause operation.
+/// Returns false if the operation does not have varPtrPtr or is not a data
+/// clause op.
+bool setVarPtrPtr(mlir::Operation *accDataClauseOp, mlir::Value varPtrPtr);
+
+/// Used to obtain `bounds` from an acc data clause operation.
+/// Returns an empty vector if there are no bounds.
+mlir::SmallVector<mlir::Value> getBounds(mlir::Operation *accDataClauseOp);
+
+/// Used to set `bounds` for an acc data clause operation. It completely
+/// replaces all bounds operands with the new list.
+/// Returns false if new bounds were not set (such as when argument is not
+/// an acc dat aclause operation).
+bool setBounds(mlir::Operation *accDataClauseOp,
+ mlir::SmallVector<mlir::Value> &bounds);
+bool setBounds(mlir::Operation *accDataClauseOp,
+ mlir::Value bound);
+
+/// Used to obtain the `dataClause` from a data clause operation.
+/// Returns empty optional if not a data operation.
+std::optional<mlir::acc::DataClause>
+getDataClause(mlir::Operation *accDataClauseOp);
+
+/// Used to set the `dataClause` on a data clause operation.
+/// Returns true if successfully set and false otherwise.
+bool setDataClause(mlir::Operation *accDataClauseOp,
+ mlir::acc::DataClause dataClause);
+
+/// Used to find out whether this data operation uses structured runtime
+/// counters. Returns false if not a data operation or if it is a data operation
+/// without the structured flag set.
+bool getStructuredFlag(mlir::Operation *accDataClauseOp);
+
+/// Used to update the data clause operation whether it represents structured
+/// or dynamic (value of `structured` is passed as false).
+/// Returns true if successfully set and false otherwise.
+bool setStructuredFlag(mlir::Operation *accDataClauseOp, bool structured);
+
+/// Used to find out whether data operation is implicit.
+/// Returns false if not a data operation or if it is a data operation without
+/// implicit flag.
+bool getImplicitFlag(mlir::Operation *accDataClauseOp);
+
+/// Used to update the data clause operation whether this operation is
+/// implicit or explicit (`implicit` set as false).
+/// Returns true if successfully set and false otherwise.
+bool setImplicitFlag(mlir::Operation *accDataClauseOp, bool implicit);
+
+/// Used to obtain the `name` from an acc operation.
+std::optional<llvm::StringRef> getVarName(mlir::Operation *accDataClauseOp);
+
+/// Used to obtain `async` operands from an acc data clause operation.
+/// Returns an empty vector if there are no such operands.
+mlir::SmallVector<mlir::Value>
+getAsyncOperands(mlir::Operation *accDataClauseOp);
+
+/// Returns an array of acc:DeviceTypeAttr attributes attached to
+/// an acc data clause operation, that correspond to the device types
+/// associated with the async clauses with an async-value.
+mlir::ArrayAttr getAsyncOperandsDeviceType(mlir::Operation *accDataClauseOp);
+
+/// Returns an array of acc:DeviceTypeAttr attributes attached to
+/// an acc data clause operation, that correspond to the device types
+/// associated with the async clauses without an async-value.
+mlir::ArrayAttr getAsyncOnly(mlir::Operation *accDataClauseOp);
+
+/// Used to get an immutable range iterating over the data operands.
+mlir::ValueRange getDataOperands(mlir::Operation *accOp);
+
+/// Used to get a mutable range iterating over the data operands.
+mlir::MutableOperandRange getMutableDataOperands(mlir::Operation *accOp);
+
+} // namespace acc
+} // namespace mlir
+
+#endif // MLIR_DIALECT_OPENACC_UTILS_OPENACCUTILS_H_
diff --git a/mlir/lib/Dialect/OpenACC/CMakeLists.txt b/mlir/lib/Dialect/OpenACC/CMakeLists.txt
index 9f57627c321fb0..31167e6af908b9 100644
--- a/mlir/lib/Dialect/OpenACC/CMakeLists.txt
+++ b/mlir/lib/Dialect/OpenACC/CMakeLists.txt
@@ -1,2 +1,3 @@
add_subdirectory(IR)
add_subdirectory(Transforms)
+add_subdirectory(Utils)
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 280260e0485bb5..1cc67629f97412 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2379,11 +2379,21 @@ checkDeclareOperands(Op &op, const mlir::ValueRange &operands,
"expect valid declare data entry operation or acc.getdeviceptr "
"as defining op");
- mlir::Value varPtr{getVarPtr(operand.getDefiningOp())};
+ mlir::Value varPtr{
+ llvm::TypeSwitch<mlir::Operation *, mlir::Value>(
+ operand.getDefiningOp())
+ .Case<ACC_DATA_ENTRY_OPS>(
+ [&](auto entry) { return entry.getVarPtr(); })
+ .Default([&](mlir::Operation *) { return mlir::Value(); })};
assert(varPtr && "declare operands can only be data entry operations which "
"must have varPtr");
std::optional<mlir::acc::DataClause> dataClauseOptional{
- getDataClause(operand.getDefiningOp())};
+ llvm::TypeSwitch<mlir::Operation *,
+ std::optional<mlir::acc::DataClause>>(
+ operand.getDefiningOp())
+ .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>(
+ [&](auto entry) { return entry.getDataClause(); })
+ .Default([&](mlir::Operation *) { return std::nullopt; })};
assert(dataClauseOptional.has_value() &&
"declare operands can only be data entry operations which must have "
"dataClause");
@@ -2409,8 +2419,13 @@ checkDeclareOperands(Op &op, const mlir::ValueRange &operands,
// since implicit data action may be inserted to do actions like updating
// device copy, in which case the variable is not necessarily implicitly
// declare'd.
+ bool operandOpImplicitFlag{
+ llvm::TypeSwitch<mlir::Operation *, bool>(operand.getDefiningOp())
+ .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>(
+ [&](auto entry) { return entry.getImplicit(); })
+ .Default([&](mlir::Operation *) { return false; })};
if (declAttr.getImplicit() &&
- declAttr.getImplicit() != acc::getImplicitFlag(operand.getDefiningOp()))
+ declAttr.getImplicit() != operandOpImplicitFlag)
return op.emitError(
"implicitness must match between declare op and flag on variable");
}
@@ -2868,127 +2883,3 @@ LogicalResult acc::WaitOp::verify() {
#define GET_TYPEDEF_CLASSES
#include "mlir/Dialect/OpenACC/OpenACCOpsTypes.cpp.inc"
-
-//===----------------------------------------------------------------------===//
-// acc dialect utilities
-//===----------------------------------------------------------------------===//
-
-mlir::Value mlir::acc::getVarPtr(mlir::Operation *accDataClauseOp) {
- auto varPtr{llvm::TypeSwitch<mlir::Operation *, mlir::Value>(accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS>(
- [&](auto entry) { return entry.getVarPtr(); })
- .Case<mlir::acc::CopyoutOp, mlir::acc::UpdateHostOp>(
- [&](auto exit) { return exit.getVarPtr(); })
- .Default([&](mlir::Operation *) { return mlir::Value(); })};
- return varPtr;
-}
-
-mlir::Value mlir::acc::getAccPtr(mlir::Operation *accDataClauseOp) {
- auto accPtr{llvm::TypeSwitch<mlir::Operation *, mlir::Value>(accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>(
- [&](auto dataClause) { return dataClause.getAccPtr(); })
- .Default([&](mlir::Operation *) { return mlir::Value(); })};
- return accPtr;
-}
-
-mlir::Value mlir::acc::getVarPtrPtr(mlir::Operation *accDataClauseOp) {
- auto varPtrPtr{
- llvm::TypeSwitch<mlir::Operation *, mlir::Value>(accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS>(
- [&](auto dataClause) { return dataClause.getVarPtrPtr(); })
- .Default([&](mlir::Operation *) { return mlir::Value(); })};
- return varPtrPtr;
-}
-
-mlir::SmallVector<mlir::Value>
-mlir::acc::getBounds(mlir::Operation *accDataClauseOp) {
- mlir::SmallVector<mlir::Value> bounds{
- llvm::TypeSwitch<mlir::Operation *, mlir::SmallVector<mlir::Value>>(
- accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>([&](auto dataClause) {
- return mlir::SmallVector<mlir::Value>(
- dataClause.getBounds().begin(), dataClause.getBounds().end());
- })
- .Default([&](mlir::Operation *) {
- return mlir::SmallVector<mlir::Value, 0>();
- })};
- return bounds;
-}
-
-mlir::SmallVector<mlir::Value>
-mlir::acc::getAsyncOperands(mlir::Operation *accDataClauseOp) {
- return llvm::TypeSwitch<mlir::Operation *, mlir::SmallVector<mlir::Value>>(
- accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>([&](auto dataClause) {
- return mlir::SmallVector<mlir::Value>(
- dataClause.getAsyncOperands().begin(),
- dataClause.getAsyncOperands().end());
- })
- .Default([&](mlir::Operation *) {
- return mlir::SmallVector<mlir::Value, 0>();
- });
-}
-
-mlir::ArrayAttr
-mlir::acc::getAsyncOperandsDeviceType(mlir::Operation *accDataClauseOp) {
- return llvm::TypeSwitch<mlir::Operation *, mlir::ArrayAttr>(accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>([&](auto dataClause) {
- return dataClause.getAsyncOperandsDeviceTypeAttr();
- })
- .Default([&](mlir::Operation *) { return mlir::ArrayAttr{}; });
-}
-
-mlir::ArrayAttr mlir::acc::getAsyncOnly(mlir::Operation *accDataClauseOp) {
- return llvm::TypeSwitch<mlir::Operation *, mlir::ArrayAttr>(accDataClauseOp)
- .Case<ACC_DATA_ENTRY_OPS, ACC_DATA_EXIT_OPS>(
- [&](auto dataClause) { return dataClause.getAsyncOnlyAttr(); })
- .Default([&](mlir::Operation *) { return mlir::ArrayAttr{}; });
-}
-
-std::optional<llvm::StringRef> mlir::acc::getVarName(mlir::Operation *accOp) {
- auto name{
- llvm::TypeSwitch<mlir::Operation *, std::optional<llvm::StringRef>>(accOp)
- .Case<ACC_DATA_ENTRY_OPS>([&](auto entry) { return entry.getName(); })
- .Default([&](mlir::Operation *) -> std::optional<llvm::StringRef> {
- return {};
- })};
- return name;
-}
-
-std::optional<mlir::acc::DataClause>
-mlir::acc::getDataClause(mlir::Operation *accDataEntryOp) {
- auto dataClause{
- llvm::TypeSwitch<mlir::Operation *, std::optional<mlir::acc::DataClause>>(
- accDataEntryOp)
- .Case<ACC_DATA_ENTRY_OPS>(
- [&](auto entry) { return entry.getDataClause(); })
- .Default([&](mlir::Operation *) { return std::nullopt; })};
- return dataClause;
-}
-
-bool mlir::acc::getImplicitFlag(mlir::Operation *accDataEntryOp) {
- auto implicit{llvm::TypeSwitch<mlir::Operation *, bool>(accDataEntryOp)
- .Case<ACC_DATA_ENTRY_OPS>(
- [&](auto entry) { return entry.getImplicit(); })
- .Default([&](mlir::Operation *) { return false; })};
- return implicit;
-}
-
-mlir::ValueRange mlir::acc::getDataOperands(mlir::Operation *accOp) {
- auto dataOperands{
- llvm::TypeSwitch<mlir::Operation *, mlir::ValueRange>(accOp)
- .Case<ACC_COMPUTE_AND_DATA_CONSTRUCT_OPS>(
- [&](auto entry) { return entry.getDataClauseOperands(); })
- .Default([&](mlir::Operation *) { return mlir::ValueRange(); })};
- return dataOperands;
-}
-
-mlir::MutableOperandRange
-mlir::acc::getMutableDataOperands(mlir::Operation *accOp) {
- auto dataOperands{
- llvm::TypeSwitch<mlir::Operation *, mlir::MutableOperandRange>(accOp)
- .Case<ACC_COMPUTE_AND_DATA_CONSTRUCT_OPS>(
- [&](auto entry) { return entry.getDataClauseOperandsMutable(); })
- .Default([&](mlir::Operation *) { return nullptr; })};
- return dataOperands;
-}
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/CMakeLists.txt b/mlir/lib/Dialect/OpenACC/Transforms/CMakeLists.txt
index 7d934956089a5a..42fcc138c5ffb9 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/CMakeLists.txt
+++ b/mlir/lib/Dialect/OpenACC/Transforms/CMakeLists.txt
@@ -15,6 +15,7 @@ add_mlir_dialect_library(MLIROpenACCTransforms
LINK_LIBS PUBLIC
MLIROpenACCDialect
+ MLIROpenACCUtils
MLIRFuncDialect
MLIRIR
MLIRPass
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/LegalizeDataValues.cpp b/mlir/lib/Dialect/OpenACC/Transforms/LegalizeDataValues.cpp
index 026b309ce4969d..028f53d05b1725 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/LegalizeDataValues.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/LegalizeDataValues.cpp
@@ -6,12 +6,11 @@
//
//===----------------------------------------------------------------------===//
-#include "mlir/Dialect/OpenACC/Transforms/Passes.h"
-
#include "mlir/Dialect/Func/IR/FuncOps.h"
#include "mlir/Dialect/OpenACC/OpenACC.h"
+#include "mlir/Dialect/OpenACC/Utils/OpenACCUtils.h"
+#include "mlir/Dialect/OpenACC/Transforms/Passes.h"
#include "mlir/Pass/Pass.h"
-#include "mlir/Transforms/RegionUtils.h"
#include "llvm/Support/ErrorHandling.h"
namespace mlir {
diff --git a/mlir/lib/Dialect/OpenACC/Utils/CMakeLists.txt b/mlir/lib/Dialect/OpenACC/Utils/CMakeLists.txt
new file mode 100644
index 00000000000000..83a843b65168ba
--- /dev/null
+++ b/mlir/lib/Dialect/OpenACC/Utils/CMakeLists.txt
@@ -0,0 +1,20 @@
+add_mlir_dialect_library(MLIROpenACCUtils
+ OpenACCUtils.cpp
+
+ ADDITIONAL_HEADER_DIRS
+ ${MLIR_MAIN_INCLUDE_DIR}/mlir/Dialect/OpenACC
+
+ DEPENDS
+ MLIROpenACCPassIncGen
+ MLIROpenACCOpsIncGen
+ MLIROpenACCEnumsIncGen
+ MLIROpenACCAttributesIncGen
+ MLIROpenACCMPOpsInterfacesIncGen
+ MLIROpenACCOpsInterfacesIncGen
+ MLIROpenACCTypeInterfacesIncGen
+
+ LINK_LIBS PUBLIC
+ MLIRIR
+ MLIROpenACCDialect
+ MLIRSupport
+)
diff --git a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp
new file mode 100644
index 00000000000000..2fa4d70fd1f3f7
--- /dev/null
+++ b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtils.cpp
@@ -0,0 +1,240 @@
+//===- OpenACCUtils.cpp ---------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Dialect/OpenACC/Utils/OpenACCUtils.h"
+#include "mlir/Dialect/OpenACC/OpenACC.h"
+#include "llvm/ADT/TypeSwitch.h"
+
+mlir::Value mlir::acc::getVarPtr(mlir::Operation *accDataClauseOp) {
+ auto varPtr{llvm::TypeSwitch<mlir::Operation *, mlir::Value>(accDataClauseOp)
+ .Case<ACC_DATA_ENTRY_OPS, mlir::acc::CopyoutOp,
+ mlir::acc::UpdateHostOp>([&](auto dataClauseOp) {
+ return dataClauseOp.getVarPtr();
+ })
+ .Default([&](mlir::Operation *) { return mlir::Value(); })};
+ return varPtr;
+}
+
+bool mlir::acc::setVarPtr(mlir::Operation *accDataClauseOp,
+ mlir::Value varPtr) {
+ bool res{llvm::TypeSwitch<mlir::Operation *, bool>(accDataClauseOp)
+ .Case<ACC_DATA_ENTRY_OPS, mlir::acc::CopyoutOp,
+ mlir::acc::Update...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
clementval
reviewed
Nov 6, 2024
Comment on lines
-86
to
-138
| /// Used to obtain the `varPtr` from a data clause operation. | ||
| /// Returns empty value if not a data clause operation or is a data exit | ||
| /// operation with no `varPtr`. | ||
| mlir::Value getVarPtr(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Used to obtain the `accPtr` from a data clause operation. | ||
| /// When a data entry operation, it obtains its result `accPtr` value. | ||
| /// If a data exit operation, it obtains its operand `accPtr` value. | ||
| /// Returns empty value if not a data clause operation. | ||
| mlir::Value getAccPtr(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Used to obtain the `varPtrPtr` from a data clause operation. | ||
| /// Returns empty value if not a data clause operation. | ||
| mlir::Value getVarPtrPtr(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Used to obtain `bounds` from an acc data clause operation. | ||
| /// Returns an empty vector if there are no bounds. | ||
| mlir::SmallVector<mlir::Value> getBounds(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Used to obtain `async` operands from an acc data clause operation. | ||
| /// Returns an empty vector if there are no such operands. | ||
| mlir::SmallVector<mlir::Value> | ||
| getAsyncOperands(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Returns an array of acc:DeviceTypeAttr attributes attached to | ||
| /// an acc data clause operation, that correspond to the device types | ||
| /// associated with the async clauses with an async-value. | ||
| mlir::ArrayAttr getAsyncOperandsDeviceType(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Returns an array of acc:DeviceTypeAttr attributes attached to | ||
| /// an acc data clause operation, that correspond to the device types | ||
| /// associated with the async clauses without an async-value. | ||
| mlir::ArrayAttr getAsyncOnly(mlir::Operation *accDataClauseOp); | ||
|
|
||
| /// Used to obtain the `name` from an acc operation. | ||
| std::optional<llvm::StringRef> getVarName(mlir::Operation *accOp); | ||
|
|
||
| /// Used to obtain the `dataClause` from a data entry operation. | ||
| /// Returns empty optional if not a data entry operation. | ||
| std::optional<mlir::acc::DataClause> | ||
| getDataClause(mlir::Operation *accDataEntryOp); | ||
|
|
||
| /// Used to find out whether data operation is implicit. | ||
| /// Returns false if not a data operation or if it is a data operation without | ||
| /// implicit flag. | ||
| bool getImplicitFlag(mlir::Operation *accDataEntryOp); | ||
|
|
||
| /// Used to get an immutable range iterating over the data operands. | ||
| mlir::ValueRange getDataOperands(mlir::Operation *accOp); | ||
|
|
||
| /// Used to get a mutable range iterating over the data operands. | ||
| mlir::MutableOperandRange getMutableDataOperands(mlir::Operation *accOp); | ||
|
|
There was a problem hiding this comment.
I think it would be nicer to refactor this into proper interfaces for these operations instead of just creating a new lib.
There was a problem hiding this comment.
I agree with this. There are a couple of reasons I decided to do this MR this way:
- The existing utilities work with opaque
Operation. So no need to have any explicit casts in source code. - The interfaces, as I am imagining, are being "DataEntryOpInterface" and "DataExitOpInterface". So from the user side, it is still more steps than just calling one of these utilities. Some operations don't have varPtr, some cannot redefine accPtr, and other inconsistencies still require more logic from use side.
- This MR makes the improvement so that we do have a place for new utilities for acc in a way that matches MLIR conventions. I did not want to add new utilities in the dialect itself.
I see this MR as orthogonal to your recommendation. The utilities can use the interfaces that we define.
There was a problem hiding this comment.
MR -> PR
I'm not sure I see the motivation to move this away from the ops. Now you need to duplicate the code in the dialect part and have an extra library to link with. These utility functions are pretty much tied to the operations or do I miss something? Ultimately I'm not against it but it feels odd to duplicate some code. Maybe these could be inlined function defined in header?
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.
This MR accomplishes the following: