[CIR][NFC] Add helper functions for CIR visibility #1652
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fangyi-zhou
commented
May 29, 2025
| @@ -126,6 +127,43 @@ template <typename Int> inline bool isValidCIRAtomicOrderingCABI(Int I) { | |||
| I <= (Int)cir::MemOrder::SequentiallyConsistent; | |||
| } | |||
|
|
|||
| /// This function is used to deduce the MLIR visibility of a CIR symbol based on | |||
| /// CIR linkage and visibility. | |||
| static inline mlir::SymbolTable::Visibility | |||
There was a problem hiding this comment.
note: I'm not sure whether this is the best place to put this function. Let me know if I should move it somewhere else.
| @@ -485,7 +486,8 @@ CIRGenModule::getOrCreateStaticVarDecl(const VarDecl &D, | |||
| cir::GlobalOp GV = builder.createVersionedGlobal( | |||
| getModule(), getLoc(D.getLocation()), Name, LTy, false, Linkage, AS); | |||
| // TODO(cir): infer visibility from linkage in global op builder. | |||
| GV.setVisibility(getMLIRVisibilityFromCIRLinkage(Linkage)); | |||
| GV.setVisibility( | |||
| cir::deduceMLIRVisibility(Linkage, cir::VisibilityKind::Default)); | |||
There was a problem hiding this comment.
My suggestion is a bit more simple and somewhat different: GV.setLinkage and Fn.setLinkage should call GV.setVisibility under the hood, no need to involve visibility kind. If you make linkage into an interface, than you don't need to implement in both places.
There was a problem hiding this comment.
Thanks for the comment! I was trying to do that in future PRs, but I ran into some issues when dealing with FuncOp (#1029 (comment))
lanza
force-pushed
the
main
branch
2 times, most recently
from
d2c4ab8 to
8f89224
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains 2 changes:
CIRGlobalValueInterface, which is implemented byGlobalOpandFuncOpwith linkage and CIR visibility. The getters/setters are analogous to existing getters/setters for linkage and symbol visibility.more notes in #1029 (comment)