Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55): Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55):eagr opened PR #9911 from eagr:pulley-sqmul to bytecodealliance:main:
part of #9783
Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55):eagr requested cfallin for a review on PR #9911.
Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55):eagr requested wasmtime-compiler-reviewers for a review on PR #9911.
Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55):eagr requested wasmtime-core-reviewers for a review on PR #9911.
Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55):eagr requested fitzgen for a review on PR #9911.
Wasmtime GitHub notifications bot (Dec 29 2024 at 09:55):eagr requested wasmtime-default-reviewers for a review on PR #9911.
Wasmtime GitHub notifications bot (Dec 29 2024 at 11:44):github-actions[bot] commented on PR #9911:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "pulley"Thus the following users have been cc'd because of the following labels:
- fitzgen: pulley
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 (Dec 29 2024 at 18:45):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 29 2024 at 18:45):alexcrichton created PR review comment:
I'm a bit surprised by the lack of parentheses around this, I would have expected:
let r = (i32::from(*a) * i32::from(b) + (1 << 14)) >> 15;as otherwise the final term
(1 << 14) >> 15looks like it currently evaluates to 0.Tests, however, are passing with this PR which means that either the precedence rules of parsing in Rust are different from what I expect or that the tests don't actually cover the case which needs the parentheses to be right here. Would you be up for debugging this a bit to figure out what's going on test-wise?
Wasmtime GitHub notifications bot (Dec 29 2024 at 19:36):eagr submitted PR review.
Wasmtime GitHub notifications bot (Dec 29 2024 at 19:36):eagr created PR review comment:
You must have always been in the good habit of applying parentheses when in doubt so you don't have to remember the precedence rules :)
I think this is equivalent to what you expect. Let me just add those parentheses and you'd see the same result. The code reads better that way anyways. And btw the tests are looking good to me.
Wasmtime GitHub notifications bot (Dec 29 2024 at 19:41):eagr updated PR #9911.
Wasmtime GitHub notifications bot (Dec 29 2024 at 20:02):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 29 2024 at 20:02):alexcrichton created PR review comment:
Oh nice, thanks for checking!
Wasmtime GitHub notifications bot (Dec 29 2024 at 20:02):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Dec 29 2024 at 20:21):alexcrichton merged PR #9911.
Last updated: Jan 24 2025 at 00:11 UTC