From c6d2bb7445c87c25f010ff5ced64c703da61a6ae Mon Sep 17 00:00:00 2001 From: iximeow Date: Wed, 20 Mar 2024 01:27:33 -0700 Subject: [PATCH 1/2] improve codegen of fmt_num to delete unreachable panic it seems LLVM doesn't realize that `curr` is always decremented at least once in either loop formatting characters of the input string by their appropriate radix, and so the later `&buf[curr..]` generates a check for out-of-bounds access and panic. this is unreachable in reality as even for `x == T::zero()` we'll produce at least the character `Self::digit(T::zero())` for at least one character output, and `curr` will always be at least one below `buf.len()`. adjust `fmt_int` to make this fact more obvious to the compiler, which fortunately (or unfortunately) results in a measurable performance improvement for workloads heavy on formatting integers. --- core/src/fmt/num.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/fmt/num.rs b/core/src/fmt/num.rs index ab2158394bf1e..abe833a4bf2d7 100644 --- a/core/src/fmt/num.rs +++ b/core/src/fmt/num.rs @@ -79,11 +79,11 @@ unsafe trait GenericRadix: Sized { if is_nonnegative { // Accumulate each digit of the number from the least significant // to the most significant figure. - for byte in buf.iter_mut().rev() { + loop { let n = x % base; // Get the current place value. x = x / base; // Deaccumulate the number. - byte.write(Self::digit(n.to_u8())); // Store the digit in the buffer. curr -= 1; + buf[curr].write(Self::digit(n.to_u8())); // Store the digit in the buffer. if x == zero { // No more digits left to accumulate. break; @@ -91,11 +91,11 @@ unsafe trait GenericRadix: Sized { } } else { // Do the same as above, but accounting for two's complement. - for byte in buf.iter_mut().rev() { + loop { let n = zero - (x % base); // Get the current place value. x = x / base; // Deaccumulate the number. - byte.write(Self::digit(n.to_u8())); // Store the digit in the buffer. curr -= 1; + buf[curr].write(Self::digit(n.to_u8())); // Store the digit in the buffer. if x == zero { // No more digits left to accumulate. break; From e938deaf0f110e3e9e0e949df4d9134fbdeca219 Mon Sep 17 00:00:00 2001 From: iximeow Date: Sat, 23 Mar 2024 11:33:09 -0700 Subject: [PATCH 2/2] try adding a test that LowerHex and friends don't panic, but it doesn't work --- core/benches/fmt.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/benches/fmt.rs b/core/benches/fmt.rs index d1cdb12e50f8c..a02bd60654251 100644 --- a/core/benches/fmt.rs +++ b/core/benches/fmt.rs @@ -148,3 +148,17 @@ fn write_u64_min(bh: &mut Bencher) { test::black_box(format!("{}", 0u64)); }); } + +#[bench] +fn write_u8_max(bh: &mut Bencher) { + bh.iter(|| { + test::black_box(format!("{}", u8::MAX)); + }); +} + +#[bench] +fn write_u8_min(bh: &mut Bencher) { + bh.iter(|| { + test::black_box(format!("{}", 0u8)); + }); +}