MIR-build: No longer emit assumes in enum-as casting #144389

scottmcm · 2025-07-24T05:32:17Z

This just uses the valid_range from the backend, so it's duplicating the range metadata that now we include on parameters and loads, and thus no longer seems to be useful -- notably there's no codegen test failures from removing it.

(Because it's using data from the same source as the backend annotations, it doesn't do anything to mitigate things like #144388 where the range in the layout is more permissive than the actual possible discriminants. A variant of this that actually checked the discriminants more specifically might be useful, so could potentially be added in future, but I don't think the current checks are actually providing value.)

r? mir

Randomly turns out that this
Fixes #121097

scottmcm · 2025-07-24T05:40:47Z

@bors try @rust-timer queue

rust-bors · 2025-07-24T05:40:50Z

⌛ Trying commit 0f4bef6 with merge a7398bc…

To cancel the try build, run the command @bors try cancel.

MIR-build: No longer emit assumes in enum-as casting

rust-bors · 2025-07-24T07:54:41Z

☀️ Try build successful (CI)
Build commit: a7398bc (a7398bcea326a20aa12c2a270bd7a9b5f3a24f39, parent: efd420c770bb179537c01063e98cb6990c439654)

rust-timer · 2025-07-24T09:16:01Z

Finished benchmarking commit (a7398bc): 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.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.6%, 0.6%]	1
Regressions ❌ (secondary)	0.7%	[0.2%, 1.1%]	2
Improvements ✅ (primary)	-0.5%	[-0.8%, -0.1%]	8
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	1
All ❌✅ (primary)	-0.3%	[-0.8%, 0.6%]	9

Max RSS (memory usage)

Results (primary -0.5%, secondary 0.3%)

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

	mean	range	count
Regressions ❌ (primary)	3.6%	[2.5%, 4.4%]	3
Regressions ❌ (secondary)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-3.6%	[-4.6%, -2.0%]	4
Improvements ✅ (secondary)	-2.3%	[-2.3%, -2.3%]	1
All ❌✅ (primary)	-0.5%	[-4.6%, 4.4%]	7

Cycles

Results (primary 2.2%, secondary 3.3%)

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

	mean	range	count
Regressions ❌ (primary)	2.2%	[2.1%, 2.2%]	2
Regressions ❌ (secondary)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.2%	[2.1%, 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.3%	[0.1%, 0.6%]	7
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	2
Improvements ✅ (primary)	-0.1%	[-0.5%, -0.0%]	30
Improvements ✅ (secondary)	-0.1%	[-0.5%, -0.0%]	42
All ❌✅ (primary)	-0.1%	[-0.5%, 0.6%]	37

Bootstrap: 468.582s -> 467.881s (-0.15%)
Artifact size: 374.67 MiB -> 374.69 MiB (0.01%)

davidtwco

This seems reasonable to me

davidtwco · 2025-07-24T13:06:18Z

Obviously assuming you can get it to pass CI

This just uses the `valid_range` from the backend, so it's duplicating the range metadata that now we include on parameters and loads.

scottmcm · 2025-07-24T19:18:20Z

Looks like avoiding calling layout like this while we're still in the middle of MIR building also fixed an ICE (and simplified a cycle error). So that's probably also a good reason to do this change -- if anyone wants to do something similar later, it'd probably be best done with a MIR optimization pass, rather than as part of building the MIR, since optimization-related stuff like this isn't relevant to analysis-phase MIR.

@bors r=davidtwco

bors · 2025-07-24T19:18:23Z

📌 Commit 01524ab has been approved by davidtwco

It is now in the queue for this repository.

bors · 2025-07-24T20:38:57Z

⌛ Testing commit 01524ab with merge b56aaec...

bors · 2025-07-24T23:48:02Z

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing b56aaec to master...

github-actions · 2025-07-24T23:50:53Z

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 246733a (parent) -> b56aaec (this PR)

Test differences

Show 8 test diffs

Stage 1

[crashes] tests/crashes/121097.rs: pass -> [missing] (J2)
[ui] tests/ui/simd/repr-simd-on-enum.rs: [missing] -> pass (J2)

Stage 2

[crashes] tests/crashes/121097.rs: pass -> [missing] (J0)
[ui] tests/ui/simd/repr-simd-on-enum.rs: [missing] -> pass (J1)

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

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard b56aaec52bc0fa35591a872fb4aac81f606e265c --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

dist-aarch64-linux: 5807.7s -> 8711.2s (50.0%)
x86_64-apple-2: 5631.1s -> 4202.7s (-25.4%)
dist-x86_64-apple: 11440.3s -> 9420.4s (-17.7%)
x86_64-apple-1: 8217.1s -> 9619.9s (17.1%)
aarch64-apple: 5205.7s -> 4665.0s (-10.4%)
dist-s390x-linux: 4773.4s -> 5226.0s (9.5%)
x86_64-gnu-aux: 6449.6s -> 5936.3s (-8.0%)
dist-various-1: 4012.6s -> 3718.9s (-7.3%)
dist-aarch64-msvc: 6029.3s -> 5607.2s (-7.0%)
aarch64-msvc-2: 5842.1s -> 5460.1s (-6.5%)

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 · 2025-07-25T03:04:26Z

Finished benchmarking commit (b56aaec): 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.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.2%, 0.7%]	13
Regressions ❌ (secondary)	0.3%	[0.1%, 0.6%]	17
Improvements ✅ (primary)	-0.4%	[-0.9%, -0.1%]	8
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	0.1%	[-0.9%, 0.7%]	21

