about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-11-24 11:19:34 +0000
committerbors <bors@rust-lang.org>2023-11-24 11:19:34 +0000
commitb06258cde4b0dd131cdbf289349ebf51b3b6388a (patch)
tree094d38db3a31a4952dab9bdc00fc4df8bec1b8b3
parentf74f700952f105536446e415b8df8061bddfb25e (diff)
parentb81e788d16d9656b9fd8905de9a3f382ccdbb56c (diff)
downloadrust-b06258cde4b0dd131cdbf289349ebf51b3b6388a.tar.gz
rust-b06258cde4b0dd131cdbf289349ebf51b3b6388a.zip
Auto merge of #118228 - Mark-Simulacrum:alloc-opt, r=scottmcm
Indicate that multiplication in Layout::array cannot overflow

Since https://github.com/rust-lang/rust/pull/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.

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

r? `@scottmcm` since you touched this in a32305a80fd1409b054e97836321bd0621b142fd - curious if you have thoughts on doing this / can confirm my model of this being correct.
-rw-r--r--library/core/src/alloc/layout.rs6
1 files changed, 5 insertions, 1 deletions
diff --git a/library/core/src/alloc/layout.rs b/library/core/src/alloc/layout.rs
index 65946e09ff9..9ef0a7d7608 100644
--- a/library/core/src/alloc/layout.rs
+++ b/library/core/src/alloc/layout.rs
@@ -450,7 +450,11 @@ impl Layout {
                 return Err(LayoutError);
             }
 
-            let array_size = element_size * n;
+            // SAFETY: We just checked that we won't overflow `usize` when we multiply.
+            // This is a useless hint inside this function, but after inlining this helps
+            // deduplicate checks for whether the overall capacity is zero (e.g., in RawVec's
+            // allocation path) before/after this multiplication.
+            let array_size = unsafe { element_size.unchecked_mul(n) };
 
             // SAFETY: We just checked above that the `array_size` will not
             // exceed `isize::MAX` even when rounded up to the alignment.