Implement SSA-based reference propagation #106285

cjgillot · 2022-12-30T10:28:11Z

Rust has a tendency to create a lot of short-lived borrows, in particular for method calls. This PR aims to remove those short-lived borrows with a const-propagation dedicated to pointers to local places.

This pass aims to transform the following pattern:

  _1 = &raw? mut? PLACE;
  _3 = *_1;
  _4 = &raw? mut? *_1;

Into

  _1 = &raw? mut? PLACE;
  _3 = PLACE;
  _4 = &raw? mut? PLACE;

where PLACE is a direct or an indirect place expression.

By removing indirection, this pass should help both dest-prop and const-prop to handle more cases.
This optimization is distinct from const-prop and dataflow const-prop since the borrow-reborrow patterns needs to preserve borrowck invariants, especially the uniqueness property of mutable references.

The pointed-to places are computed using a SSA analysis. We suppose that removable borrows are typically temporaries from autoref, so they are by construction assigned only once, and a SSA analysis is enough to catch them. For each local, we store both where and how it is used, in order to efficiently compute the all-or-nothing property. Thanks to Derefer, we only have to track locals, not places in general.

There are 3 properties that need to be upheld for this transformation to be legal:

place constness: PLACE must refer to the same memory wherever it appears;
pointer liveness: we must not introduce dereferences of dangling pointers;
&mut borrow uniqueness.

Constness

If PLACE is an indirect projection, if its of the form (*LOCAL).PROJECTIONS where:

LOCAL is SSA;
all projections in PROJECTIONS are constant (no dereference and no indexing).

If PLACE is a direct projection of a local, we consider it as constant if:

the local is always live, or it has a single StorageLive that dominates all uses;
all projections are constant.

Liveness

When performing a substitution, we must take care not to introduce uses of dangling locals.

Using a dangling borrow is UB. Therefore, we assume that for any use of *x, where x is a borrow, the pointed-to memory is live.

Limitations:

occurrences of *x in an &raw mut? *x are accepted;
raw pointers are allowed to be dangling.

In those 2 case, we do not substitute anything, to be on the safe side.

Open question: we do not differentiate borrows of ZST and non-ZST. The UB rules may be
different depending on the layout. Having a different treatment would effectively prevent this
pass from running on polymorphic MIR, which defeats the purpose of MIR opts.

Uniqueness

For &mut borrows, we also need to preserve the uniqueness property:
we must avoid creating a state where we interleave uses of *_1 and _2.
To do it, we only perform full substitution of mutable borrows:
we replace either all or none of the occurrences of *_1.

Some care has to be taken when _1 is copied in other locals.

   _1 = &raw? mut? _2;
   _3 = *_1;
   _4 = _1
   _5 = *_4

In such cases, fully substituting _1 means fully substituting all of the copies.

For immutable borrows, we do not need to preserve such uniqueness property,
so we perform all the possible substitutions without removing the _1 = &_2 statement.

rustbot · 2022-12-30T10:28:17Z

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

petrochenkov · 2022-12-30T20:57:30Z

r? rust-lang/compiler

cjgillot · 2023-01-01T13:08:07Z

@bors try @rust-timer queue

bors · 2023-01-01T13:08:15Z

⌛ Trying commit 46516095d7a9cb208c8f17a01574a9a8bc180833 with merge fea7409a5e37c7f115c6b24d93e12e52fe364c27...

bors · 2023-01-01T15:59:31Z

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

rust-timer · 2023-01-01T17:18:18Z

