-
Notifications
You must be signed in to change notification settings - Fork 12.1k
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
Merged
Merged
Changes from 7 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
df3f8df
wip: Move instrumentation passes
pasko 3ea4de1
Address the first round of review comments
pasko c3d3637
Address 2nd round of comments
pasko 5d46226
Run prelink instrumentation without LTO, plus tests
pasko 5cabc69
Address review comments for the new tests
pasko e9c64d5
Remove a couple of clang tests
pasko d095f41
Add one more -O0 check in pre-inliner
pasko e5da299
Revert "Remove a couple of clang tests"
pasko 1a4e5fb
Address comments in new tests
pasko ee48568
Merge branch 'main' into the instrumentation WIP PR
pasko fd92b90
For recovered tests in Clang only check attributes and disable llvm p…
pasko b65cec9
Convert more Clang tests to only check attributes
pasko 9996036
Merge branch 'main'
pasko 1abdb86
Run clang/test/CodeGen/... instrumentation tests only with -disable-l…
pasko 27359fe
Move EntryExitInstrumenterPass to EarlyFPM
pasko bee1fb2
remove unnecessary condition
pasko e3f2097
Adjust passes in tests
pasko 3579930
Whitespace beautify
pasko 9270233
Add the pass in llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
pasko 34658c6
Merge 'main'
pasko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
; RUN: llc -mtriple=x86_64-- -O0 < %s | FileCheck %s | ||
; RUN: llc -mtriple=x86_64-- -O1 < %s | FileCheck %s | ||
; RUN: llc -mtriple=x86_64-- -O2 < %s | FileCheck %s | ||
|
||
; The codegen should insert post-inlining instrumentation calls and should not | ||
; insert pre-inlining instrumentation. | ||
|
||
; CHECK-NOT: callq __cyg_profile_func_enter | ||
|
||
define void @leaf_function() #0 { | ||
; CHECK-LABEL: leaf_function: | ||
; CHECK: callq __cyg_profile_func_enter_bare | ||
; CHECK: {{.*}} %rdi | ||
; CHECK-NEXT: callq __cyg_profile_func_exit | ||
ret void | ||
} | ||
|
||
define void @root_function() #0 { | ||
entry: | ||
; CHECK-LABEL: root_function: | ||
; CHECK: callq __cyg_profile_func_enter_bare | ||
; CHECK-NEXT: callq leaf_function | ||
; CHECK: {{.*}} %rdi | ||
; CHECK-NEXT: callq __cyg_profile_func_exit | ||
call void @leaf_function() | ||
ret void | ||
} | ||
|
||
attributes #0 = { "instrument-function-entry"="__cyg_profile_func_enter" "instrument-function-entry-inlined"="__cyg_profile_func_enter_bare" "instrument-function-exit-inlined"="__cyg_profile_func_exit" } |
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
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
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
Oops, something went wrong.
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.
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.