[Mlir-commits] [mlir] [MLIR][LLVM] Fix uses of LLVM's visibility attr (PR #173024)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-llvm

Author: Asher Mancinelli (ashermancinelli)

<details>
<summary>Changes</summary>

I noticed that I was using LLVM::VisibilityAttr::name when looking for the attribute on LLVM ops, but LLVM::VisibilityAttr::name is just "builtin.integer"... Now I look specifically for "visibility_", but it seems like this should be in an interface method. This change does what I need, but I fear that small changes to the LLVM ops will break this pass absent something like LLVMGlobalValueInterface that lets me ask for the LLVM visibility (and not the MLIR symbol table visibility).

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


2 Files Affected:

- (modified) mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp (+3-2) 
- (modified) mlir/test/Dialect/LLVMIR/use-default-visibility.mlir (+16-16) 


``````````diff
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
index 767fc0799026b..405b632293333 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
@@ -22,11 +22,12 @@ using namespace mlir;
 
 static void updateVisibility(Operation *op,
                              LLVM::VisibilityAttr newVisibilityAttr) {
+  static constexpr char visibilityAttrName[] = "visibility_";
   if (auto visibilityAttr =
-          op->getAttrOfType<LLVM::VisibilityAttr>(LLVM::VisibilityAttr::name)) {
+          op->getAttrOfType<LLVM::VisibilityAttr>(visibilityAttrName)) {
     LLVM::Visibility visibility = visibilityAttr.getValue();
     if (visibility == LLVM::Visibility::Default) {
-      op->setAttr(LLVM::VisibilityAttr::name, newVisibilityAttr);
+      op->setAttr(visibilityAttrName, newVisibilityAttr);
     }
   }
 }
diff --git a/mlir/test/Dialect/LLVMIR/use-default-visibility.mlir b/mlir/test/Dialect/LLVMIR/use-default-visibility.mlir
index 37c75e52debcd..112c465b5514a 100644
--- a/mlir/test/Dialect/LLVMIR/use-default-visibility.mlir
+++ b/mlir/test/Dialect/LLVMIR/use-default-visibility.mlir
@@ -78,27 +78,27 @@ llvm.mlir.ifunc external protected @ifunc3: !llvm.func<void ()>, !llvm.ptr @ifun
 // HIDDEN:           llvm.comdat_selector @any any
 // HIDDEN:         }
 
-// HIDDEN-LABEL:   llvm.func @func() {
+// HIDDEN-LABEL:   llvm.func hidden @func() {
 // HIDDEN:           llvm.return
 // HIDDEN:         }
 
-// HIDDEN-LABEL:   llvm.func @ifunc_resolver() -> !llvm.ptr {
+// HIDDEN-LABEL:   llvm.func hidden @ifunc_resolver() -> !llvm.ptr {
 // HIDDEN:           %[[MLIR_0:.*]] = llvm.mlir.addressof @func : !llvm.ptr
 // HIDDEN:           llvm.return %[[MLIR_0]] : !llvm.ptr
 // HIDDEN:         }
 
-// HIDDEN-LABEL:   llvm.func @func1() {
+// HIDDEN-LABEL:   llvm.func hidden @func1() {
 // HIDDEN:           llvm.return
 // HIDDEN:         }
-// HIDDEN:         llvm.mlir.global internal constant @global1(0 : i32) {addr_space = 0 : i32} : i32
+// HIDDEN:         llvm.mlir.global internal hidden constant @global1(0 : i32) {addr_space = 0 : i32} : i32
 
-// HIDDEN-LABEL:   llvm.mlir.alias external @func1_alias {addr_space = 0 : i32} : !llvm.ptr {
+// HIDDEN-LABEL:   llvm.mlir.alias external hidden @func1_alias {addr_space = 0 : i32} : !llvm.ptr {
 // HIDDEN:           %[[MLIR_0:.*]] = llvm.mlir.addressof @func1 : !llvm.ptr
 // HIDDEN:           llvm.return %[[MLIR_0]] : !llvm.ptr
 // HIDDEN:         }
