Skip to content

GVN: Only propagate borrows from SSA locals - #150485

Merged
rust-bors[bot] merged 4 commits into
rust-lang:mainfrom
dianqk:gvn-ssa-borrow
Feb 8, 2026
Merged

GVN: Only propagate borrows from SSA locals#150485
rust-bors[bot] merged 4 commits into
rust-lang:mainfrom
dianqk:gvn-ssa-borrow

Conversation

@dianqk

@dianqkdianqk commented Dec 29, 2025

Copy link
Copy Markdown
Member

Fixes#141313. This is a more principled fix than #147886.

Using a reference that is not a borrowing of an SSA local at a new location may be UB.

The PR has two major changes.

The first one, when introducing a new dereference at a new location, is that the reference must point to an SSA local or be an immutable argument. dereference_address has handled SSA locals.

The second one, if we cannot guard to the reference point to an SSA local in visit_assign, we have to rewrite the value to opaque. This avoids unifying the following dereferences that also are references:

let b:&T = *a;// ... `a` is allowed to be modified. `c` and `b` have different borrowing lifetime.// Unifying them will extend the lifetime of `b`.let c:&T = *a;

See also #130853.

This still allows unifying non-reference dereferences:

let a:&T = ...;let b:T = *a;// ... a is NOT allowed to be modified.let c:T = *a;

r? @cjgillot

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 29, 2025
@rust-log-analyzer

This comment has been minimized.

@dianqk

Copy link
Copy Markdown
MemberAuthor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

rust-borsBot added a commit that referenced this pull request Jan 1, 2026
[DRAFT] GVN: Only propagate borrows from SSA-locals
@rust-bors

This comment has been minimized.

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 1, 2026
@rust-bors

rust-borsBot commented Jan 2, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f23e5de (f23e5de34a174f8bd281d69038b68bf6332b1af6, parent: fcd630976c460c819c4bbcaf107d0c94501205d8)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f23e5de): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
0.3%[0.1%, 0.9%]20
Regressions ❌
(secondary)
0.4%[0.1%, 2.0%]27
Improvements ✅
(primary)
-0.6%[-1.2%, -0.2%]4
Improvements ✅
(secondary)
-0.2%[-0.4%, -0.1%]7
All ❌✅ (primary)0.1%[-1.2%, 0.9%]24

Max RSS (memory usage)

