Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve codegen of fmt_num to delete unreachable panic #122770

Merged
merged 3 commits into from
Nov 14, 2024

Commits on Mar 20, 2024

  1. 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.
    iximeow committed Mar 20, 2024
    Configuration menu
    Copy the full SHA
    e7fbf72 View commit details
    Browse the repository at this point in the history

Commits on Mar 23, 2024

  1. Configuration menu
    Copy the full SHA
    cea973f View commit details
    Browse the repository at this point in the history

Commits on Nov 13, 2024

  1. Delete tests/codegen/fmt_int_no_panic.rs

    this test was included for demonstrative and discussion purposes but does not test what its name alleges - in fact it does not test much of value at all!
    
    as mentioned in this comment, rust-lang#122770 (comment) , writing a correct test for this codegen outcome is difficult (partially because the public interface to std::fmt intentionally includes an #[inline(never)] function!), so to test this correctly will require more than i can offer in 122770.
    iximeow authored Nov 13, 2024
    Configuration menu
    Copy the full SHA
    a54d6ad View commit details
    Browse the repository at this point in the history