Treat union fields as aliasing each other in value_analysis by ojuschugh1 · Pull Request #163425 · rust-lang/rust

ojuschugh1 · 2026-09-28T07:23:02Z

The problem was that for_each_aliasing_place only considered enum variants and the discriminant as aliasing, so writing to one union field left the state of the other fields alone. JumpThreading then threaded the second switchInt on u.a using the value from before u.b = 5, and DataflowConstProp folded the read of u.a to a stale constant in the same way. Union field places have only been in the map since #127087 began collecting places syntactically; before that iter_fields skipped unions.

I made for_each_aliasing_place flood everything under a union as soon as the walk reaches it, since all non-ZST union fields start at offset 0 and overlap. The alternative of not registering places below unions would also work but loses threading on union reads with no write in between.

Added a mir-opt test for each pass.

rustbot · 2026-09-28T07:23:07Z

Thanks for the pull request, and welcome! The Rust Project has assigned @dingxiangfei2009 (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 77 candidates
Random selection from 19 candidates

ojuschugh1 · 2026-10-01T17:34:49Z

Hey @TaKO8Ki @dingxiangfei2009 could you kindly review this PR? Thanks

apiraino · 2026-10-06T13:57:25Z

Hey ... could you kindly review this PR? Thanks

@ojuschugh1 please read again this comment and allow some time to a reviewer to get to this pull request (worth reminding that many of us review patches on a voluntary basis).

thanks

ojuschugh1 · 2026-10-06T15:52:51Z

Hey ... could you kindly review this PR? Thanks

@ojuschugh1 please read again this comment and allow some time to a reviewer to get to this pull request (worth reminding that many of us review patches on a voluntary basis).

thanks

Sorry about that, I'll give it time. Thanks for the reminder.

saethlin · 2026-10-10T01:52:37Z

+
+// CHECK-LABEL: fn main(
+fn main() {
+    // CHECK: debug a => [[a:_.*]];


Suggested change

// CHECK: debug a => [[a:_.*]];

It doesn't make sense to look at debuginfo in a regression test for a miscompile.

View changes since the review

Sure, i will remove it. Thanks

saethlin · 2026-10-10T01:56:31Z

+    // CHECK: [[a]] = copy ({{_.*}}.0: u8);
+    let a = unsafe { u.a }; // should not be propagated


Instead of this, make the function not main, make the crate-type a lib, and return u.a from the function. That should make it rather easy to write the FileCheck that looks for a copy from the union field to the return place.

View changes since the review

Done, it's a lib crate now with read_other_field returning u.a, and the check is _0 = copy ({{_.*}}.0: u8). Thanks

saethlin · 2026-10-10T02:02:48Z

+    // CHECK: ({{_.*}}.1: u8) = const 5_u8;
+    // CHECK-NOT: ({{_.*}}.1: u8) = const 5_u8;


This is only checking that we didn't produce a particular miscompile, it's not checking that the intended pattern is intact. Look through the MIR before and after JumpThreading when things miscompile to understand what structure we are trying to avoid optimizing, and try to write a test for that.

View changes since the review

Right, with the bug bb1 and bb2 stop jumping to bb3 and go to copies of it that jump straight to bb4/bb5, so the second switchInt is left unreachable. The checks now pin the two goto -> bb3 edges and the write followed by the second switchInt in bb3.

saethlin · 2026-10-12T13:27:02Z

There are a few nevers in the merge queue so I'm checking to see if it's reasonable to roll this up

@bors try @rust-timer queue

Treat union fields as aliasing each other in value_analysis

rust-bors · 2026-10-12T14:26:37Z

☀️ Try build successful (CI)
Build commit: ed6d54b (ed6d54b799d537ee7572c7de5aec293493e9acb2)
Base parent: fb5f32b (fb5f32bdbe75e9114c62589cb86ace65f14a5b78)

rust-timer · 2026-10-12T15:28:30Z

Finished benchmarking commit (ed6d54b): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never rustc-perf
@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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	1
Improvements ✅ (primary)	-0.3%	[-0.6%, -0.1%]	6
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.1%]	39
All ❌✅ (primary)	-0.3%	[-0.6%, -0.1%]	6

Max RSS (memory usage)

Results (primary -0.6%, secondary 2.5%)

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

	mean	range	count
Regressions ❌ (primary)	2.6%	[2.5%, 2.7%]	2
Regressions ❌ (secondary)	3.7%	[2.4%, 5.4%]	4
Improvements ✅ (primary)	-3.8%	[-6.4%, -1.2%]	2
Improvements ✅ (secondary)	-2.4%	[-2.4%, -2.4%]	1
All ❌✅ (primary)	-0.6%	[-6.4%, 2.7%]	4

Cycles

Results (secondary -1.2%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 490.047s -> 489.473s (-0.12%)
Artifact size: 406.77 MiB -> 406.90 MiB (0.03%)

saethlin · 2026-10-12T17:07:56Z

@bors r+ rollup=never note="unexpected perf improvement"

rust-bors · 2026-10-12T17:07:59Z

📌 Commit 49e02a2 has been approved by saethlin

It is now in the queue for this repository.

JonathanBrouwer · 2026-10-12T19:14:23Z

@bors rollup=iffy
Adding this to a rollup of perf-sensitive PRs

…uwer Rollup of 4 perf-sensitive pull requests Successful merges: - #163425 (Treat union fields as aliasing each other in value_analysis) - #164074 (Avoid cloning impl trait AST nodes in late resolution) - #164089 (Implement `clone_from` for `BorrowckDomain`) - #164150 (Reserve more capacity for Symbol interner)

Rollup merge of #163425 - ojuschugh1:jump-threading-fix, r=saethlin Treat union fields as aliasing each other in value_analysis Fixes #161898 The problem was that `for_each_aliasing_place` only considered enum variants and the discriminant as aliasing, so writing to one union field left the state of the other fields alone. JumpThreading then threaded the second `switchInt` on `u.a` using the value from before `u.b = 5`, and DataflowConstProp folded the read of `u.a` to a stale constant in the same way. Union field places have only been in the map since #127087 began collecting places syntactically; before that `iter_fields` skipped unions. I made `for_each_aliasing_place` flood everything under a union as soon as the walk reaches it, since all non-ZST union fields start at offset 0 and overlap. The alternative of not registering places below unions would also work but loses threading on union reads with no write in between. Added a mir-opt test for each pass.

rust-timer · 2026-10-13T05:53:35Z

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (e89ad72): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.6%, -0.2%]	3
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	35
All ❌✅ (primary)	-0.4%	[-0.6%, -0.2%]	3

Max RSS (memory usage)

Results (secondary 2.5%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.5%	[2.5%, 2.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 1.8%, secondary -3.5%)

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

	mean	range	count
Regressions ❌ (primary)	1.8%	[1.5%, 2.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-3.5%, -3.5%]	1
All ❌✅ (primary)	1.8%	[1.5%, 2.2%]	2

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.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.2%]	83
Regressions ❌ (secondary)	0.1%	[0.0%, 0.2%]	47
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.2%]	83

