rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.95k stars 12.79k forks source link

`isqrt` treated as a `black_box` #132763

Open Rudxain opened 2 weeks ago

Rudxain commented 2 weeks ago

I tried this code:

#[inline(never)]
pub const fn f(n: u8) {
    assert!(n >= 4);
    assert!(2 <= n.isqrt());
}

[!note] I've tried:

  • removing const
  • replacing u8 by usize
  • replacing 4 by 9 and 2 by 3

Outcome was the same

I expected to see this happen: 2nd assertion non-existent in assembly.

Instead, this happened:

# ... (snip)
.LBB0_4:
    leaq    .L__unnamed_4(%rip), %rdi
    leaq    .L__unnamed_5(%rip), %rdx
    movl    $32, %esi
    callq   *core::panicking::panic@GOTPCREL(%rip)
# ...
.L__unnamed_4:
    .ascii  "assertion failed: 2 <= n.isqrt()"
# ...

Meta

Playground

rustc --version --verbose:

1.84.0-nightly (2024-11-06 8549802939cd01111c46)

No Backtrace

@rustbot label: +I-slow, -C-bug

bjorn3 commented 2 weeks ago

isqrt is internally implemented with the help of a lookup table. It looks like LLVM is unable to optimize based on properties of the lookup table like in this case all entries for an input 4 and higher being at least 2.

Rudxain commented 2 weeks ago

I was about to suggest assume(TABLE.is_sorted()), but the docs were prepared to stop me!:

The more complicated the condition, the less likely this is to be useful. For example, assert_unchecked(foo.is_sorted()) is a complex enough value that the compiler is unlikely to be able to take advantage of it.

Thank you, to whoever wrote that ❤️

the8472 commented 2 weeks ago

Adding assume(output.unchecked_mul(output) <= input) at the end might work. Though a more realistic testcase to show that this benefits some code would be nice.

Edit, nevermind, you want a lower bound, not an upper bound.