[Flang] Propogate fir.declare attributes through cg-rewrite #137207
Conversation
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-codegen Author: Susan Tan (ス-ザン タン) (SusanTan) ChangesAdds attribute handling during the codegen rewrite of fir.declare operations.
This enables development based on these attributes. For instance, OpenACC codegen for fir.declare depends on the presence of the acc.declare attribute (added as a lit test) Full diff: https://github.com/llvm/llvm-project/pull/137207.diff 2 Files Affected:
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index d09d7d397e8b7..0b9de86de1e95 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -281,6 +281,20 @@ 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 +313,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"}
|
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Susan Tan (ス-ザン タン) (SusanTan) ChangesAdds attribute handling during the codegen rewrite of fir.declare operations.
This enables development based on these attributes. For instance, OpenACC codegen for fir.declare depends on the presence of the acc.declare attribute (added as a lit test) Full diff: https://github.com/llvm/llvm-project/pull/137207.diff 2 Files Affected:
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index d09d7d397e8b7..0b9de86de1e95 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -281,6 +281,20 @@ 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 +313,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"}
|
| if (!preserveDeclare) { | ||
| auto memrefOp = declareOp.getMemref().getDefiningOp(); | ||
| if (!memrefOp) { | ||
| rewriter.replaceOp(declareOp, declareOp.getMemref()); |
There was a problem hiding this comment.
Instead of duplicating this code - I would simply guard the attachment of attributes with if (declareOp.getMemref().getDefiningOp())
|
Approach seems reasonable to me but Jean needs to approve if he agrees. Thank you! |
| matchAndRewrite(fir::DeclareOp declareOp, | ||
| mlir::PatternRewriter &rewriter) const override { | ||
| if (!preserveDeclare) { | ||
| auto memrefOp = declareOp.getMemref().getDefiningOp(); |
There was a problem hiding this comment.
I find it a bit weird to propagate the attributes on the declare memref input producing op. First of all, you may end-up in weird situations where a single alloca maps to several declare (looking at you equivalences), so that could conflict.
Why do you need to propagate attribute if the cg declare is not emitted?
It looks like to me that if one wants to preserve language level info, the declare should also be preserved.
There was a problem hiding this comment.
that's a good catch, thanks Jean! I will remove this part and sought solutions downstream. thanks!
| declareOp.getTypeparams(), declareOp.getDummyScope(), | ||
| declareOp.getUniqName()); | ||
|
|
||
| // attach metadatas from fir.declare to fircg.ext_declare |
There was a problem hiding this comment.
I would update this comment to say something like:
"// Propagate all attributes to fircg.ext_declare to avoid losing metadata including CUDA and OpenACC attributes."
razvanlupusoru
left a comment
razvanlupusoru
left a comment
There was a problem hiding this comment.
Looks good to me. Let's wait for Jean's approval. Thank you!
| // Propagate all attributes to fircg.ext_declare to avoid losing metadata | ||
| // including CUDA and OpenACC attributes. | ||
| for (const mlir::NamedAttribute &attr : declareOp->getAttrs()) | ||
| xDeclOp->setAttr(attr.getName(), attr.getValue()); |
There was a problem hiding this comment.
How is this not overriding the operandSegmentSizes attribute of the AttrSizedOperandSegments interface of the newly created xDeclOp with the one from the declareOp?
I would suggest checking if the attributes names are not already used in the xDeclop to avoid any issues.
4 participants
Adds attribute handling during the codegen rewrite of fir.declare operations.
When a fir.declare operation is not preserved, non-conflicting attributes are attached directly to the associated memref operation.
When a fir.declare operation is preserved, all attributes are propagated to the corresponding fircg.ext_declare operation.
This enables development based on these attributes. For instance, OpenACC codegen for fir.declare depends on the presence of the acc.declare attribute (added as a lit test)