Wasmtime GitHub notifications bot (Apr 26 2022 at 07:38): Wasmtime GitHub notifications bot (Apr 26 2022 at 07:38):cfallin opened PR #4072 from isle-extended-lhs-givens to main:
This PR adds support for
givenclauses, as proposed in
rfcs#21. These clauses augment the left-hand side (pattern-matching
phase) of rules in the ISLE instruction-selection DSL with
sub-patterns matching on sub-expressions. The ability to list
additional match operations to perform, out-of-line from the original
toplevel pattern, greatly simplifies some idioms. See the RFC for more
details and examples of use.Creating this as a draft since the RFC is pending; the intent for now
is to provide an implementation to play with, given initial positive feedback,
but this will not merge until after the RFC does.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 26 2022 at 07:39):cfallin edited PR #4072 from isle-extended-lhs-givens to main:
This PR adds support for
givenclauses, as proposed in
RFC #21. These clauses augment the left-hand side (pattern-matching
phase) of rules in the ISLE instruction-selection DSL with
sub-patterns matching on sub-expressions. The ability to list
additional match operations to perform, out-of-line from the original
toplevel pattern, greatly simplifies some idioms. See the RFC for more
details and examples of use.Creating this as a draft since the RFC is pending; the intent for now
is to provide an implementation to play with, given initial positive feedback,
but this will not merge until after the RFC does.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 28 2022 at 20:57):cfallin updated PR #4072 from isle-extended-lhs-givens to main.
Wasmtime GitHub notifications bot (Apr 28 2022 at 20:58):cfallin has marked PR #4072 as ready for review.
Wasmtime GitHub notifications bot (Apr 28 2022 at 21:06):cfallin updated PR #4072 from isle-extended-lhs-givens to main.
Wasmtime GitHub notifications bot (Apr 28 2022 at 21:08):cfallin edited PR #4072 from isle-extended-lhs-givens to main:
This PR adds support for
givenclauses, as proposed in
bytecodealliance/rfcs#21. These clauses augment the left-hand side (pattern-matching
phase) of rules in the ISLE instruction-selection DSL with
sub-patterns matching on sub-expressions. The ability to list
additional match operations to perform, out-of-line from the original
toplevel pattern, greatly simplifies some idioms. See the RFC for more
details and examples of use.Creating this as a draft since the RFC is pending; the intent for now
is to provide an implementation to play with, given initial positive feedback,
but this will not merge until after the RFC does.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 28 2022 at 21:08):cfallin edited PR #4072 from isle-extended-lhs-givens to main:
This PR adds support for
givenclauses, as proposed in
bytecodealliance/rfcs#21. These clauses augment the left-hand side (pattern-matching
phase) of rules in the ISLE instruction-selection DSL with
sub-patterns matching on sub-expressions. The ability to list
additional match operations to perform, out-of-line from the original
toplevel pattern, greatly simplifies some idioms. See the RFC for more
details and examples of use.Creating this as a draft since the RFC is pending; the intent for now
is to provide an implementation to play with, given initial positive feedback,
but this will not merge until after the RFC does.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 28 2022 at 21:08):cfallin edited PR #4072 from isle-extended-lhs-givens to main:
This PR adds support for
if-letclauses, as proposed in
bytecodealliance/rfcs#21. These clauses augment the left-hand side (pattern-matching
phase) of rules in the ISLE instruction-selection DSL with
sub-patterns matching on sub-expressions. The ability to list
additional match operations to perform, out-of-line from the original
toplevel pattern, greatly simplifies some idioms. See the RFC for more
details and examples of use.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:09):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:09):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:09):fitzgen created PR review comment:
I sort of wonder whether we should have some sort of grouping so that it is more clear that the
if-lets are part of the LHS, and not the RHS? Because right now it could syntactically go either way if you don't already know what's going on.
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:47):cfallin updated PR #4072 from isle-extended-lhs-givens to main.
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:49):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:49):cfallin created PR review comment:
It's a good question for sure. IMHO it gets sort of complicated if we take as granted that we don't want to change syntax for cases without if-let clauses... that implies that (if we want straightforward parsing without unlimited lookahead at least) cases with if-lets need a wrapper form around the pattern, like
(rule (let PAT (if-let ...) (if-let ...)) RHS)maybe, reusing theletkeyword. It could work; happy to discuss further and we can always refactor if we find an improved syntax :-)
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:56):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2022 at 22:56):fitzgen created PR review comment:
Yeah, that's basically what peepmatic used:
(=> PATTERN RHS)and(=> (when PATTERN PREDICATES...) RHS). I didn't love it but it did work.
Wasmtime GitHub notifications bot (Apr 28 2022 at 23:36):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Apr 28 2022 at 23:36):cfallin created PR review comment:
Hmm, yeah, it's not immediately clear to me that this is less confusing -- seeing the prefix/wrapper form first, if one doesn't know what it does, is possibly worse than seeing a top-level pattern that looks like the other rules and then additional conditions separately...
Regarding which part of the rule the
if-letclauses are a part of: if I squint the right way, it seems like it actually kind of doesn't matter? In other words I can build a consistent mental model for what's going on as "go down the list of patterns/conditions, and when we reach the end we have the answer". That's a level of understanding that's more basic than the strict phase separation we talk about in the term-rewriting flow, but it's (IMHO) not too bad for a casual user tweaking some rules.I think I'll err on the side of the syntax in the RFC and go ahead and merge this but absolutely we can still talk more about it if you (or anyone else) have further thoughts!
Wasmtime GitHub notifications bot (Apr 28 2022 at 23:37):cfallin merged PR #4072.
Last updated: Dec 23 2024 at 12:05 UTC