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
[EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines #92171
[EntryExitInstrumenter] Move passes out of clang into LLVM default pipelines #92171
Changes from 2 commits
df3f8df
3ea4de1
c3d3637
5d46226
5cabc69
e9c64d5
d095f41
e5da299
1a4e5fb
ee48568
fd92b90
b65cec9
9996036
1abdb86
27359fe
bee1fb2
e3f2097
3579930
9270233
34658c6
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.
actually is it possible to add this into
EarlyFPM
below? it's nice to have fewer module->function adaptors. (e.g. inEarlyFPM
we run all the function passes on a single function before moving to the next one which is nice for memory locality)sorry for suggesting this so late, since it'll require updating all the pipeline tests... but first check that none of the other tests fail
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.
I tried this, and all non-pipeline tests pass. Though in my local FDO+ThinLTO reproducer I saw this causes
__cyg_profile_func_enter_bare
to be inserted multiple times per toplevel function.This behaviour surprised me. The bare instrumentation does not move with the change. Is removing the module to function pass adaptor making something easier to inline later on?
I'd like to investigate, but I am afraid I'll need some clues :)
The change:
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.
the check for
Phase != ThinOrFullLTOPhase::FullLTOPostLink
is unnecessary,buildModuleSimplificationPipeline
isn't called for FullLTO post linkI'm not sure why you're seeing that behavior if we're only running the post-inline instrumenter once in the codegen pipeline
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.
if you have a small local repro, you can pass
-Wl,-mllvm,-print-after-all
to the link command to have it print IR after every pass and see where the calls are getting added. or to see changes in the pre-link optimization pipeline,-mllvm=-print-after-all
(or-mllvm=-print-changed=quiet
, which doesn't work with the IR parts of the codegen pipeline for reasons)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.
Removed. Indeed there is an assertion for this a few lines prior.
Apparently I made a mistake when building the reproducer. When checking today I saw everything working as expected, i.e. <=1 bare hooks inserted per function in the final DSO. Sorry for the noise.