Skip to content

Commit

Permalink
Constant propagate int-to-float conversions (#7915)
Browse files Browse the repository at this point in the history
This commit is born out of a fuzz bug on x64 that was discovered recently.
Today, on `main`, and in the 17.0.1 release Wasmtime will panic when compiling
this wasm module for x64:

    (module
      (func (result v128)
        i32.const 0
        i32x4.splat
        f64x2.convert_low_i32x4_u))

panicking with:

    thread '<unnamed>' panicked at /home/alex/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cranelift-codegen-0.104.1/src/machinst/lower.rs:766:21:
    should be implemented in ISLE: inst = `v6 = fcvt_from_uint.f64x2 v13  ; v13 = const0`, type = `Some(types::F64X2)`
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Bisections points to the "cause" of this regression as #7859 which
more-or-less means that this has always been an issue and that PR just
happened to expose the issue. What's happening here is that egraph
optimizations are turning the IR into a form that the x64 backend can't
codegen. Namely there's no general purpose lowering of i64x2 being
converted to f64x2. The Wasm frontend never produces this but the
optimizations internally end up producing this.

Notably here the result of this function is constant and what's
happening is that a convert-of-a-splat is happening. In lieu of adding
the full general lowering to x64 (which is perhaps overdue since this is
the second or third time this panic has been triggered) I've opted to
add constant propagation optimizations for int-to-float conversions.
These are all based on the Rust `as` operator which has the same
semantics as Cranelift. This is enough to fix the issue here for the
time being.
  • Loading branch information
alexcrichton authored Feb 12, 2024
1 parent e7ab3a4 commit 8a2d9bc
Show file tree
Hide file tree
Showing 5 changed files with 279 additions and 2 deletions.
19 changes: 19 additions & 0 deletions cranelift/codegen/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,23 @@ impl<'a, 'b, 'c> generated_code::Context for IsleContext<'a, 'b, 'c> {
fn uextend_maybe_etor(&mut self, value: Value, returns: &mut Self::uextend_maybe_etor_returns) {
*returns = MaybeUnaryEtorIter::new(Opcode::Uextend, value);
}

// NB: Cranelift's defined semantics for `fcvt_from_{s,u}int` match Rust's
// own semantics for converting an integer to a float, so these are all
// implemented with `as` conversions in Rust.
fn f32_from_uint(&mut self, n: u64) -> Ieee32 {
Ieee32::with_float(n as f32)
}

fn f64_from_uint(&mut self, n: u64) -> Ieee64 {
Ieee64::with_float(n as f64)
}

fn f32_from_sint(&mut self, n: i64) -> Ieee32 {
Ieee32::with_float(n as f32)
}

fn f64_from_sint(&mut self, n: i64) -> Ieee64 {
Ieee64::with_float(n as f64)
}
}
20 changes: 20 additions & 0 deletions cranelift/codegen/src/opts/cprop.isle
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,23 @@
(bxor ty a b@(iconst _ _))
(bxor ty c d@(iconst _ _))))
(bxor ty (bxor ty a c) (bxor ty b d)))


;; Constant fold int-to-float conversions.
(rule (simplify (fcvt_from_uint $F32 (iconst_u _ n)))
(f32const $F32 (f32_from_uint n)))
(rule (simplify (fcvt_from_uint $F64 (iconst_u _ n)))
(f64const $F64 (f64_from_uint n)))
(rule (simplify (fcvt_from_sint $F32 (iconst_s _ n)))
(f32const $F32 (f32_from_sint n)))
(rule (simplify (fcvt_from_sint $F64 (iconst_s _ n)))
(f64const $F64 (f64_from_sint n)))

(decl f32_from_uint (u64) Ieee32)
(extern constructor f32_from_uint f32_from_uint)
(decl f64_from_uint (u64) Ieee64)
(extern constructor f64_from_uint f64_from_uint)
(decl f32_from_sint (i64) Ieee32)
(extern constructor f32_from_sint f32_from_sint)
(decl f64_from_sint (i64) Ieee64)
(extern constructor f64_from_sint f64_from_sint)
5 changes: 3 additions & 2 deletions cranelift/codegen/src/opts/vector.isle
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
;; For various ops lift a splat outside of the op to try to open up
;; optimization opportunities with scalars.
;;

