[Mlir-commits] [mlir] 4bdc905 - [openacc] Add implicit flag to declare attribute

[Razvan Lupusoru]

Author: Razvan Lupusoru
Date: 2023-08-29T13:13:53-07:00
New Revision: 4bdc9057e971f0d12b71d46b2a2a36873533025d

URL: https://github.com/llvm/llvm-project/commit/4bdc9057e971f0d12b71d46b2a2a36873533025d
DIFF: https://github.com/llvm/llvm-project/commit/4bdc9057e971f0d12b71d46b2a2a36873533025d.diff

LOG: [openacc] Add implicit flag to declare attribute

The declare attribute has been updated to allow implicit flag. This is
useful for variables that can be declare'd implicitly - like global
constants. The verifier has been updated to ensure that an implicit
declare'd variable has an implicit data action. The builder doesn't
require for this flag to be set so any code creating this attribute
will continue to work as-is.

Reviewed By: vzakhari

Differential Revision: https://reviews.llvm.org/D159124

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/OpenACC/OpenACC.h
    mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
    mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
    mlir/test/Dialect/OpenACC/ops.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
index f2b9a19f1858b5..f294df85ce96ea 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
@@ -73,6 +73,11 @@ mlir::Value getVarPtr(mlir::Operation *accDataEntryOp);
 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 obtain the attribute name for declare.
 static constexpr StringLiteral getDeclareAttrName() {
   return StringLiteral("acc.declare");

diff  --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 5d667bc9e065da..58c763995ebe59 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -131,8 +131,13 @@ class OpenACC_Attr<string name, string attrMnemonic,
 // easier to find out whether the variable is in a declare clause and what kind
 // of clause it is.
 def DeclareAttr : OpenACC_Attr<"Declare", "declare"> {
-  let parameters = (ins "DataClauseAttr":$dataClause);
+  let parameters = (ins "DataClauseAttr":$dataClause,
+                        DefaultValuedParameter<"bool", "false">:$implicit);
   let assemblyFormat = "`<` struct(params) `>`";
+  let builders = [AttrBuilder<(ins "DataClauseAttr":$dataClause), [{
+      return $_get($_ctxt, dataClause, /*implicit=*/false);
+    }]>
+  ];
 }
 
 // Attribute to attach functions that perform the pre/post allocation actions or

diff  --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 9b893eeb83c066..8cda5c8c214388 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -1015,11 +1015,21 @@ static LogicalResult checkDeclareOperands(Op &op,
     if (!declareAttribute)
       return op.emitError(
           "expect declare attribute on variable in declare operation");
-    if (mlir::cast<mlir::acc::DeclareAttr>(declareAttribute)
-            .getDataClause()
-            .getValue() != dataClauseOptional.value())
+
+    auto declAttr = mlir::cast<mlir::acc::DeclareAttr>(declareAttribute);
+    if (declAttr.getDataClause().getValue() != dataClauseOptional.value())
       return op.emitError(
           "expect matching declare attribute on variable in declare operation");
+
+    // If the variable is marked with implicit attribute, the matching declare
+    // data action must also be marked implicit. The reverse is not checked
+    // 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.
+    if (declAttr.getImplicit() &&
+        declAttr.getImplicit() != acc::getImplicitFlag(operand.getDefiningOp()))
+      return op.emitError(
+          "implicitness must match between declare op and flag on variable");
   }
 
   return success();
@@ -1234,3 +1244,11 @@ mlir::acc::getDataClause(mlir::Operation *accDataEntryOp) {
           .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;
+}

diff  --git a/mlir/test/Dialect/OpenACC/ops.mlir b/mlir/test/Dialect/OpenACC/ops.mlir
index e4b3a3bf548bdf..ab39a4192deb1a 100644
--- a/mlir/test/Dialect/OpenACC/ops.mlir
+++ b/mlir/test/Dialect/OpenACC/ops.mlir
@@ -1684,12 +1684,17 @@ func.func @compute3(%a: memref<10x10xf32>, %b: memref<10x10xf32>, %c: memref<10x
   %numGangs = arith.constant 10 : i64
   %numWorkers = arith.constant 10 : i64
 
+  %c20 = arith.constant 20 : i32
+  %alloc = llvm.alloca %c20 x i32 { acc.declare = #acc.declare<dataClause = acc_create, implicit = true> } : (i32) -> !llvm.ptr<i32>
+  %createlocal = acc.create varPtr(%alloc : !llvm.ptr<i32>) -> !llvm.ptr<i32> {implicit = true}
+
   %pa = acc.present varPtr(%a : memref<10x10xf32>) -> memref<10x10xf32>
   %pb = acc.present varPtr(%b : memref<10x10xf32>) -> memref<10x10xf32>
   %pc = acc.present varPtr(%c : memref<10xf32>) -> memref<10xf32>
   %pd = acc.present varPtr(%d : memref<10xf32>) -> memref<10xf32>
-  acc.declare dataOperands(%pa, %pb, %pc, %pd: memref<10x10xf32>, memref<10x10xf32>, memref<10xf32>, memref<10xf32>) {
+  acc.declare dataOperands(%pa, %pb, %pc, %pd, %createlocal: memref<10x10xf32>, memref<10x10xf32>, memref<10xf32>, memref<10xf32>, !llvm.ptr<i32>) {
   }
+
   return
 }