Max RSS (memory usage)

Results (primary -0.7%)

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

	mean	range	count
Regressions ❌ (primary)	2.8%	[1.0%, 4.6%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.2%	[-5.5%, -3.0%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.7%	[-5.5%, 4.6%]	6

Cycles

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

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.3%	[0.1%, 0.6%]	7
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	2
Improvements ✅ (primary)	-0.1%	[-0.5%, -0.0%]	31
Improvements ✅ (secondary)	-0.1%	[-0.5%, -0.0%]	42
All ❌✅ (primary)	-0.1%	[-0.5%, 0.6%]	38

Bootstrap: 469.69s -> 469.891s (0.04%)
Artifact size: 374.65 MiB -> 374.63 MiB (-0.00%)

scottmcm · 2025-07-25T08:27:24Z

I was surprised to see those post-merge results be so different, but it looks like html5ever has gone bimodal

The smaller MIR will also make things with casts like this more inlinable, which is expected to both help and hinder.

Thus overall I think this is fine as-is.

Relevant upstream PRs: - rust-lang/rust#144389 (MIR-build: No longer emit assumes in enum-as casting) - rust-lang/rust#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) - rust-lang/rust#144377 (Rename impl_of_method and trait_of_item) Resolves: model-checking#4246

Relevant upstream PRs: - rust-lang/rust#144389 (MIR-build: No longer emit assumes in enum-as casting) - rust-lang/rust#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) - rust-lang/rust#144377 (Rename impl_of_method and trait_of_item) Resolves: #4246 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

Relevant upstream PRs: - rust-lang/rust#144389 (MIR-build: No longer emit assumes in enum-as casting) - rust-lang/rust#144392 (rustc_public: Remove movability from `RigidTy/AggregateKind::Coroutine`) - rust-lang/rust#144377 (Rename impl_of_method and trait_of_item) Resolves: model-checking#4246 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

rustbot assigned davidtwco Jul 24, 2025

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 Jul 24, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Jul 24, 2025

Auto merge of #144389 - scottmcm:no-more-mir-cast-assume, r=<try>

a7398bc

MIR-build: No longer emit assumes in enum-as casting

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

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 Jul 24, 2025

davidtwco approved these changes Jul 24, 2025

View reviewed changes

scottmcm force-pushed the no-more-mir-cast-assume branch from 0f4bef6 to db94062 Compare July 24, 2025 16:36

This comment has been minimized.

Sign in to view

MIR-build: No longer emit assumes in enum-as casting

01524ab

This just uses the `valid_range` from the backend, so it's duplicating the range metadata that now we include on parameters and loads.

scottmcm mentioned this pull request Jul 24, 2025

ICE: called 'Result::unwrap()' on an 'Err' value: Unknown(Aligned) #121097

Closed

scottmcm force-pushed the no-more-mir-cast-assume branch from db94062 to 01524ab Compare July 24, 2025 17:22

This comment was marked as resolved.

Sign in to view

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 Jul 24, 2025

This was referenced Jul 24, 2025

Try to shrink Alignment-related MIR in Layout #128918

Closed

Stop generating assumes for validity ranges #129027

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 24, 2025

bors merged commit b56aaec into rust-lang:master Jul 24, 2025
11 checks passed

rustbot added this to the 1.90.0 milestone Jul 24, 2025

scottmcm deleted the no-more-mir-cast-assume branch July 25, 2025 08:11

Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Jul 29, 2025

tautschnig mentioned this pull request Jul 29, 2025

Upgrade Rust toolchain to 2025-07-29 model-checking/kani#4247

Merged

MIR-build: No longer emit assumes in enum-as casting #144389

MIR-build: No longer emit assumes in enum-as casting #144389

Uh oh!

Conversation

scottmcm commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottmcm commented Jul 24, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jul 24, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jul 24, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jul 24, 2025

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

Uh oh!

davidtwco left a comment

Choose a reason for hiding this comment

Uh oh!

davidtwco commented Jul 24, 2025

Uh oh!

This comment has been minimized.

This comment was marked as resolved.

scottmcm commented Jul 24, 2025

Uh oh!

bors commented Jul 24, 2025

Uh oh!

bors commented Jul 24, 2025

Uh oh!

bors commented Jul 24, 2025

Uh oh!

Uh oh!

github-actions bot commented Jul 24, 2025

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Jul 25, 2025

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

Uh oh!

scottmcm commented Jul 25, 2025

Uh oh!

Uh oh!

scottmcm commented Jul 24, 2025 •

edited

Loading