PCC: draw the rest of the owl: fully-working PCC on hello-world Wasm on aarch64 #7281
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
cranelift
fitzgen
reviewed
Oct 18, 2023
…on aarch64 This needed a bit more inference / magic than I was hoping for at first, specifically around constants and adds. Some instructions can now generate facts on their output registers, even if not stated. This breaks away from the "breadcrumbs" idea, but seems to be the most practical solution to a general problem that we have mini-lowering steps in various places without careful preservation of PCC facts. Two particular aspects: - Constants: amodes on aarch64 can decompose into new constant-generation instructions, and we need precise ranges on those to properly check them. To avoid making the ISLE rules nightmarish, it's best to reuse the existing semantics definitions of the Add* ALU insts, and add a few rules for MovK/MovZ/MovN. - Adds: similarly, amodes decompose into de-novo add instructions with no facts. To handle this, there's now a notion of "propagating" facts that cause an instruction with a propagating fact on the input to generate a fact on the output. Together, these heuristics mean that we'll eagerly generate a fact for `mem(mt0, 0, 0) + 8 -> mem(mt0, 8, 8)`, but we won't, for example, generate ranges on every single integer operation. With these changes and a few other misc fixes, this PR can now check a nontrivial "hello world" Wasm on aarch64 down to the machine-code level: ``` $ target/release/wasmtime compile -C enable-pcc=y ../wasm-tests/helloworld-rs.wasm ```
|
@fitzgen -- updated in live review, ready for an r+ now I think! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This needed a bit more inference / magic than I was hoping for at first,
specifically around constants and adds. Some instructions can now
generate facts on their output registers, even if not stated. This
breaks away from the "breadcrumbs" idea, but seems to be the most
practical solution to a general problem that we have mini-lowering steps
in various places without careful preservation of PCC facts. Two
particular aspects:
Constants: amodes on aarch64 can decompose into new
constant-generation instructions, and we need precise ranges on those
to properly check them. To avoid making the ISLE rules nightmarish,
it's best to reuse the existing semantics definitions of the Add* ALU
insts, and add a few rules for MovK/MovZ/MovN.
Adds: similarly, amodes decompose into de-novo add instructions with
no facts. To handle this, there's now a notion of "propagating" facts
that cause an instruction with a propagating fact on the input to
generate a fact on the output.
Together, these heuristics mean that we'll eagerly generate a fact for
mem(mt0, 0, 0) + 8 -> mem(mt0, 8, 8), but we won't, for example,generate ranges on every single integer operation.
With these changes and a few other misc fixes, this PR can now check a
nontrivial "hello world" Wasm on aarch64 down to the machine-code level:
(This builds on #7280, and I also intend to clean it up a bit, so creating as a draft for now)