Allowing re-implementation of mir_drops_elaborated query#114628
Merged
bors merged 2 commits intorust-lang:masterfrom Aug 9, 2023
Merged
Allowing re-implementation of mir_drops_elaborated query#114628bors merged 2 commits intorust-lang:masterfrom
bors merged 2 commits intorust-lang:masterfrom
Conversation
Make module inner and function run_analysis_to_runtime_passes in rustc_mir_transform public to allow re-implementing the query from the rust compiler interface.
Collaborator
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
Collaborator
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
oli-obk
reviewed
Aug 8, 2023
| } | ||
|
|
||
| fn run_analysis_to_runtime_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { | ||
| pub fn run_analysis_to_runtime_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { |
Contributor
There was a problem hiding this comment.
Please add a comment that this is public so that custom rustc drivers can overwrite the mir_drops_elaborated and run its steps itself.
Contributor
Collaborator
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 8, 2023
Allowing re-implementation of mir_drops_elaborated query For our use case of the rust compiler interface (a rust verifier called [Prusti](https://github.com/viperproject/prusti-dev/)), it would be extremely useful if we were able to "copy" the implementation of the `mir_drops_elaborated_and_const_checked` query to override it. This would mean that the following items would need to be made public: >https://github.com/rust-lang/rust/blob/6d55184d05c9bd3c46b294dcad3bfb1d0907e871/compiler/rustc_mir_transform/src/lib.rs#L434 >https://github.com/rust-lang/rust/blob/6d55184d05c9bd3c46b294dcad3bfb1d0907e871/compiler/rustc_mir_transform/src/inline.rs#L32 (for the latter its module needs to be public or it needs to be re-exported) To explain why (we think) this is necessary: I am currently working on a new feature, where we try to modify the generated executables by inserting certain additional checks, and potentially perform some optimizations based on verification results. We are using the rust compiler interface and most of our goals can be achieved by overriding queries, in our case this is currently `mir_drops_elaborated_and_const_checked`. However, at the moment this approach is somewhat limited. When overriding queries, we can call and steal the base-query and then modify the results before allocating and returning those. The problem is that the verification works with a copy of `mir_promoted`. For the modifications we want to make to the mir, we would often want to rely on results of the verifier that refer to Locations in the `mir_promoted`. We can not modify the `mir_promoted` query using these results, because to run the verification we also need the results of `mir_borrowck()`, which means `mir_promoted` will already be constructed and cached. The Locations we get from the verifier are also no longer usable to modify `mir_drops_elaborated_and_const_checked`, because the MIR obviously changes between those 2 phases. Tracking all Locations between the two seems to be pretty much unfeasible, and would also be extremely unstable. By being able to override the query with its original implementation, we could modify the MIR before drop elaboration and the various other passes are performed. I have spent quite a bit of time investigating other solutions, and didn't find any other way solving this problem. If I still missed something I would of course be happy to hear any suggestions that do not require exposing more internal compiler functionality. However, I think being able to re-implement certain queries could also benefit other use cases in the future, for example in PR rust-lang#108328 one of the approaches discussed involved doing the same thing for `mir_promoted`.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 8, 2023
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#106425 (Make ExitStatus implement Default) - rust-lang#113480 (add aarch64-unknown-teeos target) - rust-lang#113586 (Mention style for new syntax in tracking issue template) - rust-lang#113593 (CFI: Fix error compiling core with LLVM CFI enabled) - rust-lang#114612 (update llvm-wrapper include to silence deprecation warning) - rust-lang#114613 (Prevent constant rebuilds of `rustc-main` (and thus everything else)) - rust-lang#114615 (interpret: remove incomplete protection against invalid where clauses) - rust-lang#114628 (Allowing re-implementation of mir_drops_elaborated query) - rust-lang#114629 (tests: Uncomment now valid GAT code behind FIXME) - rust-lang#114630 (Migrate GUI colors test to original CSS color format) - rust-lang#114631 (add provisional cache test for new solver) r? `@ghost` `@rustbot` modify labels: rollup
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.
For our use case of the rust compiler interface (a rust verifier called Prusti), it would be extremely useful if we were able to "copy" the implementation of the
mir_drops_elaborated_and_const_checkedquery to override it. This would mean that the following items would need to be made public:To explain why (we think) this is necessary: I am currently working on a new feature, where we try to modify the generated executables by inserting certain additional checks, and potentially perform some optimizations based on verification results.
We are using the rust compiler interface and most of our goals can be achieved by overriding queries, in our case this is currently
mir_drops_elaborated_and_const_checked.However, at the moment this approach is somewhat limited. When overriding queries, we can call and steal the base-query and then modify the results before allocating and returning those.
The problem is that the verification works with a copy of
mir_promoted. For the modifications we want to make to the mir, we would often want to rely on results of the verifier that refer to Locations in themir_promoted. We can not modify themir_promotedquery using these results, because to run the verification we also need the results ofmir_borrowck(), which meansmir_promotedwill already be constructed and cached.The Locations we get from the verifier are also no longer usable to modify
mir_drops_elaborated_and_const_checked, because the MIR obviously changes between those 2 phases. Tracking all Locations between the two seems to be pretty much unfeasible, and would also be extremely unstable.By being able to override the query with its original implementation, we could modify the MIR before drop elaboration and the various other passes are performed.
I have spent quite a bit of time investigating other solutions, and didn't find any other way solving this problem. If I still missed something I would of course be happy to hear any suggestions that do not require exposing more internal compiler functionality. However, I think being able to re-implement certain queries could also benefit other use cases in the future, for example in PR #108328 one of the approaches discussed involved doing the same thing for
mir_promoted.