Skip to content

Conversation

@xlauko
Copy link
Contributor

@xlauko xlauko commented Dec 6, 2025

No description provided.

@xlauko
Copy link
Contributor Author

xlauko commented Dec 6, 2025

@xlauko xlauko marked this pull request as ready for review December 6, 2025 22:35
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Dec 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2025

@llvm/pr-subscribers-clangir

Author: Henrich Lauko (xlauko)

Changes

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

1 Files Affected:

  • (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+5-7)
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 97bd3cf850daa..cf1a7dcd77ecd 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2015,9 +2015,8 @@ mlir::LogicalResult CIRToLLVMFuncOpLowering::matchAndRewrite(
     fn.setAlwaysInline(*inlineKind == cir::InlineKind::AlwaysInline);
   }
 
-  fn.setVisibility_Attr(mlir::LLVM::VisibilityAttr::get(
-      getContext(), lowerCIRVisibilityToLLVMVisibility(
-                        op.getGlobalVisibilityAttr().getValue())));
+  fn.setVisibility_(lowerCIRVisibilityToLLVMVisibility(
+                        op.getGlobalVisibility()));
 
   rewriter.inlineRegionBefore(op.getBody(), fn.getBody(), fn.end());
   if (failed(rewriter.convertRegionTypes(&fn.getBody(), *typeConverter,
@@ -2161,14 +2160,13 @@ mlir::LogicalResult CIRToLLVMGlobalOpLowering::matchAndRewrite(
     }
   }
 
-  // Rewrite op.
+  mlir::LLVM::Visibility visibility = lowerCIRVisibilityToLLVMVisibility(
+      op.getGlobalVisibility());
   mlir::SymbolRefAttr comdatAttr = getComdatAttr(op, rewriter);
   auto newOp = rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>(
       op, llvmType, isConst, linkage, symbol, init.value_or(mlir::Attribute()),
       alignment, addrSpace, isDsoLocal, isThreadLocal, comdatAttr, attributes);
-  newOp.setVisibility_Attr(mlir::LLVM::VisibilityAttr::get(
-      getContext(), lowerCIRVisibilityToLLVMVisibility(
-                        op.getGlobalVisibilityAttr().getValue())));
+  newOp.setVisibility_(visibility);
 
   return mlir::success();
 }

@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2025

@llvm/pr-subscribers-clang

Author: Henrich Lauko (xlauko)

Changes

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

1 Files Affected:

  • (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+5-7)
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 97bd3cf850daa..cf1a7dcd77ecd 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2015,9 +2015,8 @@ mlir::LogicalResult CIRToLLVMFuncOpLowering::matchAndRewrite(
     fn.setAlwaysInline(*inlineKind == cir::InlineKind::AlwaysInline);
   }
 
-  fn.setVisibility_Attr(mlir::LLVM::VisibilityAttr::get(
-      getContext(), lowerCIRVisibilityToLLVMVisibility(
-                        op.getGlobalVisibilityAttr().getValue())));
+  fn.setVisibility_(lowerCIRVisibilityToLLVMVisibility(
+                        op.getGlobalVisibility()));
 
   rewriter.inlineRegionBefore(op.getBody(), fn.getBody(), fn.end());
   if (failed(rewriter.convertRegionTypes(&fn.getBody(), *typeConverter,
@@ -2161,14 +2160,13 @@ mlir::LogicalResult CIRToLLVMGlobalOpLowering::matchAndRewrite(
     }
   }
 
-  // Rewrite op.
+  mlir::LLVM::Visibility visibility = lowerCIRVisibilityToLLVMVisibility(
+      op.getGlobalVisibility());
   mlir::SymbolRefAttr comdatAttr = getComdatAttr(op, rewriter);
   auto newOp = rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>(
       op, llvmType, isConst, linkage, symbol, init.value_or(mlir::Attribute()),
       alignment, addrSpace, isDsoLocal, isThreadLocal, comdatAttr, attributes);
-  newOp.setVisibility_Attr(mlir::LLVM::VisibilityAttr::get(
-      getContext(), lowerCIRVisibilityToLLVMVisibility(
-                        op.getGlobalVisibilityAttr().getValue())));
+  newOp.setVisibility_(visibility);
 
   return mlir::success();
 }

@github-actions
Copy link

github-actions bot commented Dec 6, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@AmrDeveloper AmrDeveloper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xlauko
Copy link
Contributor Author

xlauko commented Dec 8, 2025

Merge activity

  • Dec 8, 3:14 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Dec 8, 3:16 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 3:19 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 3:22 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 3:26 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 3:29 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 3:49 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 3:54 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 4:17 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 4:21 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 4:24 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 4:29 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 4:32 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 8, 4:41 PM UTC: Graphite couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'Build and Test Linux', 'Build and Test Linux AArch64').

@xlauko xlauko force-pushed the users/xlauko/visibility-setters branch 12 times, most recently from cb17dda to ed27a1b Compare December 8, 2025 16:32
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

🐧 Linux x64 Test Results

  • 112638 tests passed
  • 4087 tests skipped

✅ The build succeeded and all tests passed.

Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@xlauko xlauko force-pushed the users/xlauko/visibility-setters branch from ed27a1b to e5f29e3 Compare December 8, 2025 20:24
@xlauko xlauko merged commit 89bc5ff into main Dec 8, 2025
10 checks passed
@xlauko xlauko deleted the users/xlauko/visibility-setters branch December 8, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants