Stream: git-wasmtime

Topic: wasmtime / issue #7999 Cranelift: Misoptimization of imul...

Wasmtime GitHub notifications bot (Feb 27 2024 at 10:18):

bjorn3 added the bug label to Issue #7999.

Wasmtime GitHub notifications bot (Feb 27 2024 at 10:18):

bjorn3 added the cranelift label to Issue #7999.

Wasmtime GitHub notifications bot (Feb 27 2024 at 10:18):

bjorn3 opened issue #7999:

.clif Test Case

set opt_level=speed
target x86_64

function u0:11(i8) -> i8 system_v {
block0(v0: i8):
    v1 = uextend.i64 v0
    v2 = imul_imm v1, 256
    v3 = ireduce.i8 v2
    return v3
}

Steps to Reproduce

• Compile reproducing test case with opt_level=speed.

Expected Results

The function unconditionally returns 0.

Actual Results

The function gets optimized to returning the argument:

function u0:11(i8) -> i8 system_v {
block0(v0: i8):
    return v0
}

Versions and Environment

Cranelift version or commit: 0.105 and 36fb62ca3bea7dd1456f12ead03084e2ceb48cda

Operating system: N/A

Architecture: x86_64

Extra Info

Found by @cbeuw in https://github.com/rust-lang/rustc_codegen_cranelift/issues/1460. Replacing i8 with i32 returns the correct result.

Wasmtime GitHub notifications bot (Feb 27 2024 at 12:38):

fitzgen commented on issue #7999:

Thanks for the bug report!

Might be related to https://github.com/bytecodealliance/wasmtime/pull/7882 -- cc @elliott

Wasmtime GitHub notifications bot (Feb 27 2024 at 17:55):

alexcrichton commented on issue #7999:

Is this perhaps the wrong reduction? With rustc this code also optimizes to return 0:

#[no_mangle]
pub extern "C" fn a(a: u8) -> u8 {
    let a = u64::from(a);
    let a = a * 256;
    a as u8
}

although I fear I may be missing something subtle by accident

Wasmtime GitHub notifications bot (Feb 27 2024 at 18:01):

elliottt commented on issue #7999:

Doing a little bit of debugging, reverting #7882 doesn't fix the problem, but reverting #7719 as well does. Here's the assembly before reverting #7719:

   0:   55                      pushq   %rbp
   1:   48 89 e5                movq    %rsp, %rbp
   4:   48 89 f8                movq    %rdi, %rax
   7:   48 89 ec                movq    %rbp, %rsp
   a:   5d                      popq    %rbp
   b:   c3                      retq

And here's the output after reverting both #7882 and #7719:

   0:   55                      pushq   %rbp
   1:   48 89 e5                movq    %rsp, %rbp
   4:   48 0f b6 c7             movzbq  %dil, %rax
   8:   48 c1 e0 08             shlq    $8, %rax
   c:   48 89 ec                movq    %rbp, %rsp
   f:   5d                      popq    %rbp
  10:   c3                      retq

I think there's an unexpected interaction of rules happening here, because in isolation the rules we have for imul x, 0 and the rules introduced in #7719 for reordering ireduce and extend should be sufficient to rewrite this to moving 0 into %rax.

Wasmtime GitHub notifications bot (Feb 27 2024 at 18:02):

alexcrichton commented on issue #7999:

Actual Results

The function gets optimized to returning the argument:

Aha that's what I was missing, actually reading! Apologies!

Wasmtime GitHub notifications bot (Feb 28 2024 at 02:28):

jameysharp closed issue #7999:

.clif Test Case

set opt_level=speed
target x86_64

function u0:11(i8) -> i8 system_v {
block0(v0: i8):
    v1 = uextend.i64 v0
    v2 = imul_imm v1, 256
    v3 = ireduce.i8 v2
    return v3
}

Steps to Reproduce

• Compile reproducing test case with opt_level=speed.

Expected Results

The function unconditionally returns 0.

Actual Results

The function gets optimized to returning the argument:

function u0:11(i8) -> i8 system_v {
block0(v0: i8):
    return v0
}

Versions and Environment

Cranelift version or commit: 0.105 and 36fb62ca3bea7dd1456f12ead03084e2ceb48cda

Operating system: N/A

Architecture: x86_64

Extra Info

Found by @cbeuw in https://github.com/rust-lang/rustc_codegen_cranelift/issues/1460. Replacing i8 with i32 returns the correct result.

Last updated: Oct 23 2024 at 20:03 UTC