Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06): 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 backtraceBisections 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
asoperator 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:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):alexcrichton requested cfallin for a review on PR #7915.
Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #7915.
Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):alexcrichton requested pchickey for a review on PR #7915.
Wasmtime GitHub notifications bot (Feb 12 2024 at 17:06):alexcrichton requested wasmtime-core-reviewers for a review on PR #7915.
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...
Wasmtime GitHub notifications bot (Feb 12 2024 at 17:10):cfallin has enabled auto merge for PR #7915.
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:
- cfallin: isle
- fitzgen: isle
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Feb 12 2024 at 18:05):cfallin merged PR #7915.
Last updated: Oct 23 2024 at 20:03 UTC