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.
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
Support building against llvm15 #1035
Support building against llvm15 #1035
Changes from 2 commits
2a0f69b
d6957e1
36ae6cd
34406e8
6206a41
247fa16
64bb499
bec8eec
a585985
cdd4c5f
2d5d8ee
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to other reviewers - this does result in opaque pointers being disabled for the global context multiple times, but this operation is idempotent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular, this defines the
LLVM_VERSION_MAJOR
macro referenced in various places in this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest being consistent with the #ifs, either all "<= 14" or all "< 15"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto with
ArgumentPromotionPass
in NPM as belowThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean legacy version of this pass is available as well? I couldn't find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I meant the new pass version, I wrote both of these comments together then saw the legacy pass still existed.
On that note, the NPM setup slots in differently (using pipeline tuning options) so we can’t replace these in-situ. No needed for this change though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference the loop unswitch pass has been renamed to
SimpleLoopUnswitchPass
in the new pass manager (https://reviews.llvm.org/D124376). Naturally to enable this we need to move to NPM so it makes sense that this is disabled for now but it can come back with NPM.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually
createSimpleLoopUnswitchLegacyPass
still exists in LLVM-15 so it can be dropped in here without issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding this, I have updated the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLVM 14 defaults to the soft-float ABI, LLVM 15 defaults to the hard-float ABI. We explicitly specify the soft-float ABI for consistency with the expected output. Note that hard float and hard double ABIs are tested in the subsequent two tests below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Max patch versions are 14.0.6 and 15.0.7)