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

[Asher Mancinelli]

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

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).

>From 7c80d46a4c06bcb249b9d7843f540da6a0b3ea4a Mon Sep 17 00:00:00 2001
From: Asher Mancinelli <ashermancinelli at gmail.com>
Date: Fri, 19 Dec 2025 07:38:32 -0800
Subject: [PATCH] [MLIR][LLVM] Fix uses of LLVM's visibility attr

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.
---
 .../Transforms/UseDefaultVisibilityPass.cpp   |  5 +--
 .../LLVMIR/use-default-visibility.mlir        | 32 +++++++++----------
 2 files changed, 19 insertions(+), 18 deletions(-)

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