Elide superfluous storage markers #99946

tmiasko · 2022-07-30T14:22:58Z

Follow the existing strategy of omitting the storage markers for temporaries
introduced for internal usage when elaborating derefs and deref projections.

Those temporaries are simple scalars which are used immediately after being
defined and never have their address taken. There is no benefit from storage
markers from either liveness analysis or code generation perspective.

rustbot · 2022-07-30T14:23:02Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

tmiasko · 2022-07-30T14:23:11Z

@bors try @rust-timer queue

rust-timer · 2022-07-30T14:23:13Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-30T14:23:19Z

⌛ Trying commit c9d476336c00f9ffd444b7203c5b9d9b5d1d00d7 with merge 467f2108b6f0f8a8c387d759b25d440430e7e572...

bors · 2022-07-30T16:13:44Z

☀️ Try build successful - checks-actions
Build commit: 467f2108b6f0f8a8c387d759b25d440430e7e572 (467f2108b6f0f8a8c387d759b25d440430e7e572)

rust-timer · 2022-07-30T16:13:46Z

Queued 467f2108b6f0f8a8c387d759b25d440430e7e572 with parent 110777b, future comparison URL.

rust-timer · 2022-07-30T17:29:31Z

Finished benchmarking commit (467f2108b6f0f8a8c387d759b25d440430e7e572): comparison url.

Instruction count

Primary benchmarks: mixed results
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	0.5%	1.2%	13
Regressions 😿 (secondary)	0.6%	1.4%	22
Improvements 🎉 (primary)	-0.4%	-1.0%	12
Improvements 🎉 (secondary)	-0.3%	-0.5%	7
All 😿🎉 (primary)	0.1%	1.2%	25

Max RSS (memory usage)

Results

Primary benchmarks: mixed results
Secondary benchmarks: 🎉 relevant improvement found

	mean¹	max	count²
Regressions 😿 (primary)	5.9%	7.8%	2
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-4.3%	-4.5%	3
Improvements 🎉 (secondary)	-3.4%	-3.4%	1
All 😿🎉 (primary)	-0.2%	7.8%	5

Cycles

Results

Primary benchmarks: mixed results
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	2.4%	2.4%	1
Regressions 😿 (secondary)	2.5%	2.9%	2
Improvements 🎉 (primary)	-2.3%	-2.8%	2
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-0.7%	-2.8%	3

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

tmiasko · 2022-07-31T08:42:31Z

@bors try @rust-timer queue

rust-timer · 2022-07-31T08:42:32Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-31T08:42:38Z

⌛ Trying commit 33c614ec0dc321327621b90936ad4d19fd659237 with merge 0ea3f3531361953de2d99a64b714377969aa108b...

tmiasko · 2022-07-31T08:44:26Z

@bors try-

tmiasko · 2022-07-31T09:04:10Z

@bors try @rust-timer queue

rust-timer · 2022-07-31T09:04:12Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-31T09:04:18Z

⌛ Trying commit 49ca4afb560812b0328e6beaa1c0b3709e841316 with merge b5d548b9a46d827dc93bcd09057231b8201a446e...

bors · 2022-07-31T10:34:32Z

☀️ Try build successful - checks-actions
Build commit: b5d548b9a46d827dc93bcd09057231b8201a446e (b5d548b9a46d827dc93bcd09057231b8201a446e)

rust-timer · 2022-07-31T10:34:34Z

Queued b5d548b9a46d827dc93bcd09057231b8201a446e with parent a231865, future comparison URL.

This feature of MIR patch system is no longer used.

oli-obk · 2022-08-24T14:49:16Z

compiler/rustc_mir_transform/src/inline.rs