Artifact size: 406.63 MiB -> 406.92 MiB (0.07%)

ojuschugh1 · 2026-10-13T18:39:50Z

Thanks @saethlin for the review, the comments on the tests were really helpful. Thanks to everyone else involved too.

rustbot assigned dingxiangfei2009 Sep 28, 2026

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 Sep 28, 2026

rustbot assigned TaKO8Ki and unassigned dingxiangfei2009 Oct 1, 2026

theemathas mentioned this pull request Oct 6, 2026

GVN versus unions with reference fields #163825

Open

rustbot assigned davidtwco and unassigned TaKO8Ki Oct 6, 2026

saethlin assigned saethlin and unassigned davidtwco Oct 10, 2026

saethlin reviewed Oct 10, 2026

View reviewed changes

Treat union fields as aliasing each other in value_analysis

49e02a2

ojuschugh1 force-pushed the jump-threading-fix branch from d0f65b4 to 49e02a2 Compare October 11, 2026 08:08

ojuschugh1 requested a review from saethlin October 11, 2026 15:29

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 Oct 12, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Oct 12, 2026

Auto merge of #163425 - ojuschugh1:jump-threading-fix, r=<try>

ed6d54b

Treat union fields as aliasing each other in value_analysis

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 12, 2026

rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 12, 2026

rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2026

JonathanBrouwer mentioned this pull request Oct 12, 2026

Rollup of 4 perf-sensitive pull requests #164224

Merged

rust-bors Bot merged commit 3c4fab2 into rust-lang:main Oct 12, 2026
15 checks passed

rustbot added this to the 1.101.0 milestone Oct 12, 2026

		// CHECK: [[a]] = copy ({{_.*}}.0: u8);
		let a = unsafe { u.a }; // should not be propagated

		// CHECK: ({{_.*}}.1: u8) = const 5_u8;
		// CHECK-NOT: ({{_.*}}.1: u8) = const 5_u8;

Conversation

ojuschugh1 commented Sep 28, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 28, 2026

Uh oh!

ojuschugh1 commented Oct 1, 2026

Uh oh!

apiraino commented Oct 6, 2026

Uh oh!

ojuschugh1 commented Oct 6, 2026

Uh oh!

saethlin Oct 10, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saethlin Oct 10, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saethlin Oct 10, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saethlin commented Oct 12, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Oct 12, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Oct 12, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

saethlin commented Oct 12, 2026

Uh oh!

rust-bors Bot commented Oct 12, 2026

Uh oh!

JonathanBrouwer commented Oct 12, 2026

Uh oh!

Uh oh!

rust-timer commented Oct 13, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

ojuschugh1 commented Oct 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ojuschugh1 commented Sep 28, 2026 •

edited by rustbot

Loading

saethlin Oct 10, 2026 •

edited by rustbot

Loading

saethlin Oct 10, 2026 •

edited by rustbot

Loading

saethlin Oct 10, 2026 •

edited by rustbot

Loading