-
Notifications
You must be signed in to change notification settings - Fork 14k
Validate CopyForDeref and DerefTemps better and remove them from runtime MIR #145513
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
Conversation
|
|
|
This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @vakaras Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri Some changes occurred to constck cc @fee1-dead Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred to the CTFE machinery |
This comment has been minimized.
This comment has been minimized.
|
(note that oli is on vacation and will be for quite a while) |
This comment has been minimized.
This comment has been minimized.
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
8151b0e to
829f66d
Compare
This comment has been minimized.
This comment has been minimized.
829f66d to
dfd97cd
Compare
This comment has been minimized.
This comment has been minimized.
11f84dd to
bfc73ec
Compare
|
r? mir |
|
It's been long enough since I've looked at a involved MIR change that I'm probably not best suited to this r? mir |
|
☔ The latest upstream changes (presumably #146666) made this pull request unmergeable. Please resolve the merge conflicts. |
bfc73ec to
d833c90
Compare
|
@rustbot ready |
|
@bors try jobs=test-various |
Validate CopyForDeref and DerefTemps better and remove them from runtime MIR try-job: test-various
This comment has been minimized.
This comment has been minimized.
|
@bors r=saethlin,cjgillot |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing be0ade2 (parent) -> 3be6803 (this PR) Test differencesShow 64 test diffs64 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 3be68033b67dfc2aa3ae4cfe735aa5805aebae43 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (3be6803): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 3.7%, secondary -1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.6%, secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 472.888s -> 472.207s (-0.14%) |
…hlin Undo CopyForDeref assertion in const qualif Fixes rust-lang#147733 caused by rust-lang#145513 This code in fact does not run only on runtime MIR.
…hlin Undo CopyForDeref assertion in const qualif Fixes rust-lang#147733 caused by rust-lang#145513 This code in fact does not run only on runtime MIR.
…hlin,cjgillot Validate CopyForDeref and DerefTemps better and remove them from runtime MIR (split from my WIP rust-lang#145344) This PR: - Removes `Rvalue::CopyForDeref` and `LocalInfo::DerefTemp` from runtime MIR - Using a new mir pass `EraseDerefTemps` - `CopyForDeref(x)` is turned into `Use(Copy(x))` - `DerefTemp` is turned into `Boring` - Not sure if this part is actually necessary, it made more sense in rust-lang#145344 with `DerefTemp` storing actual data that I wanted to keep from having to be kept in sync with the rest of the body in runtime MIR - Checks in validation that `CopyForDeref` and `DerefTemp` are only used together - Removes special handling for `CopyForDeref` from many places - Removes `CopyForDeref` from `custom_mir` reverting rust-lang#111587 - In runtime MIR simple copies can be used instead - In post cleanup analysis MIR it was already wrong to use due to the lack of support for creating `DerefTemp` locals - Possibly this should be its own PR? - Adds an argument to `deref_finder` to avoid creating new `DerefTemp`s and `CopyForDeref` in runtime MIR. - Ideally we would just avoid making intermediate derefs instead of fixing it at the end of a pass / during shim building - Removes some usages of `deref_finder` that I found out don't actually do anything r? oli-obk
…hlin,cjgillot Validate CopyForDeref and DerefTemps better and remove them from runtime MIR (split from my WIP rust-lang#145344) This PR: - Removes `Rvalue::CopyForDeref` and `LocalInfo::DerefTemp` from runtime MIR - Using a new mir pass `EraseDerefTemps` - `CopyForDeref(x)` is turned into `Use(Copy(x))` - `DerefTemp` is turned into `Boring` - Not sure if this part is actually necessary, it made more sense in rust-lang#145344 with `DerefTemp` storing actual data that I wanted to keep from having to be kept in sync with the rest of the body in runtime MIR - Checks in validation that `CopyForDeref` and `DerefTemp` are only used together - Removes special handling for `CopyForDeref` from many places - Removes `CopyForDeref` from `custom_mir` reverting rust-lang#111587 - In runtime MIR simple copies can be used instead - In post cleanup analysis MIR it was already wrong to use due to the lack of support for creating `DerefTemp` locals - Possibly this should be its own PR? - Adds an argument to `deref_finder` to avoid creating new `DerefTemp`s and `CopyForDeref` in runtime MIR. - Ideally we would just avoid making intermediate derefs instead of fixing it at the end of a pass / during shim building - Removes some usages of `deref_finder` that I found out don't actually do anything r? oli-obk
…hlin,cjgillot Validate CopyForDeref and DerefTemps better and remove them from runtime MIR (split from my WIP rust-lang#145344) This PR: - Removes `Rvalue::CopyForDeref` and `LocalInfo::DerefTemp` from runtime MIR - Using a new mir pass `EraseDerefTemps` - `CopyForDeref(x)` is turned into `Use(Copy(x))` - `DerefTemp` is turned into `Boring` - Not sure if this part is actually necessary, it made more sense in rust-lang#145344 with `DerefTemp` storing actual data that I wanted to keep from having to be kept in sync with the rest of the body in runtime MIR - Checks in validation that `CopyForDeref` and `DerefTemp` are only used together - Removes special handling for `CopyForDeref` from many places - Removes `CopyForDeref` from `custom_mir` reverting rust-lang#111587 - In runtime MIR simple copies can be used instead - In post cleanup analysis MIR it was already wrong to use due to the lack of support for creating `DerefTemp` locals - Possibly this should be its own PR? - Adds an argument to `deref_finder` to avoid creating new `DerefTemp`s and `CopyForDeref` in runtime MIR. - Ideally we would just avoid making intermediate derefs instead of fixing it at the end of a pass / during shim building - Removes some usages of `deref_finder` that I found out don't actually do anything r? oli-obk
(split from my WIP #145344)
This PR:
Rvalue::CopyForDerefandLocalInfo::DerefTempfrom runtime MIREraseDerefTempsCopyForDeref(x)is turned intoUse(Copy(x))DerefTempis turned intoBoringDerefTempstoring actual data that I wanted to keep from having to be kept in sync with the rest of the body in runtime MIRCopyForDerefandDerefTempare only used togetherCopyForDereffrom many placesCopyForDereffromcustom_mirreverting Custom MIR: SupportRvalue::CopyForDeref#111587DerefTemplocalsderef_finderto avoid creating newDerefTemps andCopyForDerefin runtime MIR.deref_finderthat I found out don't actually do anythingr? oli-obk