[MLIR] Add and propagate section attribute for LLVM_GlobalOp

[ranjithkh]

Summary

Add a section attribute to LLVM_GlobalOp, during module translation attribute value is propagated to llvm

[mehdi_amini]

Can you add the import side (LLVM->MLIR) as well?

[rriddle]

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

493

Drop the trivial braces here and throughout the commit.

[mehdi_amini]

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

1003

With this builder by the way? This seems to handle only a subset of the members exposed by this op.

[ftynse]

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

1003

We also don't need builders for all combinations of infrequent attributes. Something like auto op = builder.create<...>(...) followed by op.sectionAttr(value-to-set) works just fine.

[ranjithkh]

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

1003

Should I remove this builder or is this good to land?

[ftynse]

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

1003

If nobody is using it, let's remove and avoid having dead code in the codebase.

[sgrechanik]

Looks like it's it ready to land? I'll merge it tomorrow if there are no more comments.

[mehdi_amini]

It's been widely approved I think, usually if the author does not have commit access they leave a note for us to land it for them.

[ranjithkh]

I don't have commit access. Please commit this patch
Name : Ranjith Kumar H
Email : [e-mail]

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td

def LLVM_GlobalOp : LLVM_Op<"mlir.global",
     [IsolatedFromAbove, SingleBlockImplicitTerminator<"ReturnOp">, Symbol]> {
   let arguments = (ins
     TypeAttr:$type,
     UnitAttr:$constant,
     StrAttr:$sym_name,
     Linkage:$linkage,
     OptionalAttr<AnyAttr>:$value,
     DefaultValuedAttr<Confined<I32Attr, [IntNonNegative]>, "0">:$addr_space,
-    OptionalAttr<UnnamedAddr>:$unnamed_addr
+    OptionalAttr<UnnamedAddr>:$unnamed_addr,
+    OptionalAttr<StrAttr>:$section
   );
   let summary = "LLVM dialect global.";
   let description = [{
     Since MLIR allows for arbitrary operations to be present at the top level,
     global variables are defined using the `llvm.mlir.global` operation. Both
     global constants and variables can be defined, and the value may also be
     initialized in both cases.
 
let description = [{
     llvm.mlir.global @glob(0 : f32) : f32
     ```
   }];
   let regions = (region AnyRegion:$initializer);
 
   let builders = [
     OpBuilder<(ins "Type":$type, "bool":$isConstant, "Linkage":$linkage,
       "StringRef":$name, "Attribute":$value, CArg<"unsigned", "0">:$addrSpace,
-      CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs)>
+      CArg<"ArrayRef<NamedAttribute>", "{}">:$attrs)>,
+    OpBuilder<(ins "Type":$type, "bool":$isConstant, "Linkage":$linkage,
+      "StringRef":$name, "Attribute":$value, "StringAttr":$section),
+      [{
+        $_state.addAttribute("section", section);
+        build($_builder, $_state, type, isConstant, linkage, name, value);
+      }]>
   ];
 
   let extraClassDeclaration = [{
     /// Return the LLVM type of the global.
     Type getType() {
       return type();
     }
     /// Return the initializer attribute if it exists, or a null attribute.

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

if (GV->hasInitializer() && !valueAttr) {
     Value v = processConstant(GV->getInitializer());
     if (!v)
       return nullptr;
     b.create<ReturnOp>(op.getLoc(), ArrayRef<Value>({v}));
   }
   if (GV->hasAtLeastLocalUnnamedAddr())
     op.unnamed_addrAttr(UnnamedAddrAttr::get(
         context, convertUnnamedAddrFromLLVM(GV->getUnnamedAddr())));
+  if (GV->hasSection()) {
+    op.sectionAttr(b.getStringAttr(GV->getSection()));
+  }
   return globals[GV] = op;
 }
 
 Value Importer::processConstant(llvm::Constant *c) {
   OpBuilder bEntry(currentEntryBlock, currentEntryBlock->begin());
   if (Attribute attr = getConstantAsAttr(c)) {
     // These constants can be represented as attributes.
     OpBuilder b(currentEntryBlock, currentEntryBlock->begin());

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

auto *var = new llvm::GlobalVariable(
         *llvmModule, type, op.constant(), linkage,
         shouldDropGlobalInitializer(linkage, cst) ? nullptr : cst,
         op.sym_name(),
         /*InsertBefore=*/nullptr, llvm::GlobalValue::NotThreadLocal, addrSpace);
 
     if (op.unnamed_addr().hasValue())
       var->setUnnamedAddr(convertUnnamedAddrToLLVM(*op.unnamed_addr()));
 
+    if (op.section().hasValue()) {
+      var->setSection(*op.section());
+    }
+
     globalsMapping.try_emplace(op, var);
   }
 
   // Convert global variable bodies. This is done after all global variables
   // have been created in LLVM IR because a global body may refer to another
   // global or itself. So all global variables need to be mapped first.
   for (auto op : getModuleBody(mlirModule).getOps<LLVM::GlobalOp>()) {
     if (Block *initializer = op.getInitializerBlock()) {

mlir/test/Dialect/LLVMIR/global.mlir

 }
 
 // CHECK: llvm.mlir.global private local_unnamed_addr constant @local(42 : i64) : i64
 llvm.mlir.global private local_unnamed_addr constant @local(42 : i64) : i64
 
 // CHECK: llvm.mlir.global private unnamed_addr constant @foo(42 : i64) : i64
 llvm.mlir.global private unnamed_addr constant @foo(42 : i64) : i64
 
+// CHECK: llvm.mlir.global internal constant @sectionvar("teststring")  {section = ".mysection"}
+llvm.mlir.global internal constant @sectionvar("teststring")  {section = ".mysection"}: !llvm.array<10 x i8>
+
 // -----
 
 // expected-error @+1 {{requires string attribute 'sym_name'}}
 "llvm.mlir.global"() ({}) {type = i64, constant, value = 42 : i64} : () -> ()
 
 // -----
 
 // expected-error @+1 {{requires attribute 'type'}}

mlir/test/Target/LLVMIR/import.ll

 ; CHECK: llvm.mlir.global private constant @no_unnamed_addr(42 : i64) : i64
 @no_unnamed_addr = private constant i64 42
 ; CHECK: llvm.mlir.global private local_unnamed_addr constant @local_unnamed_addr(42 : i64) : i64
 @local_unnamed_addr = private local_unnamed_addr constant i64 42
 ; CHECK: llvm.mlir.global private unnamed_addr constant @unnamed_addr(42 : i64) : i64
 @unnamed_addr = private unnamed_addr constant i64 42
 
 ;
+; Section attribute
+;
+
+; CHECK: llvm.mlir.global internal constant @sectionvar("teststring")  {section = ".mysection"}
+@sectionvar = internal constant [10 x i8] c"teststring", section ".mysection"
+
+;
 ; Sequential constants.
 ;
 
 ; CHECK: llvm.mlir.global internal constant @vector_constant(dense<[1, 2]> : vector<2xi32>) : vector<2xi32>
 @vector_constant = internal constant <2 x i32> <i32 1, i32 2>
 ; CHECK: llvm.mlir.global internal constant @array_constant(dense<[1.000000e+00, 2.000000e+00]> : tensor<2xf32>) : !llvm.array<2 x f32>
 @array_constant = internal constant [2 x float] [float 1., float 2.]
 ; CHECK: llvm.mlir.global internal constant @nested_array_constant(dense<[{{\[}}1, 2], [3, 4]]> : tensor<2x2xi32>) : !llvm.array<2 x array<2 x i32>>

mlir/test/Target/LLVMIR/llvmir.mlir

 // CHECK: @no_unnamed_addr = private constant i64 42
 llvm.mlir.global private constant @no_unnamed_addr(42 : i64) : i64
 // CHECK: @local_unnamed_addr = private local_unnamed_addr constant i64 42
 llvm.mlir.global private local_unnamed_addr constant @local_unnamed_addr(42 : i64) : i64
 // CHECK: @unnamed_addr = private unnamed_addr constant i64 42
 llvm.mlir.global private unnamed_addr constant @unnamed_addr(42 : i64) : i64
 
 //
+// Section attribute.
+//
+
+// CHECK: @sectionvar = internal constant [10 x i8] c"teststring", section ".mysection"
+llvm.mlir.global internal constant @sectionvar("teststring")  {section = ".mysection"}: !llvm.array<10 x i8>
+
+//
 // Declarations of the allocation functions to be linked against. These are
 // inserted before other functions in the module.
 //
 
 // CHECK: declare i8* @malloc(i64)
 llvm.func @malloc(i64) -> !llvm.ptr<i8>
 // CHECK: declare void @free(i8*)