Wasmtime GitHub notifications bot (Feb 28 2024 at 18:15): Wasmtime GitHub notifications bot (Feb 28 2024 at 18:15):cfallin opened PR #8015 from cfallin:isle-opt-readme-7999 to bytecodealliance:main:
While debugging #7999, we learned of a possible, extremely subtle, interaction between the design of our ISLE mid-end prelude and matching rules with a certain structure. Because a
Valuerepresents an entire eclass, separate left-hand sides (as in helper rules or if-let clauses) that match on the value may match against different enodes returned by the multi-match extractor's iterator. We then may infer some property of one enode, another property of another enode, and perform a rewrite that is only valid if both of those matches are on the same enode.The precise distinction is whether it is a property of the value -- e.g., nonzero, or even, or within a range -- or a property of the operation and matched subpieces. The former is fine, the latter runs into trouble. We found that #7719 added a helper that determined whether a value "was a certain operator" -- actually, had any enode of a certain operator -- and separately, matched "the operator" and extracted its opcode and parameters (actually, any binary operator). The first half can match an opcode we support simplifying, and the second half can get the arguments and
opand blindly use them in the rewrite.This PR adds new guidance to avoid complex helpers and be aware of multi-matching behavior, preferring to write patterns directly (as the fix in #8005 does) instead. Longer-term, we also have other ideas, e.g. @jameysharp's suggestion to disallow at-patterns on multi-extractors in left hand sides to reduce the chance of hitting this footgun.
<!--
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 (Feb 28 2024 at 18:15):cfallin requested wasmtime-compiler-reviewers for a review on PR #8015.
Wasmtime GitHub notifications bot (Feb 28 2024 at 18:15):cfallin requested fitzgen for a review on PR #8015.
Wasmtime GitHub notifications bot (Feb 28 2024 at 18:41):jameysharp submitted PR review:
This is very helpful guidance, thanks for writing it!
Wasmtime GitHub notifications bot (Feb 28 2024 at 19:23):cfallin merged PR #8015.
Last updated: Oct 23 2024 at 20:03 UTC