[Mlir-commits] [mlir] remove verifier (PR #137676)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-openacc

Author: Susan Tan (ス-ザン　タン) (SusanTan)

<details>
<summary>Changes</summary>

The part that verifies the declare attributes are preserved in the verifier can fail easily during the FIR lowering pipeline. For example, during FIR lowering to FIRCG, fir.declare can be removed. Thus, any fir.declare that has acc.declare attributes will cause a verifier failure. Since the declare attribute only existed to simplify the effort of locating acc declare enter and exit points, which can be easily replaced by a def-use chain traversal, we are considering removing the verification of declare attributes in this MR.

Example:
 ``` %1 = fir.alloca !fir.array<10xf32> {bindc_name = "arr", uniq_name = "_QMmmFsubEarr"}                                                                                                                                  %2 = fir.shape %c10 : (index) -> !fir.shape<1>
  %3 = fir.declare %1(%2) {acc.declare = #acc.declare<dataClause =  acc_create>, uniq_name = "_QMmmFsubEarr"} : (!fir.ref<!fir.array<10xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<10xf32>>
  %4 = acc.create varPtr(%3 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "arr"}
  %5 = acc.declare_enter dataOperands(%4 : !fir.ref<!fir.array<10xf32>>)
  %6 = fir.array_coor %3(%2) %c1 : (!fir.ref<!fir.array<10xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>```

the acc.declare_enter itself is enough to identify when the data region starts.

---
Full diff: https://github.com/llvm/llvm-project/pull/137676.diff


1 Files Affected:

- (modified) mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp (-26) 


``````````diff
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 56f3228d3a652..66584dc708b8c 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2910,32 +2910,6 @@ checkDeclareOperands(Op &op, const mlir::ValueRange &operands,
     assert(dataClauseOptional.has_value() &&
            "declare operands can only be data entry operations which must have "
            "dataClause");
-
-    // If varPtr has no defining op - there is nothing to check further.
-    if (!var.getDefiningOp())
-      continue;
-
-    // Check that the varPtr has a declare attribute.
-    auto declareAttribute{
-        var.getDefiningOp()->getAttr(mlir::acc::getDeclareAttrName())};
-    if (!declareAttribute)
-      return op.emitError(
-          "expect declare attribute on variable in declare operation");
-
-    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();

``````````

</details>


https://github.com/llvm/llvm-project/pull/137676