Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of LLVM function attributes #49597

Merged
merged 2 commits into from
May 2, 2023
Merged

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented May 2, 2023

Improves #49551: Instead of copying attributes manually, just ensure we always call CloneFunctionInto, as it does useful work for declarations too (bailing out early, https://github.com/llvm/llvm-project/blob/33017e5a3fa2c3194522565cd0e106a931b072b3/llvm/lib/Transforms/Utils/CloneFunction.cpp#L144-L147). This includes mapping of the personality function, https://github.com/llvm/llvm-project/blob/33017e5a3fa2c3194522565cd0e106a931b072b3/llvm/lib/Transforms/Utils/CloneFunction.cpp#L111-L115, so we can get rid of that too.

x-ref #49551 (comment)

@maleadt maleadt added compiler:codegen Generation of LLVM IR and native code backport 1.8 Change should be backported to release-1.8 backport 1.9 Change should be backported to release-1.9 labels May 2, 2023
@maleadt maleadt requested a review from vtjnash May 2, 2023 12:30
@@ -410,9 +408,6 @@ bool removeAddrspaces(Module &M, AddrspaceRemapFunction ASRemapper)
}
NF->setAttributes(Attrs);

if (F->hasPersonalityFn())
NF->setPersonalityFn(MapValue(F->getPersonalityFn(), VMap));
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

looks like this was indeed fixed 8 years ago llvm/llvm-project@2ac0c27

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label May 2, 2023
@vtjnash vtjnash merged commit a8474dc into master May 2, 2023
@vtjnash vtjnash deleted the tb/llvm_attributes_bis branch May 2, 2023 14:50
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label May 2, 2023
@KristofferC KristofferC mentioned this pull request May 8, 2023
51 tasks
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.8 Change should be backported to release-1.8 compiler:codegen Generation of LLVM IR and native code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants