-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
gh-130090: Support PGO for clang-cl #129907
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
Merged
+99
−5
Merged
Changes from 3 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
859c89f
minimal changes needed for PGO with clang-cl
chris-eibl a529c39
like for MSVC, don't use link time optimization
chris-eibl 26fb51f
Do not build _freeze_module twice in case of PGO
chris-eibl 6e2cb69
remove /arch:AVX for blake2module.c again
chris-eibl 7b46aeb
Revert "Do not build _freeze_module twice in case of PGO"
chris-eibl ced66bd
always clean the profile directory when doing a new clang PGO build
chris-eibl 1aae65d
Merge remote-tracking branch 'origin/main' into clang-pgo
chris-eibl 52fd5ab
Merge branch 'main' into clang-pgo
chris-eibl c2ecb57
blurb it
chris-eibl ad72df5
Adding myself to ACKS
chris-eibl 74ec74e
extract pyproject-clangcl.props
chris-eibl 79ce418
Merge branch 'python:main' into clang-pgo
chris-eibl 8c8aa79
move MergeClangProfileData to pyproject-clangcl.props
chris-eibl 2a9da27
rename RequirePGCFiles to RequireProfileData
chris-eibl ea4de96
Apply suggestions from code review
chris-eibl 3346b9d
Use -Wno-profile-instr-unprofiled in the PGUpdate case
chris-eibl 9db1a29
introduce CLANG_PROFILE_PATH
chris-eibl 4ad2365
update readme.txt
chris-eibl 1977953
Apply suggestions from code review
chris-eibl ad2dfce
Apply suggestions from code review
chris-eibl 6edbe07
credit zooba
chris-eibl 81b4c1d
address review comments regarding PGO
chris-eibl 263870d
Explicitely set the architecture based on $(Platform)
chris-eibl db208ae
Update PCbuild/readme.txt
chris-eibl 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
2 changes: 1 addition & 1 deletion
2
Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst
This file contains hidden or 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 |
---|---|---|
@@ -1,2 +1,2 @@ | ||
Building with ``PlatformToolset=ClangCL`` on Windows now supports PGO | ||
(profile guided optimization). Patch by Chris Eibl. | ||
(profile guided optimization). Patch by Chris Eibl with invaluable support from Steve Dover. |
This file contains hidden or 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 hidden or 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.
Is there an ARM64 option as well?
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'm glad you found this option, though! Much neater way to do it (and a bit embarrassing that the built-in targets don't do 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.
I think so, yes. I see some hits in the net, and did a
clang-cl /?
, which e.g. reveals option/arm64EC
.But I do not know what to actually write here in case of
'$(Platform)'=='ARM64'
- and more importantly how to test it, since I have no arm based device.I think we should leave that for another PR of someone who does?
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.
Sure. @brandtbucher might be that someone. But at the conditions here will just not set an option on that platform and so it should work the same as today.