ISLE: Polish reference #9905
ISLE: Polish reference #9905
Conversation
github-actions
bot
added
cranelift
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "isle"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
Couldn't seem to find the spec for |
There was a problem hiding this comment.
Hi @eagr -- some comments below; I stopped reading after the last one, about 15% of the way through. Most of these edits are changing meaning in fairly important ways, and the ones that don't seem to be minor style things / subjective preferences, so I think I'd prefer not to accept an edit pass on this document -- given the magnitude of your changes I don't want to spend the time it would take to iterate back to fully correct wording. Thanks anyway, I'm sure you intended well. I'll go ahead and close this PR.
| conditions, can be compiled down into a certain sequence of machine | ||
| instruction lowering pattern is a specification describing certain | ||
| operators in the IR (CLIF) when combined under certain | ||
| conditions, can be compiled down to a certain sequence of machine |
There was a problem hiding this comment.
This rewording is incorrect. "A ... pattern is a specification that a combination of operators ... can be compiled" can't be rewritten into "A ... pattern is a specification describing certain operators ... can be compiled". The first (current) form has a subordinate clause describing what the specification does, and the verb indicates that this specification can be compiled. Your rewrite somehow attaches two actions onto "specification" -- it describes, and it can be compiled. These aren't semantically equivalent.
| @@ -26,7 +26,7 @@ instructions. For example: | |||
| be lowered to an x86 `ADD` instruction with a register and an | |||
| immediate. | |||
|
|
|||
| One could write something like the following in ISLE (simplified from | |||
| One can write something like the following in ISLE (simplified from | |||
There was a problem hiding this comment.
This rewrite loses the meaning: the whole stanza is of the form "one could ... [but] ISLE lets you ..." and changing "could" to "can" completely changes the meaning.
| @@ -262,19 +262,19 @@ subterms: | |||
| * `_` is a wildcard and matches anything, without capturing it. | |||
|
|
|||
| * Primitive constant values, such as `42` or `$Symbol`, match only if | |||
| the term is exactly equal to this constant. | |||
| the term is an exact equivalent. | |||
There was a problem hiding this comment.
"an exact equivalent" does not read the same as "is exactly equal" to me: the former implies something about an equivalence class, different forms with the same underlying denotation, etc., whereas for primitive values (e.g. integers) we really do want to say "equal". I'm not sure what "equivalent" does better here.
| for example, `(A (B x _) z)`. This pattern would match the term `(A (B | ||
| 1 2) 3)` but not `(A (C 4 5) 6)`. | ||
|
|
||
| A pattern can properly be seen as a partial function from input term | ||
| to captured (bound) variables: it either matches or it doesn't, and if | ||
| A pattern can probably be seen as a partial function from the input term |
There was a problem hiding this comment.
One cannot replace "properly" with "probably" -- that completely changes the meaning.
Basically just typo fixing and more succinct wording, without any intent to change the original meaning.