Move most coverage code out of rustc_codegen_ssa
#113355
Merged
Conversation
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
|
r? @oli-obk (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_gcc cc @antoyo |
|
@rustbot label +A-code-coverage |
rustbot
added
the
A-code-coverage
|
The GCC backend changes just replace one set of stubbed-out methods with a newer, smaller set of stubbed-out methods, so that should be uncontroversial. (Part of the motivation behind doing this before my other planned changes is to avoid the need for further churn in other backends.) |
This effectively inlines most of `FunctionCx::codegen_coverage` into the LLVM implementation of `CoverageInfoBuilderMethods`.
…d it These methods are only ever called from within `rustc_codegen_llvm`, so they can just be declared there as well.
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Jul 5, 2023
Move most coverage code out of `rustc_codegen_ssa` *This is one step in my larger coverage refactoring ambitions described at <https://github.com/rust-lang/compiler-team/issues/645>.* The backend implementation of coverage instrumentation was originally split between SSA and LLVM, perhaps in the hopes that it could be used by other backends. In practice, this split mostly just makes the coverage implementation harder to navigate and harder to modify. It seems unlikely that any backend will actually implement coverage instrumentation in the foreseeable future, especially since many parts of the existing implementation (outside the LLVM backend) are heavily tied to the specific details of LLVM's coverage instrumentation features. The current shared implementation of `codegen_coverage` is heavily tied to the details of `StatementKind::Coverage`, which makes those details difficult to change. I have reason to want to change those details as part of future fixes/improvements, so this will reduce the amount of interface churn caused by those later changes. --- This is intended to be a pure refactoring change, with no changes to actual behaviour. All of the “added” code has really just been moved from other files.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 5, 2023
…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#113010 (rust-installer & rls: remove exclusion from rustfmt & tidy ) - rust-lang#113317 ( -Ztrait-solver=next: stop depending on old solver) - rust-lang#113319 (`TypeParameterDefinition` always require a `DefId`) - rust-lang#113320 (Add some extra information to opaque type cycle errors) - rust-lang#113321 (Move `ty::ConstKind` to `rustc_type_ir`) - rust-lang#113337 (Winnow specialized impls during selection in new solver) - rust-lang#113355 (Move most coverage code out of `rustc_codegen_ssa`) - rust-lang#113356 (Add support for NetBSD/riscv64 aka. riscv64gc-unknown-netbsd.) r? `@ghost` `@rustbot` modify labels: rollup
3 tasks
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Aug 2, 2023
Coverage FFI types were historically split across two modules, because some of them were needed by code in `rustc_codegen_ssa`. Now that all of the coverage codegen code has been moved into `rustc_codegen_llvm` (rust-lang#113355), it's possible to move all of the FFI types into a single module, making it easier to see all of them at once.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 2, 2023
coverage: Consolidate FFI types into one module Coverage FFI types were historically split across two modules, because some of them were needed by code in `rustc_codegen_ssa`. Now that all of the coverage codegen code has been moved into `rustc_codegen_llvm` (rust-lang#113355), it's possible to move all of the FFI types into a single module, making it easier to see all of them at once. --- This PR only moves code and adjusts imports; there should be no functional changes.
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.
This is one step in my larger coverage refactoring ambitions described at rust-lang/compiler-team#645.
The backend implementation of coverage instrumentation was originally split between SSA and LLVM, perhaps in the hopes that it could be used by other backends.
In practice, this split mostly just makes the coverage implementation harder to navigate and harder to modify. It seems unlikely that any backend will actually implement coverage instrumentation in the foreseeable future, especially since many parts of the existing implementation (outside the LLVM backend) are heavily tied to the specific details of LLVM's coverage instrumentation features.
The current shared implementation of
codegen_coverageis heavily tied to the details ofStatementKind::Coverage, which makes those details difficult to change. I have reason to want to change those details as part of future fixes/improvements, so this will reduce the amount of interface churn caused by those later changes.This is intended to be a pure refactoring change, with no changes to actual behaviour. All of the “added” code has really just been moved from other files.