[Mlir-commits] [mlir] [MLIR][LLVM] Fix uses of LLVM's visibility attr (PR #173024)
Asher Mancinelli
llvmlistbot at llvm.org
Fri Dec 19 09:24:17 PST 2025
https://github.com/ashermancinelli updated https://github.com/llvm/llvm-project/pull/173024
>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 1/3] [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
>From 08ab499d33c54a9e380d1ac5e8367fd9e10bcbb3 Mon Sep 17 00:00:00 2001
From: Asher Mancinelli <ashermancinelli at gmail.com>
Date: Fri, 19 Dec 2025 09:00:30 -0800
Subject: [PATCH 2/3] Use LLVM visibility setters/getters
---
.../Transforms/UseDefaultVisibilityPass.cpp | 26 ++++++-------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
index 405b632293333..b5e721b2bc0f9 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
@@ -10,6 +10,7 @@
#include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/Pass/Pass.h"
+#include "llvm/ADT/TypeSwitch.h"
namespace mlir {
namespace LLVM {
@@ -20,18 +21,6 @@ namespace LLVM {
using namespace mlir;
-static void updateVisibility(Operation *op,
- LLVM::VisibilityAttr newVisibilityAttr) {
- static constexpr char visibilityAttrName[] = "visibility_";
- if (auto visibilityAttr =
- op->getAttrOfType<LLVM::VisibilityAttr>(visibilityAttrName)) {
- LLVM::Visibility visibility = visibilityAttr.getValue();
- if (visibility == LLVM::Visibility::Default) {
- op->setAttr(visibilityAttrName, newVisibilityAttr);
- }
- }
-}
-
namespace {
class UseDefaultVisibilityPass
: public LLVM::impl::LLVMUseDefaultVisibilityPassBase<
@@ -42,13 +31,14 @@ class UseDefaultVisibilityPass
void runOnOperation() override {
LLVM::Visibility useDefaultVisibility = useVisibility.getValue();
Operation *op = getOperation();
- MLIRContext *context = op->getContext();
- Dialect *llvmDialect = context->getLoadedDialect<LLVM::LLVMDialect>();
- auto newVisibilityAttr =
- LLVM::VisibilityAttr::get(context, useDefaultVisibility);
op->walk([&](Operation *op) {
- if (op->getDialect() == llvmDialect)
- updateVisibility(op, newVisibilityAttr);
+ llvm::TypeSwitch<Operation *, void>(op)
+ .Case<LLVM::LLVMFuncOp, LLVM::GlobalOp, LLVM::IFuncOp, LLVM::AliasOp>(
+ [&](auto op) {
+ if (op.getVisibility_() == LLVM::Visibility::Default) {
+ op.setVisibility_(useDefaultVisibility);
+ }
+ });
});
}
};
>From ebbb33112cd0b034fb9f115ce993aea304d9cf0e Mon Sep 17 00:00:00 2001
From: Asher Mancinelli <ashermancinelli at gmail.com>
Date: Fri, 19 Dec 2025 09:23:46 -0800
Subject: [PATCH 3/3] Early exit
---
.../Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
index b5e721b2bc0f9..248400428cfc0 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/UseDefaultVisibilityPass.cpp
@@ -30,14 +30,15 @@ class UseDefaultVisibilityPass
public:
void runOnOperation() override {
LLVM::Visibility useDefaultVisibility = useVisibility.getValue();
+ if (useDefaultVisibility == LLVM::Visibility::Default)
+ return;
Operation *op = getOperation();
op->walk([&](Operation *op) {
llvm::TypeSwitch<Operation *, void>(op)
.Case<LLVM::LLVMFuncOp, LLVM::GlobalOp, LLVM::IFuncOp, LLVM::AliasOp>(
[&](auto op) {
- if (op.getVisibility_() == LLVM::Visibility::Default) {
+ if (op.getVisibility_() == LLVM::Visibility::Default)
op.setVisibility_(useDefaultVisibility);
- }
});
});
}
More information about the Mlir-commits
mailing list