Stream: git-wasmtime

Topic: wasmtime / PR #7915 Constant propagate int-to-float conve...


view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):

alexcrichton opened PR #7915 from alexcrichton:fix-x64-panic to bytecodealliance:main:

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.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):

alexcrichton requested cfallin for a review on PR #7915.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #7915.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):

alexcrichton requested pchickey for a review on PR #7915.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):

alexcrichton requested wasmtime-core-reviewers for a review on PR #7915.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:10):

cfallin submitted PR review:

Thanks! I agree we should implement the general op eventually but this is a fine band-aid for now...

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:10):

cfallin has enabled auto merge for PR #7915.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 17:44):

github-actions[bot] commented on PR #7915:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "isle"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 18:05):

cfallin merged PR #7915.


Last updated: Dec 23 2024 at 12:05 UTC