Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang] Fix attribute printing for fir.global op #81197

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

clementval
Copy link
Contributor

The custom printer for fir.global was eluding all the attributes present on the op when printing the attribute dictionary. So any attribute that is not part of the pretty printing was therefore discarded.
This patch fix the printer and also make use of the getters for the attribute names when they are hardcoded.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Feb 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

The custom printer for fir.global was eluding all the attributes present on the op when printing the attribute dictionary. So any attribute that is not part of the pretty printing was therefore discarded.
This patch fix the printer and also make use of the getters for the attribute names when they are hardcoded.


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

2 Files Affected:

  • (modified) flang/lib/Optimizer/Dialect/FIROps.cpp (+11-7)
  • (modified) flang/test/Fir/fir-ops.fir (+7)
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 483f318b649d9c..991d83df7f6745 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -1348,12 +1348,12 @@ mlir::ParseResult fir::GlobalOp::parse(mlir::OpAsmParser &parser,
   if (parser.parseOptionalAttrDict(result.attributes))
     return mlir::failure();
 
-  if (succeeded(parser.parseOptionalKeyword("constant"))) {
+  if (succeeded(parser.parseOptionalKeyword(getConstantAttrNameStr()))) {
     // if "constant" keyword then mark this as a constant, not a variable
-    result.addAttribute("constant", builder.getUnitAttr());
+    result.addAttribute(getConstantAttrNameStr(), builder.getUnitAttr());
   }
 
-  if (succeeded(parser.parseOptionalKeyword("target")))
+  if (succeeded(parser.parseOptionalKeyword(getTargetAttrNameStr())))
     result.addAttribute(getTargetAttrNameStr(), builder.getUnitAttr());
 
   mlir::Type globalType;
@@ -1382,11 +1382,15 @@ void fir::GlobalOp::print(mlir::OpAsmPrinter &p) {
   p.printAttributeWithoutType(getSymrefAttr());
   if (auto val = getValueOrNull())
     p << '(' << val << ')';
-  p.printOptionalAttrDict((*this)->getAttrs(), (*this).getAttributeNames());
-  if (getOperation()->getAttr(fir::GlobalOp::getConstantAttrNameStr()))
-    p << " constant";
+  // Print all other attributes that are not pretty printed here.
+  p.printOptionalAttrDict((*this)->getAttrs(), /*elideAttrs=*/{
+      getSymNameAttrName(), getSymrefAttrName(), getTypeAttrName(),
+      getConstantAttrName(), getTargetAttrName(), getLinkNameAttrName(),
+      getInitValAttrName()});
+  if (getOperation()->getAttr(getConstantAttrName()))
+    p << " " << getConstantAttrNameStr();
   if (getOperation()->getAttr(getTargetAttrName()))
-    p << " target";
+    p << " " << getTargetAttrNameStr();
   p << " : ";
   p.printType(getType());
   if (hasInitializationBody()) {
diff --git a/flang/test/Fir/fir-ops.fir b/flang/test/Fir/fir-ops.fir
index 3c4095b9fdb140..962621c4e2e1ac 100644
--- a/flang/test/Fir/fir-ops.fir
+++ b/flang/test/Fir/fir-ops.fir
@@ -893,3 +893,10 @@ func.func @test_box_typecode(%a: !fir.class<none>) {
 // CHECK-LABEL: func.func @test_box_typecode(
 // CHECK-SAME: %[[A:.*]]: !fir.class<none>)
 // CHECK: %{{.*}} = fir.box_typecode %[[A]] : (!fir.class<none>) -> i32
+
+fir.global @t1 {keep_my_attr = "data"} : i32 {
+ %1 = arith.constant 0 : i32
+  fir.has_value %1 : i32
+}
+
+// CHECK-LABEL: fir.global @t1 {keep_my_attr = "data"} : i32

Copy link

github-actions bot commented Feb 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@clementval clementval merged commit 8c106a1 into llvm:main Feb 8, 2024
4 checks passed
@clementval clementval deleted the fir_globalop_printing branch February 8, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants