Cranelift: "constant immediate is out of bounds"

Kmeakin commented 3 months ago

Adding this optimization to codegen/src/opts/icmp.isle

;; sge(x, c) == sgt(x, c-1), for c != SMIN.
(rule (simplify (sge (fits_in_64 (ty_int bty)) x (iconst cty (u64_from_imm64 c))))
      (if-let $false (u64_eq c (ty_smin cty)))
      (sgt bty x (iconst cty (imm64 (u64_sub c 1)))))

Triggers a verification error when optimizing filetests/filetests/egraph/icmp-parameterized.clif:

FAIL filetests/filetests/egraph/icmp-parameterized.clif: optimize

Caused by:
    function %icmp_sgt_umax(i32) -> i8 fast {
    block0(v0: i32):
        v5 = iconst.i32 -1
    ;   ^~~~~~~~~~~~~~~~~~
    ; error: inst5 (v5 = iconst.i32 -1): constant immediate is out of bounds

        v10 = icmp sgt v0, v5  ; v5 = -1
        return v10
    }

    ; 1 verifier error detected (see above). Compilation aborted.

I suspect somewhere the immediate is being sign extended when it should be zero extended

bjorn3 commented 3 months ago

-1 in clif ir text files is -1i64, which is the same as u64::MAX. Cranelift used to ignore the upper half of the immediate, but was changed to expect the upper half to be zeroed. We didn't yet change imm64 to uimm64 yet though, so immediates are still represented as signed integers, despite unsigned integers effectively being expected by the backend.

Kmeakin commented 3 months ago

~Fixed by https://github.com/bytecodealliance/wasmtime/commit/0683b84b40207d8ec0c9044ef418fbd1f23a62f9~

bytecodealliance / wasmtime

Cranelift: "constant immediate is out of bounds" #9041