Add support for PGO to rust, fix clang pgo - #13396
Open
dcbaker wants to merge 13 commits into
Open
Conversation
| } | ||
|
|
||
| class ClangCompiler(GnuLikeCompiler): | ||
| class ClangCompiler(LLVMCompilerMixin, GnuLikeCompiler): |
Check warning
Code scanning / CodeQL
Conflicting attributes in base classes
| } | ||
|
|
||
| class RustCompiler(Compiler): | ||
| class RustCompiler(LLVMCompilerMixin, Compiler): |
Check warning
Code scanning / CodeQL
Conflicting attributes in base classes
dcbaker
force-pushed
the
submit/clang-rustc-pgo
branch
4 times, most recently
from
July 9, 2024 20:45
bfeb9bb to
8d8d8b8
Compare
tristan957
reviewed
Jul 9, 2024
| Compiles C/C++, ObjC/ObjC++, Fortran, and D sources | ||
| """Compiles C/C++, ObjC/ObjC++, Fortran, and D sources. | ||
|
|
||
| :param target: The target which the source belongs to |
Member
There was a problem hiding this comment.
Be consistent about starting the description with a capital letter or not.
|
|
||
|
|
||
| def get_base_compile_args(options: 'KeyedOptionDictType', compiler: 'Compiler', env: 'Environment') -> T.List[str]: | ||
| def get_base_compile_args(options: 'KeyedOptionDictType', compiler: 'Compiler', env: 'Environment', privatedir: str) -> T.List[str]: |
Member
There was a problem hiding this comment.
I would name the parameter private_dir to match the target function name.
Member
|
Is it possible to add any tests for PGO? |
header dependency are usually order-only, but this actually provides a full dependency.
Due to the confusing naming of the parameters, a number of arguments were incorrectly added as full dependencies when they should be order only.
This is important for Rust, which needs to have a path to write temporary files to, and we want to put that in the target's private directory.
Rustc needs a directory to read and write files from, and clang can make use of this as well (which may be useful for profiling mixed rustc and clang targets).
This helps to reduce code duplication around the code base.
GCC and Clang are actually somewhat different than each other on PGO. GCC, expects a single directory where data will be written, and will read and write itself. Clang expects a directory where it will write data as the binary runs, and then expects an external tool (llvm-profdata) to merge the resulting instrumentation files before clang uses the profile data.
LLVM based compilers do not generate profile data for static libraries, but GCC based compilers do.
Rustc works like clang, so we just need to plug into that path.
dcbaker
force-pushed
the
submit/clang-rustc-pgo
branch
from
July 10, 2024 18:16
8d8d8b8 to
069db29
Compare
Since we don't seem to have any tests for this feature
Member
Author
|
changes since the last version:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Clang (and other LLVM compilers) handle PGO slightly differently than GCC based compilers do. The following is a short list of differences:
Fixes #5251
Fixes #13371
Based on #13377