Finished benchmarking commit (fea7409a5e37c7f115c6b24d93e12e52fe364c27): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

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-perf +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.7%	[0.2%, 1.8%]	25
Regressions ❌ (secondary)	0.5%	[0.2%, 0.8%]	17
Improvements ✅ (primary)	-0.8%	[-1.6%, -0.3%]	16
Improvements ✅ (secondary)	-1.0%	[-1.9%, -0.5%]	19
All ❌✅ (primary)	0.1%	[-1.6%, 1.8%]	41

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)	3.5%	[2.5%, 4.5%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.1%	[-3.8%, -0.1%]	5
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-3.8%, 4.5%]	7

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.9%	[0.8%, 1.1%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[0.8%, 1.1%]	4

compiler/rustc_mir_transform/src/scalar_deref_prop.rs

cjgillot · 2023-01-01T21:30:50Z

compiler/rustc_mir_transform/src/scalar_deref_prop.rs

+            | PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf)
+            | PlaceContext::MutatingUse(MutatingUseContext::AddressOf) => {
+                debug!(?local, "BORROW");
+                self.assignments[local] = Set1::Many;


For immutable borrows and AddressOf, we probably don't need to mark the local as non-SSA: that borrow cannot be used, even indirectly to modify the local. Recording a direct use should be enough.

compiler/rustc_mir_transform/src/scalar_deref_prop.rs

compiler/rustc_middle/src/mir/mod.rs

rustbot · 2023-01-07T19:11:10Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

bors · 2023-05-09T18:46:50Z

⌛ Testing commit d30bbe1857fbdac2f4c03a974b22419b02b3c514 with merge bdd6f2fd486b691ba79fff426393a0a45417becb...

bors · 2023-05-09T19:09:55Z

💔 Test failed - checks-actions

cjgillot · 2023-05-09T19:57:48Z

@bors r=JakobDegen

bors · 2023-05-09T19:57:50Z

📌 Commit bde213c has been approved by JakobDegen

It is now in the queue for this repository.

bors · 2023-05-09T21:54:37Z

⌛ Testing commit bde213c with merge 50dff95...

bors · 2023-05-10T00:41:02Z

☀️ Test successful - checks-actions
Approved by: JakobDegen
Pushing 50dff95 to master...

rust-timer · 2023-05-10T01:57:09Z

Finished benchmarking commit (50dff95): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.8%	[0.2%, 1.3%]	2
Regressions ❌ (secondary)	0.4%	[0.4%, 0.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.6%	[-0.9%, -0.3%]	7
All ❌✅ (primary)	0.8%	[0.2%, 1.3%]	2

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)	3.9%	[3.9%, 3.9%]	1
Regressions ❌ (secondary)	0.8%	[0.8%, 0.8%]	1
Improvements ✅ (primary)	-3.1%	[-3.9%, -2.2%]	4
Improvements ✅ (secondary)	-3.2%	[-3.2%, -3.2%]	1
All ❌✅ (primary)	-1.7%	[-3.9%, 3.9%]	5

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)	-	-	0
Improvements ✅ (primary)	-1.3%	[-1.3%, -1.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.3%	[-1.3%, -1.3%]	1

Binary size

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.2%	[0.0%, 1.3%]	45
Regressions ❌ (secondary)	0.9%	[0.7%, 1.1%]	6
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.1%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[-0.1%, 1.3%]	48

Bootstrap: 657.982s -> 659.227s (0.19%)

Allow MIR debuginfo to point to a variable's address MIR optimizations currently do not to operate on borrowed locals. When enabling rust-lang#106285, many borrows will be left as-is because they are used in debuginfo. This pass allows to replace this pattern directly in MIR debuginfo: ```rust a => _1 _1 = &raw? mut? _2 ``` becomes ```rust a => &_2 // No statement to borrow _2. ``` This pass is implemented as a drive-by in ReferencePropagation MIR pass. This transformation allows following following MIR opts to treat _2 as an unborrowed local, and optimize it as such, even in builds with debuginfo. In codegen, when encountering `a => &..&_2`, we create a list of allocas: ```llvm store ptr %_2.dbg.spill, ptr %a.ref0.dbg.spill store ptr %a.ref0.dbg.spill, ptr %a.ref1.dbg.spill ... call void `@llvm.dbg.declare(metadata` ptr %a.ref{n}.dbg.spill, /* ... */) ``` Caveat: this transformation looses the exact type, we do not differentiate `a` as a immutable, mutable reference or a raw pointer. Everything is declared to `*mut` to codegen. I'm not convinced this is a blocker.

Mark-Simulacrum · 2023-05-16T12:52:55Z

Regressions appear to mostly be bimodality.

cjgillot added the A-mir-opt Area: MIR optimizations label Dec 30, 2022

rustbot assigned petrochenkov Dec 30, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 30, 2022

rustbot assigned nagisa and unassigned petrochenkov Dec 30, 2022

cjgillot force-pushed the refprop-ssa branch from 1444cce to aa84bd9 Compare January 1, 2023 13:05

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 1, 2023

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 1, 2023

cjgillot force-pushed the refprop-ssa branch from 4651609 to f5e054a Compare January 1, 2023 21:16

This comment has been minimized.

Sign in to view

Rageking8 reviewed Jan 2, 2023

View reviewed changes

compiler/rustc_mir_transform/src/scalar_deref_prop.rs Outdated Show resolved Hide resolved

cjgillot commented Jan 2, 2023

View reviewed changes

cjgillot force-pushed the refprop-ssa branch 3 times, most recently from 00ae1c3 to a70ab55 Compare January 7, 2023 19:11

cjgillot marked this pull request as ready for review January 7, 2023 19:11

cjgillot force-pushed the refprop-ssa branch from a70ab55 to b52c012 Compare January 7, 2023 21:41

cjgillot added 5 commits May 9, 2023 17:59

Only check that StorageLive dominates address-taking.

3268f2e

Do not check StorageLive dominates address-taking.

3b4e1fe

Only check storage liveness for direct projections.

a67bf08

Correct StorageLive comment.

c17e878

Separate test cases into bbs.

8e5910f

This comment has been minimized.

Sign in to view

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

Add needs-unwind.

bde213c

cjgillot force-pushed the refprop-ssa branch from d30bbe1 to bde213c Compare May 9, 2023 19:39

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 May 9, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label May 10, 2023

bors merged commit 50dff95 into rust-lang:master May 10, 2023

rustbot added this to the 1.71.0 milestone May 10, 2023

bors mentioned this pull request May 10, 2023

Implement a global value numbering MIR optimization #109597

Merged

RalfJung mentioned this pull request May 10, 2023

Miri reports UB with opt-level 4 in code that should not have UB #111422

Closed

cjgillot deleted the refprop-ssa branch May 10, 2023 15:11

cjgillot mentioned this pull request May 10, 2023

Allow MIR debuginfo to point to a variable's address #111440

Merged

Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label May 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SSA-based reference propagation #106285

Implement SSA-based reference propagation #106285

cjgillot commented Dec 30, 2022 •

edited

Loading

rustbot commented Dec 30, 2022

petrochenkov commented Dec 30, 2022

cjgillot commented Jan 1, 2023

This comment has been minimized.

bors commented Jan 1, 2023

This comment has been minimized.

bors commented Jan 1, 2023

This comment has been minimized.

rust-timer commented Jan 1, 2023

This comment has been minimized.

cjgillot Jan 1, 2023

rustbot commented Jan 7, 2023

bors commented May 9, 2023

This comment has been minimized.

bors commented May 9, 2023

cjgillot commented May 9, 2023

bors commented May 9, 2023

bors commented May 9, 2023

bors commented May 10, 2023

rust-timer commented May 10, 2023

Mark-Simulacrum commented May 16, 2023

Implement SSA-based reference propagation #106285

Implement SSA-based reference propagation #106285

Conversation

cjgillot commented Dec 30, 2022 • edited Loading

Constness

Liveness

Uniqueness

rustbot commented Dec 30, 2022

petrochenkov commented Dec 30, 2022

cjgillot commented Jan 1, 2023

This comment has been minimized.

bors commented Jan 1, 2023

This comment has been minimized.

bors commented Jan 1, 2023

This comment has been minimized.

rust-timer commented Jan 1, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

This comment has been minimized.

Choose a reason for hiding this comment

rustbot commented Jan 7, 2023

bors commented May 9, 2023

This comment has been minimized.

bors commented May 9, 2023

cjgillot commented May 9, 2023

bors commented May 9, 2023

bors commented May 9, 2023

bors commented May 10, 2023

rust-timer commented May 10, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Mark-Simulacrum commented May 16, 2023

cjgillot commented Dec 30, 2022 •

edited

Loading