[flang-commits] [flang] [mlir] [OpenACC] Remove attribute verification (PR #137674)
Susan Tan ス-ザン タン via flang-commits
flang-commits at lists.llvm.org
Mon Apr 28 10:30:37 PDT 2025
https://github.com/SusanTan created https://github.com/llvm/llvm-project/pull/137674
The part that verifies the declare attributes are preserved in the verifier can fail easily during a 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 thinking of removing the verification of declae 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>>)`
the acc.declare_enter itself is enough to identify when the data region starts.
>From 3301f3e57951c0a8e2bba78611bbf10a8387ed9f Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Thu, 24 Apr 2025 08:58:30 -0700
Subject: [PATCH 1/5] propogate attributes through cg-rewrite
---
flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp | 20 +++++++++++++++++-
flang/test/Fir/declare-codegen.fir | 22 +++++++++++++++++---
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index d09d7d397e8b7..052c96b15570f 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -281,6 +281,19 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
matchAndRewrite(fir::DeclareOp declareOp,
mlir::PatternRewriter &rewriter) const override {
if (!preserveDeclare) {
+ auto memrefOp = declareOp.getMemref().getDefiningOp();
+ if(!memrefOp){
+ rewriter.replaceOp(declareOp, declareOp.getMemref());
+ return mlir::success();
+ }
+
+ // attach metadatas from the fir.declare to its memref (if it's an operation)
+ mlir::NamedAttrList elidedAttrs =
+ mlir::NamedAttrList{memrefOp->getAttrs()};
+ for (const mlir::NamedAttribute &attr : declareOp->getAttrs())
+ if (!elidedAttrs.get(attr.getName()))
+ memrefOp->setAttr(attr.getName(), attr.getValue());
+
rewriter.replaceOp(declareOp, declareOp.getMemref());
return mlir::success();
}
@@ -299,11 +312,16 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
else
return mlir::failure();
}
- // FIXME: Add FortranAttrs and CudaAttrs
+
auto xDeclOp = rewriter.create<fir::cg::XDeclareOp>(
loc, declareOp.getType(), declareOp.getMemref(), shapeOpers, shiftOpers,
declareOp.getTypeparams(), declareOp.getDummyScope(),
declareOp.getUniqName());
+
+ // attach metadatas from fir.declare to fircg.ext_declare
+ for (const mlir::NamedAttribute &attr : declareOp->getAttrs())
+ xDeclOp->setAttr(attr.getName(), attr.getValue());
+
LLVM_DEBUG(llvm::dbgs()
<< "rewriting " << declareOp << " to " << xDeclOp << '\n');
rewriter.replaceOp(declareOp, xDeclOp.getOperation()->getResults());
diff --git a/flang/test/Fir/declare-codegen.fir b/flang/test/Fir/declare-codegen.fir
index fe8d84ef2d19f..ad0208219ddf7 100644
--- a/flang/test/Fir/declare-codegen.fir
+++ b/flang/test/Fir/declare-codegen.fir
@@ -23,7 +23,7 @@ func.func private @bar(%arg0: !fir.ref<!fir.array<12x23xi32>>)
// DECL-LABEL: func.func @test(
// DECL-SAME: %[[arg0:.*]]: !fir.ref<!fir.array<12x23xi32>>) {
-// DECL: fircg.ext_declare
+// DECL: fircg.ext_declare{{.*}}{uniq_name = "_QFarray_numeric_lboundsEx"}
func.func @useless_shape_with_duplicate_extent_operand(%arg0: !fir.ref<!fir.array<3x3xf32>>) {
@@ -37,7 +37,7 @@ func.func @useless_shape_with_duplicate_extent_operand(%arg0: !fir.ref<!fir.arra
// NODECL-NEXT: return
// DECL-LABEL: func.func @useless_shape_with_duplicate_extent_operand(
-// DECL: fircg.ext_declare
+// DECL: fircg.ext_declare{{.*}}{uniq_name = "u"}
// Test DCE does not crash because of unreachable code.
func.func @unreachable_code(%arg0: !fir.ref<!fir.char<1,10>>) {
@@ -51,4 +51,20 @@ func.func @unreachable_code(%arg0: !fir.ref<!fir.char<1,10>>) {
// NODECL-LABEL: func.func @unreachable_code(
// NODECL-NOT: uniq_name = "live_code"
// DECL-LABEL: func.func @unreachable_code(
-// DECL: uniq_name = "live_code"
+// DECL: fircg.ext_declare{{.*}}{uniq_name = "live_code"}
+// DECL: fir.declare{{.*}}{uniq_name = "dead_code"}
+
+
+// Test that attributes get attached to the memref operation when fir.declare is not preserved
+func.func @test_local_memref() {
+ %0 = fir.alloca !fir.array<10xi32> {uniq_name = "local_array"}
+ %1 = fir.declare %0 {uniq_name = "local_array_declare", acc.declare = #acc.declare<dataClause = acc_create>} : (!fir.ref<!fir.array<10xi32>>) -> !fir.ref<!fir.array<10xi32>>
+ return
+}
+// NODECL-LABEL: func.func @test_local_memref() {
+
+// Note: the uniq_name is not attached of fir.declare is not preserved because it conflicts with the uniq_name of the memref
+// NODECL: fir.alloca{{.*}}{acc.declare = #acc.declare<dataClause = acc_create>, uniq_name = "local_array"}
+
+// DECL-LABEL: func.func @test_local_memref() {
+// DECL: fircg.ext_declare{{.*}}{acc.declare = #acc.declare<dataClause = acc_create>, uniq_name = "local_array_declare"}
>From 353e86a946eb79011fd4f52f9e1d776b8b2eca6e Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Thu, 24 Apr 2025 08:59:33 -0700
Subject: [PATCH 2/5] format
---
flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index 052c96b15570f..0b9de86de1e95 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -281,13 +281,14 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
matchAndRewrite(fir::DeclareOp declareOp,
mlir::PatternRewriter &rewriter) const override {
if (!preserveDeclare) {
- auto memrefOp = declareOp.getMemref().getDefiningOp();
- if(!memrefOp){
- rewriter.replaceOp(declareOp, declareOp.getMemref());
- return mlir::success();
+ auto memrefOp = declareOp.getMemref().getDefiningOp();
+ if (!memrefOp) {
+ rewriter.replaceOp(declareOp, declareOp.getMemref());
+ return mlir::success();
}
-
- // attach metadatas from the fir.declare to its memref (if it's an operation)
+
+ // attach metadatas from the fir.declare to its memref (if it's an
+ // operation)
mlir::NamedAttrList elidedAttrs =
mlir::NamedAttrList{memrefOp->getAttrs()};
for (const mlir::NamedAttribute &attr : declareOp->getAttrs())
@@ -319,8 +320,8 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
declareOp.getUniqName());
// attach metadatas from fir.declare to fircg.ext_declare
- for (const mlir::NamedAttribute &attr : declareOp->getAttrs())
- xDeclOp->setAttr(attr.getName(), attr.getValue());
+ for (const mlir::NamedAttribute &attr : declareOp->getAttrs())
+ xDeclOp->setAttr(attr.getName(), attr.getValue());
LLVM_DEBUG(llvm::dbgs()
<< "rewriting " << declareOp << " to " << xDeclOp << '\n');
>From aa366aed1644489dc94f3471920c2c13e82baec4 Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Mon, 28 Apr 2025 09:41:07 -0700
Subject: [PATCH 3/5] remove propogation when fir.declare is removed
---
flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp | 14 --------------
flang/test/Fir/declare-codegen.fir | 5 -----
2 files changed, 19 deletions(-)
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index 0b9de86de1e95..587bcaf718c0b 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -281,20 +281,6 @@ class DeclareOpConversion : public mlir::OpRewritePattern<fir::DeclareOp> {
matchAndRewrite(fir::DeclareOp declareOp,
mlir::PatternRewriter &rewriter) const override {
if (!preserveDeclare) {
- auto memrefOp = declareOp.getMemref().getDefiningOp();
- if (!memrefOp) {
- rewriter.replaceOp(declareOp, declareOp.getMemref());
- return mlir::success();
- }
-
- // attach metadatas from the fir.declare to its memref (if it's an
- // operation)
- mlir::NamedAttrList elidedAttrs =
- mlir::NamedAttrList{memrefOp->getAttrs()};
- for (const mlir::NamedAttribute &attr : declareOp->getAttrs())
- if (!elidedAttrs.get(attr.getName()))
- memrefOp->setAttr(attr.getName(), attr.getValue());
-
rewriter.replaceOp(declareOp, declareOp.getMemref());
return mlir::success();
}
diff --git a/flang/test/Fir/declare-codegen.fir b/flang/test/Fir/declare-codegen.fir
index ad0208219ddf7..cf4f28eda1cb4 100644
--- a/flang/test/Fir/declare-codegen.fir
+++ b/flang/test/Fir/declare-codegen.fir
@@ -61,10 +61,5 @@ func.func @test_local_memref() {
%1 = fir.declare %0 {uniq_name = "local_array_declare", acc.declare = #acc.declare<dataClause = acc_create>} : (!fir.ref<!fir.array<10xi32>>) -> !fir.ref<!fir.array<10xi32>>
return
}
-// NODECL-LABEL: func.func @test_local_memref() {
-
-// Note: the uniq_name is not attached of fir.declare is not preserved because it conflicts with the uniq_name of the memref
-// NODECL: fir.alloca{{.*}}{acc.declare = #acc.declare<dataClause = acc_create>, uniq_name = "local_array"}
-
// DECL-LABEL: func.func @test_local_memref() {
// DECL: fircg.ext_declare{{.*}}{acc.declare = #acc.declare<dataClause = acc_create>, uniq_name = "local_array_declare"}
>From 24f2eef9e7717c643cd033d33e6b7b8506f17dff Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Mon, 28 Apr 2025 10:15:36 -0700
Subject: [PATCH 4/5] remove verifier
---
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 28 -------------------------
1 file changed, 28 deletions(-)
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 56f3228d3a652..d57b21238d9d9 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2910,34 +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();
}
>From db189d570afd3383819cdf9c735c4f55cffb6316 Mon Sep 17 00:00:00 2001
From: Susan Tan <zujunt at nvidia.com>
Date: Mon, 28 Apr 2025 10:17:25 -0700
Subject: [PATCH 5/5] missed {
---
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index d57b21238d9d9..66584dc708b8c 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2910,6 +2910,8 @@ checkDeclareOperands(Op &op, const mlir::ValueRange &operands,
assert(dataClauseOptional.has_value() &&
"declare operands can only be data entry operations which must have "
"dataClause");
+ }
+
return success();
}
More information about the flang-commits
mailing list