-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Winch: Different results than Cranelift with floating-point f64.ne
instruction
#8632
Comments
Subscribe to Label Action
This issue or pull request has been labeled: "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
For reference the compiler outputs for this function are: cranelift
winch
|
Thanks for catching this one, Alex! I'll take a look. |
A quick update: I've identified the root cause and I'm working on fix. There's a bug in the stack shuffling algorithm which is causing truncation of values when handling multi-value returns. |
Fixes: bytecodealliance#8632 This commit fixes the handling of f64 comparison operations in Winch. f64 comparison operations are defined as [f64 f64] -> [i32] The previous implemementation was widening the result to `[i64]` which caused issues which stack shuffling in multi-value returns.
The stack shuffling algorithm was a bit of a red herring. Here's a fix: https://github.com/bytecodealliance/wasmtime/pull/8685/files |
Fixes: #8632 This commit fixes the handling of f64 comparison operations in Winch. f64 comparison operations are defined as [f64 f64] -> [i32] The previous implemementation was widening the result to `[i64]` which caused issues which stack shuffling in multi-value returns.
Given this input:
I locally get:
Notably the second return value here, the
f64.ne
branch, differs in Winch and Cranelift. I believe that Cranelift is correct in this case, hence the bug report for winch.cc @saulecabrera
The text was updated successfully, but these errors were encountered: