Wasmtime GitHub notifications bot (Nov 07 2025 at 06:52): 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.htmlPlease 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 thanslts (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 ==> falserule, 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)
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.
Wasmtime GitHub notifications bot (Aug 10 2026 at 05:00):bongjunj updated PR #12001.
Wasmtime GitHub notifications bot (Aug 10 2026 at 05:12)::memo: bongjunj submitted PR review.
Wasmtime GitHub notifications bot (Aug 10 2026 at 05:12)::speech_balloon: bongjunj created PR review comment:
@fitzgen Hi, this PR had totally slipped my mind.
I have re-evaluated your generalized optimization in my thought experiment,
and I think we better not to include this without a clear evidence of performance improvement.
The rule looks closer to a canonicalization, than a direct optimization.
This only rewrites an operation followed by two selects to a single select followed by two operations.
Therefore, there is a risk where the compile time is wasted only to grow the intermediate e-graph, not contributing to the total reduction of the runtime.While the rule indeed reduces the cost of the computation according to the cost model (as a simple operation like add has a cost of 3, and a select has a cost of 4), the difference is subtle. I have attached the cost model in use at the bottom.
In addition, the change of a select instruction can interfere with the control-flow structure of the input program, which could lead to unexpected performance degradation.
If it were for simple arithmetic/bitwise operations, the reduction clearly contributes to performance improvement since there is no branch behavior. But, here with branches, I'm not sure which one is better.For these reasons, I would like to close this PR at the moment until we clearly understand the optimization opportunities related to this transformation. Thanks for reading.
Wasmtime GitHub notifications bot (Aug 10 2026 at 05:13)::speech_balloon: bongjunj edited PR review comment.
Wasmtime GitHub notifications bot (Aug 10 2026 at 05:16)::speech_balloon: bongjunj edited PR review comment.
Wasmtime GitHub notifications bot (Aug 10 2026 at 05:16)::cross_mark: bongjunj closed without merge PR #12001.
Last updated: Aug 30 2026 at 09:07 UTC