Results (primary -1.1%, secondary -4.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.4%[1.9%, 2.8%]2
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
-3.4%[-6.8%, -0.6%]3
Improvements ✅
(secondary)
-4.7%[-6.5%, -3.7%]3
All ❌✅ (primary)-1.1%[-6.8%, 2.8%]5

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results (primary 0.1%, secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
0.7%[0.0%, 6.9%]11
Regressions ❌
(secondary)
0.1%[0.0%, 0.2%]14
Improvements ✅
(primary)
-0.1%[-1.0%, -0.0%]27
Improvements ✅
(secondary)
-0.1%[-0.1%, -0.0%]5
All ❌✅ (primary)0.1%[-1.0%, 6.9%]38

Bootstrap: 475.254s -> 472.456s (-0.59%)
Artifact size: 390.85 MiB -> 390.82 MiB (-0.01%)

@rustbotrustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 2, 2026
@dianqk

Copy link
Copy Markdown
MemberAuthor

@bors try parent=fcd630976c460c819c4bbcaf107d0c94501205d8 @rust-timer queue

@rust-timer

This comment has been minimized.

rust-borsBot added a commit that referenced this pull request Jan 2, 2026
[DRAFT] GVN: Only propagate borrows from SSA-locals
@rust-bors

This comment has been minimized.

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 2, 2026
@rust-bors

rust-borsBot commented Jan 2, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 2183340 (218334063dc5d70b7f9d69202bf22750167c6cbf, parent: fcd630976c460c819c4bbcaf107d0c94501205d8)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (2183340): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
0.3%[0.1%, 0.9%]17
Regressions ❌
(secondary)
0.3%[0.1%, 0.8%]23
Improvements ✅
(primary)
-0.6%[-1.2%, -0.2%]4
Improvements ✅
(secondary)
-0.2%[-0.4%, -0.1%]8
All ❌✅ (primary)0.1%[-1.2%, 0.9%]21

Max RSS (memory usage)

Results (primary -1.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
1.7%[1.7%, 1.7%]1
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
-3.0%[-4.6%, -0.6%]3
Improvements ✅
(secondary)
--0
All ❌✅ (primary)-1.9%[-4.6%, 1.7%]4

Cycles

Results (secondary -2.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
--0
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-2.3%[-2.3%, -2.3%]1
All ❌✅ (primary)--0

Binary size

Results (primary 0.2%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
0.7%[0.0%, 6.9%]11
Regressions ❌
(secondary)
0.1%[0.0%, 0.2%]14
Improvements ✅
(primary)
-0.2%[-0.9%, -0.0%]11
Improvements ✅
(secondary)
-0.1%[-0.1%, -0.0%]5
All ❌✅ (primary)0.2%[-0.9%, 6.9%]22

Bootstrap: 475.254s -> 472.783s (-0.52%)
Artifact size: 390.85 MiB -> 390.84 MiB (-0.00%)

@rustbotrustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 2, 2026
@rust-log-analyzer

This comment has been minimized.

@dianqk

Copy link
Copy Markdown
MemberAuthor

@bors try @rust-timer queue

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 4, 2026
@rust-bors

rust-borsBot commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f2def88 (f2def887c3d08e9845884a5677473348acc2e23f, parent: db3e99bbab28c6ca778b13222becdea54533d908)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f2def88): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
1.5%[0.9%, 2.1%]2
Regressions ❌
(secondary)
0.7%[0.3%, 2.0%]5
Improvements ✅
(primary)
-1.5%[-1.9%, -1.0%]2
Improvements ✅
(secondary)
--0
All ❌✅ (primary)0.0%[-1.9%, 2.1%]4

Max RSS (memory usage)

Results (primary -0.6%, secondary 0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.5%[1.7%, 3.5%]3
Regressions ❌
(secondary)
1.5%[0.5%, 2.5%]2
Improvements ✅
(primary)
-5.3%[-7.6%, -3.1%]2
Improvements ✅
(secondary)
-2.0%[-2.0%, -2.0%]1
All ❌✅ (primary)-0.6%[-7.6%, 3.5%]5

Cycles

Results (primary 2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.8%[2.8%, 2.8%]1
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
--0
All ❌✅ (primary)2.8%[2.8%, 2.8%]1

Binary size

Results (primary 0.0%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
0.4%[0.0%, 1.2%]5
Regressions ❌
(secondary)
1.2%[1.2%, 1.2%]1
Improvements ✅
(primary)
-0.3%[-1.4%, -0.0%]6
Improvements ✅
(secondary)
-0.1%[-0.2%, -0.0%]8
All ❌✅ (primary)0.0%[-1.4%, 1.2%]11

Bootstrap: 472.94s -> 474.175s (0.26%)
Artifact size: 398.12 MiB -> 398.12 MiB (-0.00%)

@rustbotrustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 5, 2026
@dianqk

Copy link
Copy Markdown
MemberAuthor

The two latest perf results that were built with LLVM 22 are different. I am not sure why, but the latest result seems acceptable to me.

@cjgillot

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-borsBot commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 9c029d2 has been approved by cjgillot

It is now in the queue for this repository.

@rust-borsrust-borsBot 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 Feb 8, 2026
@rust-bors

This comment has been minimized.

@rust-borsrust-borsBot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 8, 2026
@rust-bors

rust-borsBot commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

☀️ Test successful - CI
Approved by: cjgillot
Duration: 3h 8m 13s
Pushing 13c3873 to main...

@rust-bors
rust-borsBot merged commit 13c3873 into rust-lang:mainFeb 8, 2026
13 checks passed
@rustbotrustbot added this to the 1.95.0 milestone Feb 8, 2026
@github-actions

Copy link
Copy Markdown
Contributor
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 08a4ce5 (parent) -> 13c3873 (this PR)

Test differences

Show 4 test diffs

4 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 13c38730d981289cc7ae4cc109fd7756bf83ee67 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-apple-various: 1h 12m -> 1h 1m (-15.5%)
  2. aarch64-gnu-llvm-20-1: 1h 3m -> 57m 6s (-10.6%)
  3. x86_64-gnu-llvm-21-2: 1h 28m -> 1h 36m (+9.5%)
  4. x86_64-msvc-ext3: 1h 42m -> 1h 52m (+9.4%)
  5. dist-i586-gnu-i586-i686-musl: 1h 26m -> 1h 34m (+9.2%)
  6. x86_64-msvc-2: 2h 17m -> 2h 29m (+8.9%)
  7. x86_64-gnu-stable: 2h 28m -> 2h 15m (-8.9%)
  8. x86_64-msvc-ext2: 1h 35m -> 1h 42m (+8.2%)
  9. dist-x86_64-apple: 2h 9m -> 1h 59m (-7.4%)
  10. pr-check-1: 34m 21s -> 31m 50s (-7.3%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (13c3873): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
1.0%[0.3%, 2.0%]3
Regressions ❌
(secondary)
0.7%[0.3%, 1.9%]4
Improvements ✅
(primary)
-1.0%[-1.0%, -1.0%]2
Improvements ✅
(secondary)
--0
All ❌✅ (primary)0.2%[-1.0%, 2.0%]5

Max RSS (memory usage)

Results (primary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.8%[2.7%, 3.0%]3
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
-4.0%[-4.0%, -4.0%]1
Improvements ✅
(secondary)
--0
All ❌✅ (primary)1.1%[-4.0%, 3.0%]4

Cycles

Results (primary 2.1%, secondary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.1%[2.1%, 2.1%]1
Regressions ❌
(secondary)
2.6%[2.1%, 3.2%]2
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-2.0%[-2.0%, -2.0%]1
All ❌✅ (primary)2.1%[2.1%, 2.1%]1

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
0.4%[0.0%, 1.2%]5
Regressions ❌
(secondary)
1.2%[1.2%, 1.2%]1
Improvements ✅
(primary)
-0.1%[-0.4%, -0.0%]6
Improvements ✅
(secondary)
-0.1%[-0.2%, -0.0%]8
All ❌✅ (primary)0.1%[-0.4%, 1.2%]11

Bootstrap: 476.152s -> 471.737s (-0.93%)
Artifact size: 397.96 MiB -> 397.92 MiB (-0.01%)

@dianqk
dianqk deleted the gvn-ssa-borrow branch February 8, 2026 10:49
@panstromek

Copy link
Copy Markdown
Contributor

perf triage:

Results match pre merge results, which was accepted in #150485 (comment).

@rustbot label: +perf-regression-triaged

@rustbotrustbot added the perf-regression-triaged The performance regression has been triaged. label Feb 9, 2026
// ```
if projection_ty.ty.is_ref() {
return None;
}

@RalfJungRalfJungApr 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand how this check is enough. What if we have a &(&T,) type? Then the projection_ty is a tuple, not a reference, but it contains a reference and that could still lead to the same problems, couldn't it?

View changes since the review

rust-borsBot pushed a commit that referenced this pull request Jun 21, 2026
 GVN: Don't assume nested shared references are read-only.
Fixes#155884.
#150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types.
cc @RalfJung@saethlin r? @cjgillot
pullBot pushed a commit to xtqqczze/rust-lang-miri that referenced this pull request Jun 22, 2026
 GVN: Don't assume nested shared references are read-only.
Fixesrust-lang/rust#155884.
rust-lang/rust#150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types.
cc @RalfJung@saethlin r? @cjgillot
pullBot pushed a commit to p-avital/rust-analyzer that referenced this pull request Jun 22, 2026
 GVN: Don't assume nested shared references are read-only.
Fixesrust-lang/rust#155884.
rust-lang/rust#150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types.
cc @RalfJung@saethlin r? @cjgillot
github-actionsBot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jun 29, 2026
 GVN: Don't assume nested shared references are read-only.
Fixesrust-lang/rust#155884.
rust-lang/rust#150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types.
cc @RalfJung@saethlin r? @cjgillot
github-actionsBot pushed a commit to rust-lang/stdarch that referenced this pull request Jul 16, 2026
 GVN: Don't assume nested shared references are read-only.
Fixesrust-lang/rust#155884.
rust-lang/rust#150485 only checks the type, whether it is a reference, which is not enough. The PR extends to other types.
cc @RalfJung@saethlin r? @cjgillot
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-borsThis PR was explicitly merged by bors.perf-regressionPerformance regression.perf-regression-triagedThe performance regression has been triaged.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GVN misunderstands aliasing, can create overlapping assignments (again)

8 participants

@dianqk@rust-log-analyzer@rust-timer@rustbot@Kobzol@cjgillot@panstromek@RalfJung