[Flang][MLIR] Alter Fir.GlobalOp to print and lower external attributes

[agozillon]

Summary

Fir.GlobalOp's currently do not respect attributes that
are applied to them, this change will do two things:

1. Allow lowering of arbitrary attributes applied to Fir.GlobalOp's to LLVMGlobalOp's during CodeGen
2. Allow printing and parsing of arbitrarily applied attributes

This allows applying other dialects attributes (or other
fir attributes) to fir.GlobalOps on the fly and have them
exist in the resulting LLVM dialect IR or FIR IR.

[agozillon]

Add missing newline

[agozillon]

Not sure who the best reviewer(s) is for this, unfortunately! So please do add reviewers if I am missing any.

This is something I came across while utilizing an OpenMP dialect attribute that I'd like to be able to apply to fir.GlobalOp's. I am unsure if ignoring externally applied attributes is the desired behavior, however. Although aligning it a little more with the LLVM Dialects GlobalOp's in this respect seemed reasonable.

[jeanPerier]

Makes sense to me.
I think there is a way to get all the names of the attributes explicitly defined for an operation using getAttributeNames(). It is generated by MLIR in FIROps.h.inc and looks like:

`
class GlobalOp  /* ....*/ {
  // ....
  static ::llvm::ArrayRef<::llvm::StringRef> getAttributeNames() {
    static ::llvm::StringRef attrNames[] = {::llvm::StringRef("constant"), ::llvm::StringRef("initVal"), ::llvm::StringRef("linkName"), ::llvm::StringRef("sym_name"), ::llvm::StringRef("symref"), ::llvm::StringRef("target"), ::llvm::StringRef("type")};
    return ::llvm::ArrayRef(attrNames);
  }
`

Can you try using it?
This would avoid having to manually list of attributes explicitly handled by the GlobalOp in codgen and in the printer.

[agozillon]

• [Flang][MLIR] Apply use of getAttributeNames to simplify and make changes more robust

[agozillon]

In D148352#4279572, @jeanPerier wrote:

Makes sense to me.
I think there is a way to get all the names of the attributes explicitly defined for an operation using getAttributeNames(). It is generated by MLIR in FIROps.h.inc and looks like:

`
class GlobalOp  /* ....*/ {
  // ....
  static ::llvm::ArrayRef<::llvm::StringRef> getAttributeNames() {
    static ::llvm::StringRef attrNames[] = {::llvm::StringRef("constant"), ::llvm::StringRef("initVal"), ::llvm::StringRef("linkName"), ::llvm::StringRef("sym_name"), ::llvm::StringRef("symref"), ::llvm::StringRef("target"), ::llvm::StringRef("type")};
    return ::llvm::ArrayRef(attrNames);
  }
`

Can you try using it?
This would avoid having to manually list of attributes explicitly handled by the GlobalOp in codgen and in the printer.

Thank you for pointing this out, I was unaware that it only gave the explicit attributes! I've applied the changes now and they work nicely.

[jeanPerier]

LGTM, thanks

[agozillon]

In D148352#4283085, @jeanPerier wrote:

LGTM, thanks

Thank you very much for the review!

flang/lib/Optimizer/CodeGen/CodeGen.cpp

matchAndRewrite(fir::GlobalOp global, OpAdaptor adaptor,
     if (global.getType().isa<fir::BaseBoxType>())
       tyAttr = tyAttr.cast<mlir::LLVM::LLVMPointerType>().getElementType();
     auto loc = global.getLoc();
     mlir::Attribute initAttr = global.getInitVal().value_or(mlir::Attribute());
     auto linkage = convertLinkage(global.getLinkName());
     auto isConst = global.getConstant().has_value();
     auto g = rewriter.create<mlir::LLVM::GlobalOp>(
         loc, tyAttr, isConst, linkage, global.getSymName(), initAttr);
+
+    // Apply all non-Fir::GlobalOp attributes to the LLVM::GlobalOp, preserving
+    // them; whilst taking care not to apply attributes that are lowered in
+    // other ways.
+    llvm::SmallDenseSet<llvm::StringRef> elidedAttrsSet(
+        global.getAttributeNames().begin(), global.getAttributeNames().end());
+    for (auto &attr : global->getAttrs())
+      if (!elidedAttrsSet.contains(attr.getName().strref()))
+        g->setAttr(attr.getName(), attr.getValue());
+
     auto &gr = g.getInitializerRegion();
     rewriter.inlineRegionBefore(global.getRegion(), gr, gr.end());
     if (!gr.empty()) {
       // Replace insert_on_range with a constant dense attribute if the
       // initialization is on the full range.
       auto insertOnRangeOps = gr.front().getOps<fir::InsertOnRangeOp>();
       for (auto insertOp : insertOnRangeOps) {
         if (isFullRange(insertOp.getCoor(), insertOp.getType())) {

flang/lib/Optimizer/Dialect/FIROps.cpp

mlir::ParseResult fir::GlobalOp::parse(mlir::OpAsmParser &parser,
   if (mlir::succeeded(parser.parseOptionalLParen())) {
     mlir::Attribute attr;
     if (parser.parseAttribute(attr, "initVal", result.attributes) ||
         parser.parseRParen())
       return mlir::failure();
     simpleInitializer = true;
   }
 
+  if (parser.parseOptionalAttrDict(result.attributes))
+    return mlir::failure();
+
   if (succeeded(parser.parseOptionalKeyword("constant"))) {
     // if "constant" keyword then mark this as a constant, not a variable
     result.addAttribute("constant", builder.getUnitAttr());
   }
 
   if (succeeded(parser.parseOptionalKeyword("target")))
     result.addAttribute(getTargetAttrNameStr(), builder.getUnitAttr());
 

 
 void fir::GlobalOp::print(mlir::OpAsmPrinter &p) {
   if (getLinkName())
     p << ' ' << *getLinkName();
   p << ' ';
   p.printAttributeWithoutType(getSymrefAttr());
   if (auto val = getValueOrNull())
     p << '(' << val << ')';
+  p.printOptionalAttrDict((*this)->getAttrs(), (*this).getAttributeNames());
   if (getOperation()->getAttr(fir::GlobalOp::getConstantAttrNameStr()))
     p << " constant";
   if (getOperation()->getAttr(getTargetAttrName()))
     p << " target";
   p << " : ";
   p.printType(getType());
   if (hasInitializationBody()) {
     p << ' ';

flang/test/Fir/global-attributes.fir

+// RUN: fir-opt --fir-to-llvm-ir %s | FileCheck %s
+// RUN: tco --fir-to-llvm-ir %s | FileCheck %s
+// RUN: fir-opt %s | FileCheck %s --check-prefix=READ-OUT
+// RUN: tco --emit-fir %s | FileCheck %s --check-prefix=READ-OUT
+
+// CHECK: llvm.mlir.global external @_QMtest_0Edata_int() {{{.*}}test = "string_attribute_maintained"{{.*}}} : i32 {
+// CHECK: [[CST0:%.*]] = llvm.mlir.constant(10 : i32) : i32
+// CHECK: llvm.return [[CST0]] : i32
+// CHECK: }
+// READ-OUT: fir.global @_QMtest_0Edata_int {test = "string_attribute_maintained"} : i32 {
+// READ-OUT: %c10_i32 = arith.constant 10 : i32
+// READ-OUT: fir.has_value %c10_i32 : i32
+// READ-OUT: }
+fir.global @_QMtest_0Edata_int {test = "string_attribute_maintained"} : i32 {
+  %c10_i32 = arith.constant 10 : i32
+  fir.has_value %c10_i32 : i32
+}