;; NB: for int-to-float conversion op this simplification is also
;; required for correctness at this time due to #6562
;; required for the x64 backend because it doesn't fully implement int-to-float
;; conversions for 64x2 vectors, for more information see #6562
(rule (simplify (fcvt_from_uint float_vector_ty (splat _ x)))
(splat float_vector_ty (fcvt_from_uint (lane_type float_vector_ty) x)))
(rule (simplify (fcvt_from_sint float_vector_ty (splat _ x)))
Expand Down
222 changes: 222 additions & 0 deletions cranelift/filetests/filetests/egraph/fcvt-from-int.clif
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
test optimize
set opt_level=speed
target x86_64
target aarch64
target s390x
target riscv64

function %i32_0_to_f32() -> f32 {
block0:
v0 = iconst.i32 0
v1 = fcvt_from_sint.f32 v0
return v1
; check: v2 = f32const 0.0
; check: return v2
}

function %i32_neg1_to_f32() -> f32 {
block0:
v0 = iconst.i32 -1
v1 = fcvt_from_sint.f32 v0
return v1
; check: v2 = f32const -0x1.000000p0
; check: return v2
}

function %i32_1_to_f32() -> f32 {
block0:
v0 = iconst.i32 1
v1 = fcvt_from_sint.f32 v0
return v1
; check: v2 = f32const 0x1.000000p0
; check: return v2
}

function %u32_0_to_f32() -> f32 {
block0:
v0 = iconst.i32 0
v1 = fcvt_from_uint.f32 v0
return v1
; check: v2 = f32const 0.0
; check: return v2
}

function %u32_neg1_to_f32() -> f32 {
block0:
v0 = iconst.i32 -1
v1 = fcvt_from_uint.f32 v0
return v1
; check: v2 = f32const 0x1.000000p32
; check: return v2
}

function %u32_1_to_f32() -> f32 {
block0:
v0 = iconst.i32 1
v1 = fcvt_from_uint.f32 v0
return v1
; check: v2 = f32const 0x1.000000p0
; check: return v2
}

function %i32_0_to_f64() -> f64 {
block0:
v0 = iconst.i32 0
v1 = fcvt_from_sint.f64 v0
return v1
; check: v2 = f64const 0.0
; check: return v2
}

function %i32_neg1_to_f64() -> f64 {
block0:
v0 = iconst.i32 -1
v1 = fcvt_from_sint.f64 v0
return v1
; check: v2 = f64const -0x1.0000000000000p0
; check: return v2
}

function %i32_1_to_f64() -> f64 {
block0:
v0 = iconst.i32 1
v1 = fcvt_from_sint.f64 v0
return v1
; check: v2 = f64const 0x1.0000000000000p0
; check: return v2
}

function %u32_0_to_f64() -> f64 {
block0:
v0 = iconst.i32 0
v1 = fcvt_from_uint.f64 v0
return v1
; check: v2 = f64const 0.0
; check: return v2
}

function %u32_neg1_to_f64() -> f64 {
block0:
v0 = iconst.i32 -1
v1 = fcvt_from_uint.f64 v0
return v1
; check: v2 = f64const 0x1.fffffffe00000p31
; check: return v2
}

function %u32_1_to_f64() -> f64 {
block0:
v0 = iconst.i32 1
v1 = fcvt_from_uint.f64 v0
return v1
; check: v2 = f64const 0x1.0000000000000p0
; check: return v2
}

function %i64_0_to_f32() -> f32 {
block0:
v0 = iconst.i64 0
v1 = fcvt_from_sint.f32 v0
return v1
; check: v2 = f32const 0.0
; check: return v2
}

function %i64_neg1_to_f32() -> f32 {
block0:
v0 = iconst.i64 -1
v1 = fcvt_from_sint.f32 v0
return v1
; check: v2 = f32const -0x1.000000p0
; check: return v2
}

function %i64_1_to_f32() -> f32 {
block0:
v0 = iconst.i64 1
v1 = fcvt_from_sint.f32 v0
return v1
; check: v2 = f32const 0x1.000000p0
; check: return v2
}

function %u64_0_to_f32() -> f32 {
block0:
v0 = iconst.i64 0
v1 = fcvt_from_uint.f32 v0
return v1
; check: v2 = f32const 0.0
; check: return v2
}

function %u64_neg1_to_f32() -> f32 {
block0:
v0 = iconst.i64 -1
v1 = fcvt_from_uint.f32 v0
return v1
; check: v2 = f32const 0x1.000000p64
; check: return v2
}

function %u64_1_to_f32() -> f32 {
block0:
v0 = iconst.i64 1
v1 = fcvt_from_uint.f32 v0
return v1
; check: v2 = f32const 0x1.000000p0
; check: return v2
}

function %i64_0_to_f64() -> f64 {
block0:
v0 = iconst.i64 0
v1 = fcvt_from_sint.f64 v0
return v1
; check: v2 = f64const 0.0
; check: return v2
}

function %i64_neg1_to_f64() -> f64 {
block0:
v0 = iconst.i64 -1
v1 = fcvt_from_sint.f64 v0
return v1
; check: v2 = f64const -0x1.0000000000000p0
; check: return v2
}

function %i64_1_to_f64() -> f64 {
block0:
v0 = iconst.i64 1
v1 = fcvt_from_sint.f64 v0
return v1
; check: v2 = f64const 0x1.0000000000000p0
; check: return v2
}

function %u64_0_to_f64() -> f64 {
block0:
v0 = iconst.i64 0
v1 = fcvt_from_uint.f64 v0
return v1
; check: v2 = f64const 0.0
; check: return v2
}

function %u64_neg1_to_f64() -> f64 {
block0:
v0 = iconst.i64 -1
v1 = fcvt_from_uint.f64 v0
return v1
; check: v2 = f64const 0x1.0000000000000p64
; check: return v2
}

function %u64_1_to_f64() -> f64 {
block0:
v0 = iconst.i64 1
v1 = fcvt_from_uint.f64 v0
return v1
; check: v2 = f64const 0x1.0000000000000p0
; check: return v2
}
15 changes: 15 additions & 0 deletions tests/misc_testsuite/int-to-float-splat.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
(module
(func (param i32) (result v128)
local.get 0
i32x4.splat
f64x2.convert_low_i32x4_u
)
)

(module
(func (result v128)
i32.const 0
i32x4.splat
f64x2.convert_low_i32x4_u
)
)

0 comments on commit 8a2d9bc

Please sign in to comment.