@@ -616,7 +616,9 @@ impl<'tcx> Inliner<'tcx> {
                // If there are any locals without storage markers, give them storage only for the
                // duration of the call.
                for local in callee_body.vars_and_temps_iter() {
-                    if integrator.always_live_locals.contains(local) {
+                    if !callee_body.local_decls[local].internal


cc @rust-lang/wg-const-eval I think we could do something similar for internal locals without storage markers in const eval. Basically not eagerly initialize them and also not error when writing to them without marking them as live first.

Sounds like you are suggesting a change to the operational semantics of MIR? I don't quite understand what change you are suggesting, though.

Somewhat. We'd need to change MIR validation to make sure that internal locals are only referenced after being assigned to first. If that is guaranteed, then it's just an evaluator optimization to lazily initialize internal locals

We'd also have to make sure that syntactically they are never accessed after the StorageDead, and that they never have their address taken. Otherwise we could miss UB.

oli-obk · 2022-08-24T14:49:29Z

@bors r+

bors · 2022-08-24T14:49:32Z

📌 Commit f8ca6aa has been approved by oli-obk

It is now in the queue for this repository.

compiler/rustc_mir_transform/src/deref_separator.rs

oli-obk · 2022-08-24T14:52:32Z

@bors r-

tmiasko · 2022-08-24T19:33:15Z

@bors r=oli-obk rollup=never

Marking as triaged based on perf comparision without codegen unit partitioning bias #99946 (comment).

bors · 2022-08-24T19:33:16Z

📌 Commit f8ca6aa has been approved by oli-obk

It is now in the queue for this repository.

bors · 2022-08-24T23:51:09Z

⌛ Testing commit f8ca6aa with merge 5462da5...

bors · 2022-08-25T02:32:05Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 5462da5 to master...

rust-timer · 2022-08-25T03:46:57Z

Finished benchmarking commit (5462da5): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	0.9%	[0.9%, 0.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.6%	[-1.9%, -0.2%]	14
Improvements ✅ (secondary)	-0.5%	[-1.5%, -0.3%]	38
All ❌✅ (primary)	-0.5%	[-1.9%, 0.9%]	15

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	6.7%	[2.3%, 13.0%]	3
Regressions ❌ (secondary)	8.9%	[7.0%, 10.8%]	2
Improvements ✅ (primary)	-5.2%	[-11.2%, -2.6%]	6
Improvements ✅ (secondary)	-2.9%	[-4.3%, -2.0%]	8
All ❌✅ (primary)	-1.2%	[-11.2%, 13.0%]	9

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.1%	[4.1%, 4.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.3%, -2.0%]	3
All ❌✅ (primary)	-	-	0

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

rust-highfive assigned oli-obk Jul 30, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 30, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 30, 2022

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 30, 2022

tmiasko force-pushed the elide-storage-makers branch from c9d4763 to 33c614e Compare July 31, 2022 08:42

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 31, 2022

tmiasko force-pushed the elide-storage-makers branch from 33c614e to df0f5e9 Compare July 31, 2022 08:43

tmiasko force-pushed the elide-storage-makers branch from df0f5e9 to 49ca4af Compare July 31, 2022 08:45

This comment has been minimized.

Sign in to view

tmiasko added 3 commits August 23, 2022 10:08

Elide storage markers for internal locals when inlining

625af2c

Remove support for adding statement to all successors

c2b74f9

This feature of MIR patch system is no longer used.

./x.py test --bless

f8ca6aa

tmiasko force-pushed the elide-storage-makers branch from 14af9eb to f8ca6aa Compare August 23, 2022 08:48

oli-obk reviewed Aug 24, 2022

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2022

RalfJung reviewed Aug 24, 2022

View reviewed changes

compiler/rustc_mir_transform/src/deref_separator.rs Show resolved Hide resolved

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2022

tmiasko added the perf-regression-triaged The performance regression has been triaged. label Aug 24, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 25, 2022

bors merged commit 5462da5 into rust-lang:master Aug 25, 2022

rustbot added this to the 1.65.0 milestone Aug 25, 2022

bors mentioned this pull request Aug 25, 2022

Enable MIR inlining for generators too #99782

Closed

rustbot removed the perf-regression Performance regression. label Aug 25, 2022

tmiasko deleted the elide-storage-makers branch August 25, 2022 07:06

tmiasko removed the perf-regression-triaged The performance regression has been triaged. label Aug 25, 2022

matthiaskrgr mentioned this pull request Aug 4, 2023

ICE: broken mir with -Zprint-mono-items=eager between passes RemoveStorageMarkers and Inline #114458

Closed

matthiaskrgr mentioned this pull request Dec 18, 2023

MIR validation ICE: Use of local which has no storage #118990

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Elide superfluous storage markers #99946

Elide superfluous storage markers #99946

tmiasko commented Jul 30, 2022 •

edited

Loading

rustbot commented Jul 30, 2022

tmiasko commented Jul 30, 2022

rust-timer commented Jul 30, 2022

bors commented Jul 30, 2022

bors commented Jul 30, 2022

rust-timer commented Jul 30, 2022

rust-timer commented Jul 30, 2022

tmiasko commented Jul 31, 2022

rust-timer commented Jul 31, 2022

bors commented Jul 31, 2022

tmiasko commented Jul 31, 2022

tmiasko commented Jul 31, 2022

rust-timer commented Jul 31, 2022

bors commented Jul 31, 2022

This comment has been minimized.

bors commented Jul 31, 2022

rust-timer commented Jul 31, 2022

oli-obk Aug 24, 2022

RalfJung Aug 24, 2022

oli-obk Aug 24, 2022

RalfJung Aug 24, 2022

oli-obk commented Aug 24, 2022

bors commented Aug 24, 2022

oli-obk commented Aug 24, 2022

tmiasko commented Aug 24, 2022

bors commented Aug 24, 2022

bors commented Aug 24, 2022

bors commented Aug 25, 2022

rust-timer commented Aug 25, 2022

Elide superfluous storage markers #99946

Elide superfluous storage markers #99946

Conversation

tmiasko commented Jul 30, 2022 • edited Loading

rustbot commented Jul 30, 2022

tmiasko commented Jul 30, 2022

rust-timer commented Jul 30, 2022

bors commented Jul 30, 2022

bors commented Jul 30, 2022

rust-timer commented Jul 30, 2022

rust-timer commented Jul 30, 2022

Footnotes

tmiasko commented Jul 31, 2022

rust-timer commented Jul 31, 2022

bors commented Jul 31, 2022

tmiasko commented Jul 31, 2022

tmiasko commented Jul 31, 2022

rust-timer commented Jul 31, 2022

bors commented Jul 31, 2022

This comment has been minimized.

bors commented Jul 31, 2022

rust-timer commented Jul 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oli-obk commented Aug 24, 2022

bors commented Aug 24, 2022

oli-obk commented Aug 24, 2022

tmiasko commented Aug 24, 2022

bors commented Aug 24, 2022

bors commented Aug 24, 2022

bors commented Aug 25, 2022

rust-timer commented Aug 25, 2022

Overall result: ✅ improvements - no action needed

Footnotes

tmiasko commented Jul 30, 2022 •

edited

Loading