-// HIDDEN:         llvm.func @decl1()
-// HIDDEN:         llvm.mlir.global internal constant @comdat1(1 : i64) comdat(@llvm_comdat::@any) {addr_space = 0 : i32} : i64
-// HIDDEN:         llvm.mlir.ifunc external @ifunc1 : !llvm.func<void ()>, !llvm.ptr @ifunc_resolver
+// HIDDEN:         llvm.func hidden @decl1()
+// HIDDEN:         llvm.mlir.global internal hidden constant @comdat1(1 : i64) comdat(@llvm_comdat::@any) {addr_space = 0 : i32} : i64
+// HIDDEN:         llvm.mlir.ifunc external hidden @ifunc1 : !llvm.func<void ()>, !llvm.ptr @ifunc_resolver
 
 // HIDDEN-LABEL:   llvm.func hidden @func2() {
 // HIDDEN:           llvm.return
@@ -130,27 +130,27 @@ llvm.mlir.ifunc external protected @ifunc3: !llvm.func<void ()>, !llvm.ptr @ifun
 // PROTECTED:           llvm.comdat_selector @any any
 // PROTECTED:         }
 
-// PROTECTED-LABEL:   llvm.func @func() {
+// PROTECTED-LABEL:   llvm.func protected @func() {
 // PROTECTED:           llvm.return
 // PROTECTED:         }
 
-// PROTECTED-LABEL:   llvm.func @ifunc_resolver() -> !llvm.ptr {
+// PROTECTED-LABEL:   llvm.func protected @ifunc_resolver() -> !llvm.ptr {
 // PROTECTED:           %[[MLIR_0:.*]] = llvm.mlir.addressof @func : !llvm.ptr
 // PROTECTED:           llvm.return %[[MLIR_0]] : !llvm.ptr
 // PROTECTED:         }
 
-// PROTECTED-LABEL:   llvm.func @func1() {
+// PROTECTED-LABEL:   llvm.func protected @func1() {
 // PROTECTED:           llvm.return
 // PROTECTED:         }
-// PROTECTED:         llvm.mlir.global internal constant @global1(0 : i32) {addr_space = 0 : i32} : i32
+// PROTECTED:         llvm.mlir.global internal protected constant @global1(0 : i32) {addr_space = 0 : i32} : i32
 
-// PROTECTED-LABEL:   llvm.mlir.alias external @func1_alias {addr_space = 0 : i32} : !llvm.ptr {
+// PROTECTED-LABEL:   llvm.mlir.alias external protected @func1_alias {addr_space = 0 : i32} : !llvm.ptr {
 // PROTECTED:           %[[MLIR_0:.*]] = llvm.mlir.addressof @func1 : !llvm.ptr
 // PROTECTED:           llvm.return %[[MLIR_0]] : !llvm.ptr
 // PROTECTED:         }
-// PROTECTED:         llvm.func @decl1()
-// PROTECTED:         llvm.mlir.global internal constant @comdat1(1 : i64) comdat(@llvm_comdat::@any) {addr_space = 0 : i32} : i64
-// PROTECTED:         llvm.mlir.ifunc external @ifunc1 : !llvm.func<void ()>, !llvm.ptr @ifunc_resolver
+// PROTECTED:         llvm.func protected @decl1()
+// PROTECTED:         llvm.mlir.global internal protected constant @comdat1(1 : i64) comdat(@llvm_comdat::@any) {addr_space = 0 : i32} : i64
+// PROTECTED:         llvm.mlir.ifunc external protected @ifunc1 : !llvm.func<void ()>, !llvm.ptr @ifunc_resolver
 
 // PROTECTED-LABEL:   llvm.func hidden @func2() {
 // PROTECTED:           llvm.return

``````````

</details>


https://github.com/llvm/llvm-project/pull/173024