[CIR][CIRGen] Add setNonAliasAttributes for GlobalOp and FuncOp #707
Conversation
This file contains 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
bcardosolopes
approved these changes
Jul 2, 2024
| } | ||
|
|
||
| if (const auto *CSA = D->getAttr<CodeSegAttr>()) { | ||
| assert(!MissingFeatures::setSectionForFuncOp()); |
There was a problem hiding this comment.
@roro47 back to our discussion about sections, perhaps this could be related with the linker problem?
bcardosolopes
changed the title
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.
In this PR:
as title we added setNonAliasAttributes in the skeleton of OG's setNonAliasAttributes, and call this function in buildGlobalFunctionDefinition after code for FuncOP is generated. This is needed for CIR OG to know FuncOP is not declaration anymore, thus giving shouldAssumeDsoLocal another run to make dso_local right.
A couple of notes about test;
having to changed driver.c, because in terms of dso_local for func, masOS is different from other targets as even in OG, as macOS is !isOSBinFormatELF(), thus even OG doesn't set dso_local for its functions.
most of functions in existing tests still not getting dso_local in LLVM yet because they fall into case of (RM != llvm::Reloc::Static && !LOpts.PIE) , which is more complicated to implement as we need to get canBenefitFromLocalAlias right. So I treated it as a missing feature and default it to false. We gonna leave it to another PR to address. In this PR, I just added additional test with -fpie option to my test so we get dso_local for functions without having to deal with this case.
Next 2 PRs:
PR1. call setNonAliasAttributes in buildGlobalVarDefinition, after initialization for GlobalOP is found, similar to FuncOp.
didn't to it in this PR as there are many more test cases needed to be fixed/added for this case.
PR2: try to implement canBenefitFromLocalAlias.