rust-lang / rustc_codegen_cranelift

Cranelift based backend for rustc
Apache License 2.0
1.52k stars 94 forks source link

u128 comparison fails with opt-level 1 #1484

Closed cuviper closed 1 month ago

cuviper commented 2 months ago

This little program is fine with opt 0, but fails with opt 1.

fn main() {
    const X: u128 = 0x8000_0000_0000_0000;
    assert_eq!((|| X)(), X);
}
$ rustc +nightly foo.rs -Zcodegen-backend=cranelift -Copt-level=1 && ./foo
thread 'main' panicked at foo.rs:3:5:
assertion `left == right` failed
  left: 9223372036854775808
 right: 9223372036854775808
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Note that this constant could fit in u64, but it looks like it's sign-extending that %r8 MSB into %rcx:

    845c:       e8 86 00 00 00          call   84e7 <_ZN3foo4main28_$u7b$$u7b$closure$u7d$$u7d$17h8cd7c7807c97b1efE>
    8461:       48 8d 34 24             lea    (%rsp),%rsi
    8465:       48 89 06                mov    %rax,(%rsi)
    8468:       48 89 56 08             mov    %rdx,0x8(%rsi)
    846c:       48 8d 34 24             lea    (%rsp),%rsi
    8470:       48 8b 06                mov    (%rsi),%rax
    8473:       48 8b 56 08             mov    0x8(%rsi),%rdx
    8477:       49 b8 00 00 00 00 00    movabs $0x8000000000000000,%r8
    847e:       00 00 80
    8481:       4c 89 c1                mov    %r8,%rcx
    8484:       48 c1 f9 3f             sar    $0x3f,%rcx
    8488:       4c 39 c0                cmp    %r8,%rax
    848b:       41 0f 94 c0             sete   %r8b
    848f:       48 39 ca                cmp    %rcx,%rdx
    8492:       41 0f 94 c1             sete   %r9b
    8496:       4d 21 c8                and    %r9,%r8
    8499:       49 f7 c0 01 00 00 00    test   $0x1,%r8
    84a0:       0f 84 09 00 00 00       je     84af <_ZN3foo4main17hb0af6480194903a8E+0x62>
    84a6:       48 83 c4 50             add    $0x50,%rsp
    84aa:       48 89 ec                mov    %rbp,%rsp
    84ad:       5d                      pop    %rbp
    84ae:       c3                      ret

The closure properly zeros the top half instead:

00000000000084e7 <_ZN3foo4main28_$u7b$$u7b$closure$u7d$$u7d$17h8cd7c7807c97b1efE>:
    84e7:       55                      push   %rbp
    84e8:       48 89 e5                mov    %rsp,%rbp
    84eb:       48 b8 00 00 00 00 00    movabs $0x8000000000000000,%rax
    84f2:       00 00 80
    84f5:       48 31 d2                xor    %rdx,%rdx
    84f8:       48 89 ec                mov    %rbp,%rsp
    84fb:       5d                      pop    %rbp
    84fc:       c3                      ret

$ rustc +nightly -Vv
rustc 1.79.0-nightly (f9b161492 2024-04-19)
binary: rustc
commit-hash: f9b16149208c8a8a349c32813312716f6603eb6f
commit-date: 2024-04-19
host: x86_64-unknown-linux-gnu
release: 1.79.0-nightly
LLVM version: 18.1.4
bjorn3 commented 2 months ago

-Zmir-opt-level=2 without -Copt-level=1 reproduces this too. Looks like

v3 = icmp_imm eq v2, 0x8000_0000_0000_0000

gets legalized to

v12 = iconst.i64 0x8000_0000_0000_0000
v13 = sextend.i128 v12  ; v12 = 0x8000_0000_0000_0000
v3 = icmp eq v2, v13

which is to be expected as immediates in clif ir are signed. The problem is that cranelift_frontend::switch::icmp_imm_u128 uses icmp_imm here despite the value to check against not fitting in an i64.

bjorn3 commented 2 months ago

https://github.com/bytecodealliance/wasmtime/pull/8422 fixes this bug.

bjorn3 commented 1 month ago

Fixed with the latest Cranelift release.