Stop generating assumes for validity ranges

[erikdesjardins]

After #128371, we represent validity ranges as parameter / return value attributes, so we no longer need to use assumes.

r? @ghost

[erikdesjardins]

Previously, LLVM would intersect the validity ranges of Ordering and OneTwoThree, and deduce that the only valid value is 1. Without the assumes, this no longer happens (although the information is not lost--the argument and return value get range attributes, so this could in theory be recovered via a new LLVM opt).

This was added in https://github.com/rust-lang/rust/pull/109993/files#diff-658bfaf1eeca92ac9090a307cc820f2a7afa3c29c047d707a34da3531133f63aR89-R101. This specific example feels pretty obscure, so I don't think this change is significant, unless there's a real-world usecase this is derived from (@scottmcm?)

[scottmcm]

I don't remember where this came from. The specific test is certainly contrived, but I vaguely recall that there was something in this vein, maybe it was transmuting a NonZero<Foo> to something?

Assuming this gets range attributes on the parameter and return such that LLVM has enough information that it could do it, then I'm totally fine regressing this test with a FIXME link to an LLVM bug asking for the opt.

[lqd]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 0d514ae with merge 3f964f8...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3f964f8 (3f964f812fff756b46b59cc4cd4cdb0f43b936c2)

[rust-timer]

Finished benchmarking commit (3f964f8): 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.9%	[0.9%, 0.9%]	1
Regressions ❌ (secondary)	1.1%	[1.1%, 1.1%]	1
Improvements ✅ (primary)	-0.6%	[-0.6%, -0.6%]	2
Improvements ✅ (secondary)	-0.8%	[-1.2%, -0.6%]	7
All ❌✅ (primary)	-0.1%	[-0.6%, 0.9%]	3

Max RSS (memory usage)

Results (primary -0.4%, secondary -2.1%)

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)	2.7%	[1.4%, 3.7%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.8%	[-3.9%, -1.8%]	4
Improvements ✅ (secondary)	-2.1%	[-2.2%, -2.0%]	3
All ❌✅ (primary)	-0.4%	[-3.9%, 3.7%]	7

Cycles

Results (primary 0.9%, secondary -3.6%)

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

Binary size

Results (primary 0.0%, secondary 0.1%)

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%, 0.6%]	27
Regressions ❌ (secondary)	0.1%	[0.0%, 0.2%]	48
Improvements ✅ (primary)	-0.2%	[-1.5%, -0.0%]	23
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	0.0%	[-1.5%, 0.6%]	50

Bootstrap: 753.626s -> 753.637s (0.00%)
Artifact size: 341.40 MiB -> 341.46 MiB (0.02%)

[scottmcm]

fix: seems like this should be keeping at least the ret i8 %x?

[scottmcm]

general comment: the CHECK that just checks the declaration makes these not transmute tests any more, because the body could be removed and it wouldn't matter.

Please put body checks back, or change them to be signature tests instead of transmute tests.

[scottmcm]

Do range operand bundles work yet? (Last I tried I couldn't get them to parse)

I ask because the problem with this PR is that if we inline NonZero::new_unchecked and NonZero::get, we can easily end up with

    _2 = _1 as NonZero<u32> (Transmute);
    _3 = _2 as u32 (Transmute);

where there's nowhere to put the range attributes, and thus transmute-then-transmute will regress with this PR because that'll compile to just %1 without any range information.

(For example, something like Layout::new_unchecked(size, align).align() would lose the range information it gets from the assume today.)

Meaning that

so we no longer need to use assumes.

isn't quite true.

I wonder if we need a MIR primitive that we can inline for such things, when the inlined function's parameter isn't a caller parameter and such...

[andjo403]

No range operand bundles is still not implemented