[mlir][llvm] Add visibility attribute

[Dinistro]

Summary

This commit introduces the LLVM's visibility attribute and adds it to
both globals and functions.

Furthermore, this commit ensures that "thread_local" is printed in the
correct place and adds a test for that.

[gysit]

Thanks LGTM module nit comments.

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

1626

I would move this out of the if condition.

2117

Same as above.

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

981

nit: attribute ?

mlir/test/Dialect/LLVMIR/global.mlir

96

nit: -> types.

[Dinistro]

address reviewer comments

mlir/include/mlir/Dialect/LLVMIR/LLVMEnums.td

def UnnamedAddr : LLVM_EnumAttr<
     "UnnamedAddr",
     "::llvm::GlobalValue::UnnamedAddr",
     "LLVM GlobalValue UnnamedAddr",
     [UnnamedAddrNone, UnnamedAddrLocal, UnnamedAddrGlobal]> {
   let cppNamespace = "::mlir::LLVM";
 }
 
 //===----------------------------------------------------------------------===//
+// Visibility
+//===----------------------------------------------------------------------===//
+
+def VisibilityDefault
+    : LLVM_EnumAttrCase<"Default", "", "DefaultVisibility", 0>;
+def VisibilityHidden
+    : LLVM_EnumAttrCase<"Hidden", "hidden", "HiddenVisibility", 1>;
+def VisibilityProtected
+    : LLVM_EnumAttrCase<"Protected", "protected", "ProtectedVisibility", 2>;
+
+def Visibility : LLVM_EnumAttr<
+    "Visibility",
+    "::llvm::GlobalValue::VisibilityTypes",
+    "LLVM GlobalValue Visibility",
+    [VisibilityDefault, VisibilityHidden, VisibilityProtected]> {
+  let cppNamespace = "::mlir::LLVM";
+}
+
+//===----------------------------------------------------------------------===//
 // ModRefInfo
 //===----------------------------------------------------------------------===//
 
 def ModRefInfoNoModRef : LLVM_EnumAttrCase<"NoModRef", "none", "NoModRef", 0>;
 def ModRefInfoRef : LLVM_EnumAttrCase<"Ref", "read", "Ref", 1>;
 def ModRefInfoMod : LLVM_EnumAttrCase<"Mod", "write", "Mod", 2>;
 def ModRefInfoModRef : LLVM_EnumAttrCase<"ModRef", "readwrite", "ModRef", 3>;
 

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

