Remove storage markers if they won't be used during code generation #78360
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 638ae965adce2470ddf9937dc0674af9b490622e with merge 2cb5cb2ebd84701a032939c589afbc536d832a40... |
|
Thanks for perf-run. At this point it is mostly an experiment. r? @ghost |
|
☀️ Try build successful - checks-actions |
|
Queued 2cb5cb2ebd84701a032939c589afbc536d832a40 with parent f392479, future comparison URL. |
|
Finished benchmarking try commit (2cb5cb2ebd84701a032939c589afbc536d832a40): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
An improvement for full and incremental builds of larger benchmarks. A minor regression for very small benchmarks.
|
638ae96
to
fd13f5e
Compare
|
This a simple transformation removing storage markers at opt-level=0 where they won't have any further use. The CTFE having its own dedicated MIR will be unaffected. r? @wesleywiser |
This comment has been minimized.
This comment has been minimized.
fd13f5e
to
5bc26c8
Compare
|
Should do a perf run for the new code then? |
|
A new perf run would be nice to have. Thanks. |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. |
|
⌛ Trying commit 5bc26c8ece9b697078c730fff254e297afacba6b with merge 07d546cab150e55802417ec8b182be1a5522ad45... |
|
☀️ Try build successful - checks-actions |
|
⌛ Testing commit 31c01be3f262afb19a28fe79d0889905171f3e9f with merge 01bf843c8bba585821820c45b7605889f914a9e5... |
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
The storage markers constitute a substantial portion of all MIR statements. At the same time, for builds without any optimizations, the storage markers have no further use during and after MIR optimization phase. If storage markers are not necessary for code generation, remove them.
The storage markers are removed at -C opt-level=0 and as a result output of mir opt tests vary based on used optimization level. Use opt-level=1 for mir-opt tests to avoid the issue.
31c01be
to
57de468
Compare
|
Rebased and added |
|
@bors r=wesleywiser,oli-obk |
|
📌 Commit 57de468 has been approved by |
|
☀️ Test successful - checks-actions |
|
Storage markers are also used by Miri, and this caused some really strange test failures where we fail to detect some UB with optimizations enabled. I am not sure what is going on... FWIW, would be good to ping @rust-lang/miri for MIR optimization changes that could affect UB detection. This is the second time in a week that I had to dig through rustc PR history to figure out why a test suddenly fails.^^ |
rustup; fix tests for new MIR optimization Somehow rust-lang/rust#78360 manages to mask UB. This would make sense if there were loops or things like that, but there are not, so really this is just very confusing...
rustup; fix tests for new MIR optimization Somehow rust-lang/rust#78360 manages to mask UB. This would make sense if there were loops or things like that, but there are not, so really this is just very confusing...
The storage markers constitute a substantial portion of all MIR
statements. At the same time, for builds without any optimizations,
the storage markers have no further use during and after MIR
optimization phase.
If storage markers are not necessary for code generation, remove them.