Stream: git-wasmtime

Topic: wasmtime / PR #12001 [Cranelift] `(n < m) → ((if c then m...

Wasmtime GitHub notifications bot (Nov 07 2025 at 06:52):

bongjunj opened PR #12001 from bongjunj:select_slt_falase to bytecodealliance:main:

…= false`

<!--
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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Nov 07 2025 at 06:52):

bongjunj requested fitzgen for a review on PR #12001.

Wasmtime GitHub notifications bot (Nov 07 2025 at 06:52):

bongjunj requested wasmtime-compiler-reviewers for a review on PR #12001.

Wasmtime GitHub notifications bot (Nov 07 2025 at 09:45):

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

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 (Nov 07 2025 at 17:40):

fitzgen submitted PR review:

Thanks again for finding another missing optimization! Comment below with a suggested way to do this a little differently.

Wasmtime GitHub notifications bot (Nov 07 2025 at 17:41):

fitzgen created PR review comment:

It might make sense to add a general rule to dedupe selects, something like this:

(rule (simplify (slt ty (select _ cond a b)
                        (select _ cond c d)))
      (select ty cond (slt ty cond a c)
                      (slt ty cond b d)))

I think this could be an intermediate step that would reveal optimization possibilities for existing small rules and effectively subsume this larger rule. I think this is also beneficial on its own, since selects should generally be more expensive than slts (although I am not sure that our cost functions encode that at the moment), so I'm not worried about unnecessarily blowing up the enode count in this case.

All that said, we would really want the equivalent of this rule for ~all operators, not just slt:

(rule (simplify (iadd ty (select _ cond a b)
                         (select _ cond c d)))
      (select ty cond (iadd ty cond a c)
                      (iadd ty cond b d)))

And all those rules would be annoying to write in ISLE today without macros or higher-order terms.

But then again, roughly the same could be said about this rule as-is (it is combining a cprop rule, a x < x ==> false rule, and the pull-selects-out rule I sketched above; we could do the same kind of thing for all other operators' rules by combining them with their own version of the pull-selects-out rule).

So after writing all this out, I think I have convinced myself that my proposed intermediate rule is the way to go, rather than writing out the "combined" rule as you have here. (And we don't need to add all the other operator variants of that rule now, but probably should eventually.) But we should check that adding that rule really is enough to do the "combined" rewrite you've proposed in this PR. We should be able to check that via your existing tests.

Does all that make sense?

Wasmtime GitHub notifications bot (Nov 07 2025 at 17:41):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Nov 07 2025 at 18:08):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Nov 07 2025 at 18:08):

fitzgen created PR review comment:

(although I am not sure that our cost functions encode that at the moment)

https://github.com/bytecodealliance/wasmtime/pull/12006

Wasmtime GitHub notifications bot (Nov 09 2025 at 14:31):

bongjunj submitted PR review.

Wasmtime GitHub notifications bot (Nov 09 2025 at 14:31):

bongjunj created PR review comment:

Hi, just want to let you know it could take a while to leave my thoughts here due to my schedule. Gonna come back later soon! Thanks for your thoughtful comment.

Wasmtime GitHub notifications bot (Nov 09 2025 at 23:13):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Nov 09 2025 at 23:13):

fitzgen created PR review comment:

For sure, no rush! I’m out of office visiting family for a week or so, so I will also be slow to respond

Wasmtime GitHub notifications bot (Nov 12 2025 at 08:52):

bongjunj edited PR #12001.

Last updated: Dec 06 2025 at 06:05 UTC