let arguments = (ins
     StrAttr:$sym_name,
     Linkage:$linkage,
     UnitAttr:$dso_local,
     UnitAttr:$thread_local_,
     OptionalAttr<AnyAttr>:$value,
     OptionalAttr<I64Attr>:$alignment,
     DefaultValuedAttr<ConfinedAttr<I32Attr, [IntNonNegative]>, "0">:$addr_space,
     OptionalAttr<UnnamedAddr>:$unnamed_addr,
-    OptionalAttr<StrAttr>:$section
+    OptionalAttr<StrAttr>:$section,
+    DefaultValuedAttr<Visibility, "mlir::LLVM::Visibility::Default">:$visibility_
   );
   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 arguments = (ins
     UnitAttr:$dso_local,
     DefaultValuedAttr<CConv, "CConv::C">:$CConv,
     OptionalAttr<FlatSymbolRefAttr>:$personality,
     OptionalAttr<StrAttr>:$garbageCollector,
     OptionalAttr<ArrayAttr>:$passthrough,
     OptionalAttr<DictArrayAttr>:$arg_attrs,
     OptionalAttr<DictArrayAttr>:$res_attrs,
     OptionalAttr<I64Attr>:$function_entry_count,
-    OptionalAttr<LLVM_MemoryEffectsAttr>:$memory
+    OptionalAttr<LLVM_MemoryEffectsAttr>:$memory,
+    DefaultValuedAttr<Visibility, "mlir::LLVM::Visibility::Default">:$visibility_
   );
 
   let regions = (region AnyRegion:$body);
 
   let skipDefaultBuilders = 1;
 
   let builders = [
     OpBuilder<(ins "StringRef":$name, "Type":$type,

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

if (addrSpace != 0)
     result.addAttribute(getAddrSpaceAttrName(result.name),
                         builder.getI32IntegerAttr(addrSpace));
   result.attributes.append(attrs.begin(), attrs.end());
   result.addRegion();
 }
 
 void GlobalOp::print(OpAsmPrinter &p) {
   p << ' ' << stringifyLinkage(getLinkage()) << ' ';
+  StringRef visibility = stringifyVisibility(getVisibility_());
+  if (!visibility.empty())
+    p << visibility << ' ';
+  if (getThreadLocal_())
+    p << "thread_local ";
   if (auto unnamedAddr = getUnnamedAddr()) {
     StringRef str = stringifyUnnamedAddr(*unnamedAddr);
     if (!str.empty())
       p << str << ' ';
   }
-  if (getThreadLocal_())
-    p << "thread_local ";
   if (getConstant())
     p << "constant ";
   p.printSymbolName(getSymName());
   p << '(';
   if (auto value = getValueOrNull())
     p.printAttribute(value);
   p << ')';
   // Note that the alignment attribute is printed using the
   // default syntax here, even though it is an inherent attribute
   // (as defined in https://mlir.llvm.org/docs/LangRef/#attributes)
-  p.printOptionalAttrDict(
-      (*this)->getAttrs(),
-      {SymbolTable::getSymbolAttrName(), getGlobalTypeAttrName(),
-       getConstantAttrName(), getValueAttrName(), getLinkageAttrName(),
-       getUnnamedAddrAttrName(), getThreadLocal_AttrName()});
+  p.printOptionalAttrDict((*this)->getAttrs(),
+                          {SymbolTable::getSymbolAttrName(),
+                           getGlobalTypeAttrName(), getConstantAttrName(),
+                           getValueAttrName(), getLinkageAttrName(),
+                           getUnnamedAddrAttrName(), getThreadLocal_AttrName(),
+                           getVisibility_AttrName()});
 
   // Print the trailing type unless it's a string global.
   if (getValueOrNull().dyn_cast_or_null<StringAttr>())
     return;
   p << " : " << getType();
 
   Region &initializer = getInitializerRegion();
   if (!initializer.empty()) {
#define REGISTER_ENUM_TYPE(Ty)                                                 \
   struct EnumTraits<Ty> {                                                      \
     static StringRef stringify(Ty value) { return stringify##Ty(value); }      \
     static unsigned getMaxEnumVal() { return getMaxEnumValFor##Ty(); }         \
   }
 
 REGISTER_ENUM_TYPE(Linkage);
 REGISTER_ENUM_TYPE(UnnamedAddr);
 REGISTER_ENUM_TYPE(CConv);
+REGISTER_ENUM_TYPE(Visibility);
 } // namespace
 
 /// Parse an enum from the keyword, or default to the provided default value.
 /// The return type is the enum type by default, unless overriden with the
 /// second template argument.
 template <typename EnumTy, typename RetTy = EnumTy>
 static RetTy parseOptionalLLVMKeyword(OpAsmParser &parser,
                                       OperationState &result,

 ParseResult GlobalOp::parse(OpAsmParser &parser, OperationState &result) {
   MLIRContext *ctx = parser.getContext();
   // Parse optional linkage, default to External.
   result.addAttribute(getLinkageAttrName(result.name),
                       LLVM::LinkageAttr::get(
                           ctx, parseOptionalLLVMKeyword<Linkage>(
                                    parser, result, LLVM::Linkage::External)));
 
-  if (succeeded(parser.parseOptionalKeyword("thread_local")))
-    result.addAttribute(getThreadLocal_AttrName(result.name),
-                        parser.getBuilder().getUnitAttr());
+  // Parse optional visibility, default to Default.
+  result.addAttribute(getVisibility_AttrName(result.name),
+                      parser.getBuilder().getI64IntegerAttr(
+                          parseOptionalLLVMKeyword<LLVM::Visibility, int64_t>(
+                              parser, result, LLVM::Visibility::Default)));
 
   // Parse optional UnnamedAddr, default to None.
   result.addAttribute(getUnnamedAddrAttrName(result.name),
                       parser.getBuilder().getI64IntegerAttr(
                           parseOptionalLLVMKeyword<UnnamedAddr, int64_t>(
                               parser, result, LLVM::UnnamedAddr::None)));
 
+  if (succeeded(parser.parseOptionalKeyword("thread_local")))
+    result.addAttribute(getThreadLocal_AttrName(result.name),
+                        parser.getBuilder().getUnitAttr());
+
   if (succeeded(parser.parseOptionalKeyword("constant")))
     result.addAttribute(getConstantAttrName(result.name),
                         parser.getBuilder().getUnitAttr());
 
   StringAttr name;
   if (parser.parseSymbolName(name, getSymNameAttrName(result.name),
                              result.attributes) ||
       parser.parseLParen())

 ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
   // Default to external linkage if no keyword is provided.
   result.addAttribute(
       getLinkageAttrName(result.name),
       LinkageAttr::get(parser.getContext(),
                        parseOptionalLLVMKeyword<Linkage>(
                            parser, result, LLVM::Linkage::External)));
 
+  // Parse optional visibility, default to Default.
+  result.addAttribute(getVisibility_AttrName(result.name),
+                      parser.getBuilder().getI64IntegerAttr(
+                          parseOptionalLLVMKeyword<LLVM::Visibility, int64_t>(
+                              parser, result, LLVM::Visibility::Default)));
+
   // Default to C Calling Convention if no keyword is provided.
   result.addAttribute(
       getCConvAttrName(result.name),
       CConvAttr::get(parser.getContext(), parseOptionalLLVMKeyword<CConv>(
                                               parser, result, LLVM::CConv::C)));
 
   StringAttr nameAttr;
   SmallVector<OpAsmParser::Argument> entryArgs;

 
 // Print the LLVMFuncOp. Collects argument and result types and passes them to
 // helper functions. Drops "void" result since it cannot be parsed back. Skips
 // the external linkage since it is the default value.
 void LLVMFuncOp::print(OpAsmPrinter &p) {
   p << ' ';
   if (getLinkage() != LLVM::Linkage::External)
     p << stringifyLinkage(getLinkage()) << ' ';
+  StringRef visibility = stringifyVisibility(getVisibility_());
+  if (!visibility.empty())
+    p << visibility << ' ';
   if (getCConv() != LLVM::CConv::C)
     p << stringifyCConv(getCConv()) << ' ';
 
   p.printSymbolName(getName());
 
   LLVMFunctionType fnType = getFunctionType();
   SmallVector<Type, 8> argTypes;
   SmallVector<Type, 1> resTypes;
   argTypes.reserve(fnType.getNumParams());
   for (unsigned i = 0, e = fnType.getNumParams(); i < e; ++i)
     argTypes.push_back(fnType.getParamType(i));
 
   Type returnType = fnType.getReturnType();
   if (!returnType.isa<LLVMVoidType>())
     resTypes.push_back(returnType);
 
   function_interface_impl::printFunctionSignature(p, *this, argTypes,
                                                   isVarArg(), resTypes);
   function_interface_impl::printFunctionAttributes(
       p, *this,
       {getFunctionTypeAttrName(), getArgAttrsAttrName(), getResAttrsAttrName(),
-       getLinkageAttrName(), getCConvAttrName()});
+       getLinkageAttrName(), getCConvAttrName(), getVisibility_AttrName()});
 
   // Print the body if this is not an external function.
   Region &body = getBody();
   if (!body.empty()) {
     p << ' ';
     p.printRegion(body, /*printEntryBlockArgs=*/false,
                   /*printBlockTerminators=*/true);
   }

mlir/lib/Target/LLVMIR/ModuleImport.cpp

if (globalVar->hasInitializer() && !valueAttr) {
     builder.create<ReturnOp>(globalOp.getLoc(), *initializer);
   }
   if (globalVar->hasAtLeastLocalUnnamedAddr()) {
     globalOp.setUnnamedAddr(
         convertUnnamedAddrFromLLVM(globalVar->getUnnamedAddr()));
   }
   if (globalVar->hasSection())
     globalOp.setSection(globalVar->getSection());
+  globalOp.setVisibility_(
+      convertVisibilityFromLLVM(globalVar->getVisibility()));
 
   return success();
 }
 
 LogicalResult
 ModuleImport::convertGlobalCtorsAndDtors(llvm::GlobalVariable *globalVar) {
   if (!globalVar->hasInitializer() || !globalVar->hasAppendingLinkage())
     return failure();
LogicalResult ModuleImport::processFunction(llvm::Function *func) {
   if (FlatSymbolRefAttr personality = getPersonalityAsAttr(func))
     funcOp.setPersonalityAttr(personality);
   else if (func->hasPersonalityFn())
     emitWarning(funcOp.getLoc(), "could not deduce personality, skipping it");
 
   if (func->hasGC())
     funcOp.setGarbageCollector(StringRef(func->getGC()));
 
+  funcOp.setVisibility_(convertVisibilityFromLLVM(func->getVisibility()));
+
   // Handle Function attributes.
   processFunctionAttributes(func, funcOp);
 
   // Convert non-debug metadata by using the dialect interface.
   SmallVector<std::pair<unsigned, llvm::MDNode *>> allMetadata;
   func->getAllMetadata(allMetadata);
   for (auto &[kind, node] : allMetadata) {
     if (!iface.isConvertibleMetadata(kind))

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

if (op.getSection().has_value())
       var->setSection(*op.getSection());
 
     addRuntimePreemptionSpecifier(op.getDsoLocal(), var);
 
     std::optional<uint64_t> alignment = op.getAlignment();
     if (alignment.has_value())
       var->setAlignment(llvm::MaybeAlign(alignment.value()));
 
+    var->setVisibility(convertVisibilityToLLVM(op.getVisibility_()));
+
     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()) {
for (auto [argIdx, llvmArg] : llvm::enumerate(llvmFunc->args())) {
         llvmArg.addAttrs(attrBuilder);
       }
     }
 
     // Forward the pass-through attributes to LLVM.
     if (failed(forwardPassthroughAttributes(
             function.getLoc(), function.getPassthrough(), llvmFunc)))
       return failure();
+
+    // Convert visibility attribute.
+    llvmFunc->setVisibility(convertVisibilityToLLVM(function.getVisibility_()));
   }
 
   return success();
 }
 
 LogicalResult ModuleTranslation::convertFunctions() {
   // Convert functions.
   for (auto function : getModuleBody(mlirModule).getOps<LLVMFuncOp>()) {

mlir/test/Dialect/LLVMIR/func.mlir

llvm.func @variadic_def(...) {
     llvm.return
   }
 
   // CHECK-LABEL: llvm.func @memory_attr
   // CHECK-SAME: attributes {memory = #llvm.memory_effects<other = none, argMem = read, inaccessibleMem = readwrite>} {
   llvm.func @memory_attr() attributes {memory = #llvm.memory_effects<other = none, argMem = read, inaccessibleMem = readwrite>} {
     llvm.return
   }
+
+  // CHECK-LABEL: llvm.func hidden @hidden
+  llvm.func hidden @hidden() {
+    llvm.return
+  }
+
+  // CHECK-LABEL: llvm.func protected @protected
+  llvm.func protected @protected() {
+    llvm.return
+  }
 }
 
 // -----
 
 module {
   // expected-error@+1 {{requires one region}}
   "llvm.func"() {function_type = !llvm.func<void ()>, sym_name = "no_region"} : () -> ()
 }

mlir/test/Dialect/LLVMIR/global.mlir

 llvm.mlir.global private local_unnamed_addr constant @local(42 : i64) : i64
 
 // CHECK: llvm.mlir.global private unnamed_addr constant @foo(42 : i64) {addr_space = 0 : i32} : i64
 llvm.mlir.global private unnamed_addr constant @foo(42 : i64) : i64
 
 // CHECK: llvm.mlir.global internal constant @sectionvar("teststring") {addr_space = 0 : i32, section = ".mysection"}
 llvm.mlir.global internal constant @sectionvar("teststring")  {section = ".mysection"}: !llvm.array<10 x i8>
 
+// CHECK: llvm.mlir.global internal thread_local constant @thread_local(42 : i32)
+llvm.mlir.global internal thread_local constant @thread_local(42 : i32) : i32
+
+// Visibility types.
+// CHECK: llvm.mlir.global internal hidden constant @hidden(42 : i32)
+llvm.mlir.global internal hidden constant @hidden(42 : i32) : i32
+
+// CHECK: llvm.mlir.global internal protected unnamed_addr @protected(42 : i32)
+llvm.mlir.global internal protected unnamed_addr @protected(42 : i32) : i32
+
 // -----
 
 // expected-error @+1 {{op requires attribute 'sym_name'}}
 "llvm.mlir.global"() ({}) {linkage = "private", type = i64, constant, global_type = i64, value = 42 : i64} : () -> ()
 
 // -----
 
 // expected-error @+1 {{requires attribute 'global_type'}}

mlir/test/Target/LLVMIR/Import/function-attributes.ll

 // -----
 
 ; CHECK-LABEL: @passthrough_string_only
 ; CHECK-SAME: attributes {passthrough = ["no-enum-attr"]}
 ; CHECK:   llvm.return
 define void @passthrough_string_only() "no-enum-attr" {
   ret void
 }
+
+// -----
+
+; CHECK-LABEL: llvm.func hidden @hidden()
+define hidden void @hidden() {
+  ret void
+}
+
+// -----
+
+; CHECK-LABEL: llvm.func protected @protected()
+define protected void @protected() {
+  ret void
+}

mlir/test/Target/LLVMIR/Import/global-variables.ll

 
 define void @foo() {
   ret void
 }
 
 define void @bar() {
   ret void
 }
+
+; // -----
+
+; Visibility attribute.
+
+; CHECK: llvm.mlir.global external hidden constant @hidden("string")
+@hidden = hidden constant [6 x i8] c"string"
+
+; CHECK: llvm.mlir.global external protected constant @protected(42 : i64)
+@protected = protected constant i64 42

mlir/test/Target/LLVMIR/llvmir.mlir

 llvm.mlir.global extern_weak @extern_weak() : i32
 // CHECK: @linkonce_odr = linkonce_odr global i32 42
 llvm.mlir.global linkonce_odr @linkonce_odr(42 : i32) : i32
 // CHECK: @weak_odr = weak_odr global i32 42
 llvm.mlir.global weak_odr @weak_odr(42 : i32) : i32
 // CHECK: @external = external global i32
 llvm.mlir.global external @external() : i32
 
+
+//
+// Visibility attribute.
+//
+
+// CHECK: @hidden = hidden constant [6 x i8] c"string"
+llvm.mlir.global external hidden constant @hidden("string")
+
+// CHECK: @protected = protected constant i64 42
+llvm.mlir.global external protected constant @protected(42 : i64) : i64
+
 //
 // UnnamedAddr attribute.
 //
 
 // 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: define internal void @func_internal
 llvm.func internal @func_internal() {
   llvm.return
 }
 
 //
+// Visibility attribute.
+//
+
+// CHECK-LABEL: define hidden void @hidden_func()
+llvm.func hidden @hidden_func() {
+  llvm.return
+}
+
+// CHECK-LABEL: define protected void @protected_func()
+llvm.func protected @protected_func() {
+  llvm.return
+}
+
+//
 // dso_local attribute.
 //
 
 // CHECK: define dso_local void @dso_local_func
 llvm.func @dso_local_func() attributes {dso_local} {
   llvm.return
 }