wasmtime / PR #6140 ISLE: split algebraic.isle into sever... · git-wasmtime

I've verified that the new split-up .isle files have everything from the old algebraic.isle, except for this comment. It wasn't obvious where this comment should go, but I want to keep it somewhere, and eventually add more advice for rule authors.

I finally figured out that where I would like you to put this paragraph is in a new README.md inside this opts directory. You don't have to add any new documentation, but that would be a great place if you want to write down anything that you've figured out.

Wasmtime GitHub notifications bot (Apr 07 2023 at 00:50):

Kmeakin updated PR #6140.

Wasmtime GitHub notifications bot (Apr 07 2023 at 00:52):

Kmeakin submitted PR review.

Wasmtime GitHub notifications bot (Apr 07 2023 at 00:52):

Kmeakin created PR review comment:

I'm a bit confused what this comment means. It states that " we cannot pull a new eclass id out of thin air and refer to it", but then immediately contradicts itself with "other
than a piece of the input or a new node that we construct"

Wasmtime GitHub notifications bot (Apr 07 2023 at 01:00):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Apr 07 2023 at 01:00):

jameysharp created PR review comment:

I can see why that's confusing as written, although I'm not yet sure how to fix it.

The right-hand side of a rule must not use a Value unless it either pattern-matched it, or created it from a new instruction.

So the example of x+y-y→x is okay because x came from the left-hand side pattern. Also x+x→x*2 is okay because it creates new instructions for 2 and *. But x+y→z is not okay, where z is any other Value from somewhere else in the function.

Wasmtime GitHub notifications bot (Apr 07 2023 at 02:46):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 07 2023 at 02:46):

cfallin created PR review comment:

Yeah, it's not so much a "contradiction" as a refinement: these are the only kinds of values one may use.

Maybe another to say it is: the leaves of the right hand side must be pieces from the left hand side.

Wasmtime GitHub notifications bot (Apr 07 2023 at 02:51):

Kmeakin submitted PR review.

Wasmtime GitHub notifications bot (Apr 07 2023 at 02:51):

Kmeakin created PR review comment:

So the example of x+y-y→x is okay because x came from the left-hand side pattern. Also x+x→x*2 is okay because it creates new instructions for 2 and *. But x+y→z is not okay, where z is any other Value from somewhere else in the function.

Doesn't that just follow from the requirement that ISLE programs must be well scoped (ie not refer to variables that have not been bound)? Seems so obvious that it wouldn't need documenting. But can keep it in if you prefer

Wasmtime GitHub notifications bot (Apr 10 2023 at 15:56):

Kmeakin requested jameysharp for a review on PR #6140.

Wasmtime GitHub notifications bot (Apr 10 2023 at 15:56):

Kmeakin requested cfallin for a review on PR #6140.

Wasmtime GitHub notifications bot (Apr 10 2023 at 23:57):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Apr 10 2023 at 23:57):

jameysharp created PR review comment:

Yes, I would prefer to keep that text in a README.md. We can improve the text later.

Wasmtime GitHub notifications bot (Apr 11 2023 at 21:04):

Kmeakin updated PR #6140.

Wasmtime GitHub notifications bot (Apr 11 2023 at 21:05):

Kmeakin updated PR #6140.

Wasmtime GitHub notifications bot (Apr 11 2023 at 21:22):

jameysharp updated PR #6140.

Wasmtime GitHub notifications bot (Apr 11 2023 at 21:25):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Apr 11 2023 at 21:26):

jameysharp has enabled auto merge for PR #6140.

Wasmtime GitHub notifications bot (Apr 11 2023 at 22:19):

jameysharp merged PR #6140.

Last updated: Dec 23 2024 at 13:07 UTC