Stream: git-wasmtime

Topic: wasmtime / PR #7719 Simplify ireduce of modular operators...

Wasmtime GitHub notifications bot (Dec 21 2023 at 20:01):

scottmcm requested elliottt for a review on PR #7719.

Wasmtime GitHub notifications bot (Dec 21 2023 at 20:01):

scottmcm opened PR #7719 from scottmcm:modular-reduce to bytecodealliance:main:

This is a follow-up to fitzgen's comment in https://github.com/bytecodealliance/wasmtime/pull/7693#discussion_r1430497487

The version that was removed from that PR had some concerns because it added things to the egraph that might not be useful, since it added more instructions, potentially: going from (small)(x OP y) to (small)x OP (small)y isn't always a win since it's one more CLIF instruction.

This PR addresses that concern by restricting the pattern only to places where it's going to be strictly fewer total instructions. It does that by only applying when all the arguments to the inner operator are either extended or constants. In those cases, pushing the ireduce into the arguments will collapse with the instruction that's already there -- either because extend-then-reduce back is a nop, because cprop will simplify it, or thanks to #7711.

The particular motivating example for this -- which is why the mulhi pattern is in here too -- is Rust's u64::widening_mul. LLVM doesn't have an intrinsic for wide multiplication, so the way it's often written is by casting the arguments to a wider type, doing the multiplication, then reducing back again. But that means the obvious translation in CLIF ends up with an expensive 128-bit multiplication. Collapsing that down to a 64-bit imul+umulhi is much cheaper.

Wasmtime GitHub notifications bot (Dec 21 2023 at 20:01):

scottmcm requested wasmtime-compiler-reviewers for a review on PR #7719.

Wasmtime GitHub notifications bot (Dec 21 2023 at 22:45):

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

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 (Dec 22 2023 at 05:18):

elliottt requested fitzgen for a review on PR #7719.

Wasmtime GitHub notifications bot (Jan 02 2024 at 17:23):

fitzgen submitted PR review:

Thanks!

Wasmtime GitHub notifications bot (Jan 02 2024 at 18:51):

alexcrichton merged PR #7719.

Wasmtime GitHub notifications bot (Feb 06 2024 at 19:32):

alexcrichton commented on PR #7719:

A bisection I just ran points to this PR as the cause of https://github.com/bytecodealliance/wasmtime/issues/7874 (cc'ing here to close the loop)

Last updated: Oct 23 2024 at 20:03 UTC