Indicate that multiplication in Layout::array cannot overflow

[Mark-Simulacrum]

Since #113113, we have added a check that skips calling into the allocator at all if capacity == 0. The global, default allocator will not actually try to allocate though; it returns a dangling pointer explicitly. However, these two checks are not merged/deduplicated by LLVM and so we're comparing to zero twice whenever vectors are allocated/grown. Probably cheap, but also potentially expensive in code size and seems like an unfortunate miss.

This removes that extra check by telling LLVM that the multiplication as part of Layout::array can't overflow, turning the original non-zero value into a zero value afterwards. In my checks locally this successfully drops the duplicate comparisons.

See https://rust.godbolt.org/z/b6nPP9dcK for a code example.

pub fn foo(elements: usize) -> Vec<u32> {
    Vec::with_capacity(elements)
}

r? @scottmcm since you touched this in a32305a - curious if you have thoughts on doing this / can confirm my model of this being correct.

[Mark-Simulacrum]

@bors try @rust-timer queue

[bors]

⌛ Trying commit b81e788 with merge bc314d2...

[bors]

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

[rust-timer]

Finished benchmarking commit (bc314d2): comparison URL.

Overall result: ✅ improvements - no 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.

@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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.6%, -0.4%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.5%	[-0.6%, -0.4%]	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)	-	-	0
Regressions ❌ (secondary)	2.6%	[1.6%, 3.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	-	-	0

Cycles

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

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.0%	[0.0%, 0.0%]	10
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	10

Bootstrap: 675.686s -> 674.254s (-0.21%)
Artifact size: 313.72 MiB -> 313.69 MiB (-0.01%)

[scottmcm]

Hmm, interesting, I would have expected LLVM could just do this.

*goes off to investigate*

Looks like phase ordering strikes again.

SimplifyCFG (https://rust.godbolt.org/z/e8Y9ohfro) optimizes it into a form where it's no longer legal to make it shl nuw (https://alive2.llvm.org/ce/z/wgquGi) because LLVM doesn't know that the payload of a None is allowed to be whatever. In the step before that, it actually would have been legal (https://alive2.llvm.org/ce/z/E37oVn) to add the nuw flag, since then it's still under the branch and thus protected.

Sounds like this is worth doing, then.
@bors r+

This makes me want to try other options for how to write the checks again (like #100866), so I'd kinda like to have a codegen test for this, but I can't think of one that's more than "does this have nuw?", which isn't necessarily that useful since it's not checking the with_capacity property that you mention as the real desire here.

[bors]

📌 Commit b81e788 has been approved by scottmcm

It is now in the queue for this repository.

[bors]

⌛ Testing commit b81e788 with merge b06258c...

[bors]

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing b06258c to master...

[rust-timer]

Finished benchmarking commit (b06258c): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.5%, -0.3%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.5%, -0.3%]	3

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-2.2%, -1.1%]	2
All ❌✅ (primary)	-	-	0

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)	-	-	0
Improvements ✅ (secondary)	-1.9%	[-2.0%, -1.8%]	4
All ❌✅ (primary)	-	-	0

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.0%	[0.0%, 0.0%]	10
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	10

Bootstrap: 678.134s -> 674.301s (-0.57%)
Artifact size: 313.53 MiB -> 313.51 MiB (-0.01%)