Skip to content

Commit d3b2e82

Browse files
committed
Use unchecked indexing in integer _fmt_inner to keep bounds checks elided
The `_fmt_inner` helper in `core::fmt::num` wrote into its `MaybeUninit<u8>` buffer through regular `[]` indexing and then relied on surrounding `core::hint::assert_unchecked` hints to convince LLVM to elide the resulting bounds checks. Under `-Copt-level=z` with fat LTO (and other configurations where the range information fails to propagate) the hints were dropped and the optimizer left a `panic_bounds_check` call on the hot integer-formatting path. Switch the per-pair buffer writes to `get_unchecked_mut` / `get_unchecked` on both `buf` and `DECIMAL_PAIRS`. Each `unsafe` block carries a short rationale comment (why unchecked indexing) and a tight `SAFETY` comment stating the invariants (`offset + N <= buf.len()`, and the numeric bound on the `DECIMAL_PAIRS` index). The existing `assert_unchecked`s are retained because they still document the `offset` / `buf.len()` invariants and help downstream codegen for the rest of the routine. Also add a codegen-llvm regression test with `opt-3` and `opt-z` revisions that checks the `Display` path for `usize` and `u64` does not contain any `panic_bounds_check`, with a paired sanity check to make sure the symbol itself is still emitted by the compiler for a real out-of-bounds index so the `CHECK-NOT`s cannot pass vacuously.
1 parent 12f35ad commit d3b2e82

2 files changed

Lines changed: 76 additions & 9 deletions

File tree

library/core/src/fmt/num.rs

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,19 @@ macro_rules! impl_Display {
210210
remain /= scale;
211211
let pair1 = (quad / 100) as usize;
212212
let pair2 = (quad % 100) as usize;
213-
buf[offset + 0].write(DECIMAL_PAIRS[pair1 * 2 + 0]);
214-
buf[offset + 1].write(DECIMAL_PAIRS[pair1 * 2 + 1]);
215-
buf[offset + 2].write(DECIMAL_PAIRS[pair2 * 2 + 0]);
216-
buf[offset + 3].write(DECIMAL_PAIRS[pair2 * 2 + 1]);
213+
// Unchecked indexing here avoids relying on LLVM to elide the
214+
// bounds checks from the surrounding `assert_unchecked` hints;
215+
// this regressed under `opt-level=z` + fat LTO on LLVM 21
216+
// (see rust-lang/rust#152061).
217+
//
218+
// SAFETY: `offset + 4 <= buf.len()`, and `pair1, pair2 < 100`
219+
// so all `DECIMAL_PAIRS` indices are `< 200 == DECIMAL_PAIRS.len()`.
220+
unsafe {
221+
buf.get_unchecked_mut(offset).write(*DECIMAL_PAIRS.get_unchecked(pair1 * 2));
222+
buf.get_unchecked_mut(offset + 1).write(*DECIMAL_PAIRS.get_unchecked(pair1 * 2 + 1));
223+
buf.get_unchecked_mut(offset + 2).write(*DECIMAL_PAIRS.get_unchecked(pair2 * 2));
224+
buf.get_unchecked_mut(offset + 3).write(*DECIMAL_PAIRS.get_unchecked(pair2 * 2 + 1));
225+
}
217226
}
218227

219228
// Format per two digits from the lookup table.
@@ -228,8 +237,14 @@ macro_rules! impl_Display {
228237

229238
let pair = (remain % 100) as usize;
230239
remain /= 100;
231-
buf[offset + 0].write(DECIMAL_PAIRS[pair * 2 + 0]);
232-
buf[offset + 1].write(DECIMAL_PAIRS[pair * 2 + 1]);
240+
// See the outer loop for the rationale (rust-lang/rust#152061).
241+
//
242+
// SAFETY: `offset + 2 <= buf.len()`, and `pair < 100` so the
243+
// `DECIMAL_PAIRS` indices are `< 200 == DECIMAL_PAIRS.len()`.
244+
unsafe {
245+
buf.get_unchecked_mut(offset).write(*DECIMAL_PAIRS.get_unchecked(pair * 2));
246+
buf.get_unchecked_mut(offset + 1).write(*DECIMAL_PAIRS.get_unchecked(pair * 2 + 1));
247+
}
233248
}
234249

235250
// Format the last remaining digit, if any.
@@ -242,10 +257,14 @@ macro_rules! impl_Display {
242257
unsafe { core::hint::assert_unchecked(offset <= buf.len()) }
243258
offset -= 1;
244259

245-
// Either the compiler sees that remain < 10, or it prevents
246-
// a boundary check up next.
247260
let last = (remain & 15) as usize;
248-
buf[offset].write(DECIMAL_PAIRS[last * 2 + 1]);
261+
// See the outer loop for the rationale (rust-lang/rust#152061).
262+
//
263+
// SAFETY: `offset < buf.len()`, and `last < 16` so
264+
// `last * 2 + 1 < 32 < 200 == DECIMAL_PAIRS.len()`.
265+
unsafe {
266+
buf.get_unchecked_mut(offset).write(*DECIMAL_PAIRS.get_unchecked(last * 2 + 1));
267+
}
249268
// not used: remain = 0;
250269
}
251270

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
//@ revisions: opt-3 opt-z
2+
//@[opt-3] compile-flags: -Copt-level=3
3+
//@[opt-z] compile-flags: -Copt-level=z
4+
// Regression test for https://github.com/rust-lang/rust/issues/152061.
5+
//
6+
// `impl fmt::Display` for integers, lowered through `_fmt_inner` in
7+
// `library/core/src/fmt/num.rs`, used to leave a `panic_bounds_check`
8+
// path in optimised LLVM IR when LLVM failed to propagate the
9+
// `assume`-based range information (notably under `opt-level=z` + fat
10+
// LTO with LLVM 21). The implementation was rewritten to use
11+
// `get_unchecked{_mut}` for the buffer writes, so the `panic_bounds_check`
12+
// path must not appear regardless of the optimiser's propagation.
13+
14+
#![crate_type = "lib"]
15+
16+
use std::fmt::Write;
17+
18+
pub struct NoopWriter;
19+
impl Write for NoopWriter {
20+
fn write_str(&mut self, _s: &str) -> std::fmt::Result {
21+
Ok(())
22+
}
23+
}
24+
25+
// CHECK-LABEL: @format_usize
26+
#[no_mangle]
27+
pub fn format_usize(w: &mut NoopWriter, x: usize) {
28+
// The Display path through `_fmt_inner` must not emit a bounds check.
29+
// CHECK-NOT: panic_bounds_check
30+
let _ = write!(w, "{}", x);
31+
}
32+
33+
// CHECK-LABEL: @format_u64
34+
#[no_mangle]
35+
pub fn format_u64(w: &mut NoopWriter, x: u64) {
36+
// CHECK-NOT: panic_bounds_check
37+
let _ = write!(w, "{}", x);
38+
}
39+
40+
// Sanity check: make sure `panic_bounds_check` is still the symbol LLVM
41+
// emits for a non-elidable out-of-bounds index, so the `CHECK-NOT`s
42+
// above are guarding against something real and cannot pass vacuously.
43+
// CHECK-LABEL: @test_check
44+
#[no_mangle]
45+
pub fn test_check(arr: &[u8], i: usize) -> u8 {
46+
// CHECK: panic_bounds_check
47+
arr[i]
48+
}

0 commit comments

Comments
 (0)