-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Add IRPGO and CSIRPGO options to Swift #84335
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
base: main
Are you sure you want to change the base?
Conversation
Updated config
I don't have a strong opinion on naming. My feeling is that context sensitive and non context sensitive LLVM IR level instrumentation should follow a similar pattern to make clear they are related. We have already deviated from clang's naming scheme -- that is, clang's |
You could add LLVM IR level tests that test the functionality of this PR (look at most tests in
Preferably, you would as That would look something like the following (I ran
|
Thanks for the guidance here, it definitely helped me understand things better! |
Added tests
f5c8119
to
fb55618
Compare
Option dedicated for IR level PGO uses
2c32821
to
83f4c14
Compare
Hi @aschwaighofer, I've added the tests based on your suggestions. When you have a moment, could you please review the changes and share your feedback? Thanks |
@kavon would love to hear your perspective on this PR as well, when you have a moment. |
@swift-ci test |
This PR introduces three new instrumentation flags and plumbs them through to IRGen:
-ir-profile-generate
- enable IR-level instrumentation.-cs-profile-generate
- enable context-sensitive IR-level instrumentation.-ir-profile-use
- IR-level PGO input profdata file to enable profile-guided optimization (both IRPGO and CSIRPGO)Context: https://forums.swift.org/t/ir-level-pgo-instrumentation-in-swift/82123
Swift-driver PR: swiftlang/swift-driver#1992
Tests and validation:
This PR includes ir level verification tests, also checks few edge-cases when
-ir-profile-use
supplied profile is either missing or is an invalid IR profile.However, for argument validation, linking, and generating IR profiles that can later be consumed by -cs-profile-generate, I’ll need corresponding swift-driver changes. Those changes are being tracked in swiftlang/swift-driver#1992
The plan is:
Open Questions (Reviewer Input Requested)1. Naming:Is
-ir-profile-generate
a good flag name?-cs-profile-generate
pairs nicely with Clang’s-fcs-profile-generate
. However,-ir-profile-generate
feels closer to Clang's-fprofile-generate
but-profile-generate
already exists in Swift as a frontend level and not IR level. Should it stay as is, or is there a clearer alternative?2. Missing LLVM runtime + Blocked on testsAt the moment, I haven’t added tests because they currently fail locally due to missing LLVM runtime symbols. This is expected, since the required runtime linking through libclang APIs will be handled in swift-driver(swiftlang/swift-driver#1992). However, swift-driver itself depends on these new options being available in Swift.Does that mean the correct order of the PRs should be the following?1. Create a PR in Swift that introduces just the new options, and land that PR first.2. Land the corresponding swift-driver PR to handle runtime linking,
3. Then follow up with a Swift PR adding the full functionality and tests?
~~