fitzgen requested cfallin for a review on PR #9089.
fitzgen requested wasmtime-default-reviewers for a review on PR #9089.
fitzgen opened PR #9089 from fitzgen:pulley-cranelift-backend
to bytecodealliance:main
:
This commit adds two new backends for Cranelift that emits 32- and 64-bit Pulley
bytecode. The backends are both actually the same, with a common implementation
living incranelift/codegen/src/isa/pulley_shared
. Each backend configures an
ISA flag that determines the pointer size, and lowering inspects this flag's
value when lowering memory accesses.To avoid multiple ISLE compilation units, and to avoid compiling duplicate
copies of Pulley's generatedMInst
, I couldn't useMInst
as theMachInst
implementation directly. Instead, there is anInstAndKind
type that is a
newtype over the generatedMInst
but which also carries a phantom type
parameter that implements thePulleyTargetKind
trait. There are two
implementations of this trait, a 32- and 64-bit version. This is necessary
because there are various static trait methods for the mach backend which we
must implement, and which return the pointer width, but don't have access to any
self
. Therefore, we are forced to monomorphize some amount of code. This type
parameter is fairly infectious, and all the "big" backend
types (PulleyBackend<P>
,PulleyABICallSite<P>
, etc...) are parameterized
over it. Nonetheless, not everything is parameterized over aPulleyTargetKind
,
and we manage to avoid duplicateMInst
definitions and lowering code.Note that many methods are still stubbed out with
todo!
s. It is expected that
we will fill in those implementations as the work on Pulley progresses.
The first commit is https://github.com/bytecodealliance/wasmtime/pull/9085; this depends on that PR.
fitzgen requested wasmtime-compiler-reviewers for a review on PR #9089.
fitzgen updated PR #9089.
fitzgen updated PR #9089.
github-actions[bot] commented on PR #9089:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "cranelift:meta", "isle"Thus the following users have been cc'd because of the following labels:
- cfallin: isle
- fitzgen: isle
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
fitzgen updated PR #9089.
alexcrichton submitted PR review:
Very nice! Great to see this all shape up so well
alexcrichton submitted PR review:
Very nice! Great to see this all shape up so well
alexcrichton created PR review comment:
This is a pretty huge mouthful and it seems like it might be somewhat common, so perhaps a helper function in this module to perform the conversion? For example
to_pulley(r)
or something like that
alexcrichton created PR review comment:
This looks like a lot of copy/paste with a nontrivial amount of logic, would it be possible to refactor these?
alexcrichton created PR review comment:
I think I saw this as well in
abi.rs
as well aslower.isle
perhaps, coul the Rust-side parts be deduplicated into a helper?
fitzgen submitted PR review.
fitzgen created PR review comment:
The one in
abi.rs
is constructingMInst::Xconst*
variants while this one is calling differentenc::xconst*
functions, so there isn't a great way to dedupe this. We could make something that is abstract enough to handle both but that abstraction won't actually be any more concise, simpler, or easy to understand, unfortunately.
fitzgen updated PR #9089.
fitzgen updated PR #9089.
fitzgen has enabled auto merge for PR #9089.
fitzgen updated PR #9089.
fitzgen has enabled auto merge for PR #9089.
fitzgen updated PR #9089.
fitzgen has enabled auto merge for PR #9089.
fitzgen merged PR #9089.
Last updated: Dec 23 2024 at 12:05 UTC