Stream: git-wasmtime

Topic: wasmtime / PR #15052 Cranelift: ISLE: Clif type args shou...

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 21:03):

avanhatt opened PR #15052 from avanhatt:isle-ctrl-ty to bytecodealliance:main:

Per discussion with @cfallin, when a Clif ISLE term has a type parameter, the type should always be the controlling type from the Clif definition (e.g., the width of x/y in (icmp ty x y)), not the result type (e.g., i8) when they differ. Make this change and update the affected rules (257 of them) and ISLE specs. This requires introducing a has_type helper into opt for cases where rules do intend to match on the result type.

Note this had the nice consequence of improving GVN due to it previously failing to deduplicate across the version of the instruction that correctly used the controlling type (before the egraph) and the one using the result type (in the egraph). I have not yet tried to measure if this has any performance impact. This PR also fixes more cases where opt rules were only firing at i8 due to the conflation of the two types (related to #14964).

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 21:04):

avanhatt requested fitzgen for a review on PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 21:04):

avanhatt removed the review request for wasmtime-compiler-reviewers from PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 21:04):

avanhatt requested wasmtime-compiler-reviewers for a review on PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 23:44):

github-actions[bot] added the label isle on PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 23:44):

github-actions[bot] added the label cranelift on PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 23:44):

github-actions[bot] added the label cranelift:meta on PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2026 at 00:56):

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

Subscribe to Label Action

cc @avanhatt, @cfallin, @fitzgen, @mmcloughlin

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "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 (Oct 29 2026 at 01:10):

:thumbs_up: cfallin submitted PR review:

This looks correct to me, after going over the rule edits -- thanks for the careful work!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2026 at 01:10):

cfallin added PR #15052 Cranelift: ISLE: Clif type args should always be controlling type to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2026 at 01:34):

:check: cfallin merged PR #15052.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2026 at 01:34):

cfallin removed PR #15052 Cranelift: ISLE: Clif type args should always be controlling type from the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2026 at 14:40):

fitzgen commented on PR #15052:

Hi @avanhatt, it looks like this PR caused a +0.68% geomean virtual cycles (+0.75% instructions retired) regression on our Sightglass benchmarks, and +1.25% virtual cycles (+1.37% instructions retired) regression on shootout-keccak in particular.

Details and reproduction info: https://github.com/bytecodealliance/are-we-fast-yet/issues/14

(Note that the callgrind measure is linux only, but -m insts-retired works on macos as well).

This doesn't mean we need to revert this change, especially because IIUC it fixes a bug, but it would be appreciated if you could investigate whether there are any easy wins available to do this same work in fewer cycles/instructions.

Happy to answer questions if you have any or help out if you don't have cycles.

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 29 2026 at 14:42):

fitzgen edited a comment on PR #15052:

Hi @avanhatt, it looks like this PR caused a +0.68% geomean virtual cycles (+0.75% instructions retired) compilation regression on our Sightglass benchmarks, and a +1.25% virtual cycles (+1.37% instructions retired) compilation regression on shootout-keccak in particular.

Details and reproduction info: https://github.com/bytecodealliance/are-we-fast-yet/issues/14

(Note that the callgrind measure is linux only, but -m insts-retired works on macos as well).

This doesn't mean we need to revert this change, especially because IIUC it fixes a bug, but it would be appreciated if you could investigate whether there are any easy wins available to do this same work in fewer cycles/instructions.

Happy to answer questions if you have any or help out if you don't have cycles.

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 30 2026 at 17:04):

avanhatt commented on PR #15052:

This doesn't mean we need to revert this change, especially because IIUC it fixes a bug, but it would be appreciated if you could investigate whether there are any easy wins available to do this same work in fewer cycles/instructions.

Taking a look and will report back, thanks!

Last updated: Oct 31 2026 at 23:03 UTC