Skip to content

winch: Initial integration with wasmtime #6119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

KevinRizzoTO
Copy link
Contributor

@KevinRizzoTO KevinRizzoTO commented Mar 29, 2023

Another big thank you to @alexcrichton for helping out with this and wiring up some of the initial pieces!

What this includes:

  • wiring up the wasmtime_environ::Compiler trait for Winch with wasmtime
  • including a new compilation strategy for winch to be used in the Config
  • the new compiler flag for the CLI, which will help us a lot in testing winch features
  • an initial implementation of the host-to-wasm trampoline for winch so it can be called in wasmtime

Things to note:

  • only supports x86_64, as winch is iterating on it should fill out other architectures
  • support for Typed functions isn't included, these don't use trampolines which winch requires at this time
  • no changes to support the Wasmtime* calling conventions
  • the new trampoline doesn't save all callee-saved registers prior to the function call, nor does it restore them prior to the epilogue

The intention is to work on the above iteratively, but this first pass will make it much easier to test winch. We've been manually running it's output in a test environment, but moving that over to wasmtime would greatly simplify the process.

KevinRizzoTO and others added 24 commits March 29, 2023 14:42
This is effectively an option to select the `Strategy` enumeration.
Hook this into the `TargetIsa::compile_function` hook as well. Currently
this doesn't take into account `Tunables`, but that's left as a TODO for
later.
Most of these are a WIP. It's missing trampolines for x64, but a basic
one exists for aarch64. It's missing the handling of arguments that
exist on the stack.

It currently imports `cranelift_wasm::WasmFuncType` since it's what's
passed to the `Compiler` trait. It's a bit awkward to use in the
`winch_codegen` crate since it mostly operates on `wasmparser` types.
I've had to hack in a conversion to get things working. Long term, I'm
not sure it's wise to rely on this type but it seems like it's easier on
the Cranelift side when creating the stub IR.
@KevinRizzoTO KevinRizzoTO requested review from a team as code owners March 29, 2023 19:11
@KevinRizzoTO KevinRizzoTO requested review from elliottt and removed request for a team March 29, 2023 19:11
@saulecabrera
Copy link
Member

The docs fix worked -- there was another failure regarding the order of crates in publish.rs: the relationship between wasmtime-winch and winch-environ was not correct. I've addressed that in 7d83eec

@saulecabrera saulecabrera enabled auto-merge April 4, 2023 18:55
@saulecabrera saulecabrera added this pull request to the merge queue Apr 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2023
@saulecabrera saulecabrera enabled auto-merge April 4, 2023 19:42
@saulecabrera saulecabrera force-pushed the wasmtime-winch-integration branch from 1242b79 to ef5f567 Compare April 4, 2023 20:01
@saulecabrera saulecabrera added this pull request to the merge queue Apr 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2023
@saulecabrera saulecabrera added this pull request to the merge queue Apr 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2023
@saulecabrera
Copy link
Member

saulecabrera commented Apr 5, 2023

Just one minor (hopefully final) change in f307452 to only run winch tests when in unix and x86_64, since there's no support for the fastcall calling convention yet.

@saulecabrera saulecabrera enabled auto-merge April 5, 2023 00:13
@saulecabrera saulecabrera added this pull request to the merge queue Apr 5, 2023
Merged via the queue into bytecodealliance:main with commit 3a92aa3 Apr 5, 2023
@KevinRizzoTO
Copy link
Contributor Author

@saulecabrera thank you for getting this over the line!

saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Apr 6, 2023
cfallin pushed a commit that referenced this pull request Apr 6, 2023
brendandburns pushed a commit to brendandburns/wasmtime that referenced this pull request Apr 13, 2023
saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Apr 27, 2023
This change introduces handling of traps and relocations in Winch, which
was left out in bytecodealliance#6119.

In order to so, this change moves the `CompiledFunction` struct to the
`wasmtime-cranelift-shared` crate, allowing Cranelift and Winch to
operate on a single, shared representation, following some of the ideas
discussed in bytecodealliance#5944.

Even though Winch doesn't rely on all the fields of `CompiledFunction`,
it eventually will. With the addition of relocations and traps it
started to be more evident that even if we wanted to have different
representations of a compiled function, they would end up being very
similar.

This change also consolidates where the `traps` and `relocations` of the
`CompiledFunction` get created, by introducing a constructor that
operates on a `MachBufferFinalized<Final>`, esentially encapsulating
this process in a single place for both Winch and Cranelift.
saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request Apr 27, 2023
This change introduces handling of traps and relocations in Winch, which
was left out in bytecodealliance#6119.

In order to so, this change moves the `CompiledFunction` struct to the
`wasmtime-cranelift-shared` crate, allowing Cranelift and Winch to
operate on a single, shared representation, following some of the ideas
discussed in bytecodealliance#5944.

Even though Winch doesn't rely on all the fields of `CompiledFunction`,
it eventually will. With the addition of relocations and traps it
started to be more evident that even if we wanted to have different
representations of a compiled function, they would end up being very
similar.

This change also consolidates where the `traps` and `relocations` of the
`CompiledFunction` get created, by introducing a constructor that
operates on a `MachBufferFinalized<Final>`, esentially encapsulating
this process in a single place for both Winch and Cranelift.
saulecabrera added a commit to saulecabrera/wasmtime that referenced this pull request May 1, 2023
This change introduces handling of traps and relocations in Winch, which
was left out in bytecodealliance#6119.

In order to so, this change moves the `CompiledFunction` struct to the
`wasmtime-cranelift-shared` crate, allowing Cranelift and Winch to
operate on a single, shared representation, following some of the ideas
discussed in bytecodealliance#5944.

Even though Winch doesn't rely on all the fields of `CompiledFunction`,
it eventually will. With the addition of relocations and traps it
started to be more evident that even if we wanted to have different
representations of a compiled function, they would end up being very
similar.

This change also consolidates where the `traps` and `relocations` of the
`CompiledFunction` get created, by introducing a constructor that
operates on a `MachBufferFinalized<Final>`, esentially encapsulating
this process in a single place for both Winch and Cranelift.
saulecabrera added a commit that referenced this pull request May 2, 2023
* winch: Handle relocations and traps

This change introduces handling of traps and relocations in Winch, which
was left out in #6119.

In order to so, this change moves the `CompiledFunction` struct to the
`wasmtime-cranelift-shared` crate, allowing Cranelift and Winch to
operate on a single, shared representation, following some of the ideas
discussed in #5944.

Even though Winch doesn't rely on all the fields of `CompiledFunction`,
it eventually will. With the addition of relocations and traps it
started to be more evident that even if we wanted to have different
representations of a compiled function, they would end up being very
similar.

This change also consolidates where the `traps` and `relocations` of the
`CompiledFunction` get created, by introducing a constructor that
operates on a `MachBufferFinalized<Final>`, esentially encapsulating
this process in a single place for both Winch and Cranelift.

* Rework the shared `CompiledFunction`

This commit reworks the shared `CompiledFunction` struct. The compiled
function now contains the essential pieces to derive all the information
to create the final object file and to derive the debug information for
the function.

This commit also decouples the dwarf emission process by introducing
a `metadata` field in the `CompiledFunction` struct, which is used as
the central structure for dwarf emission.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime winch Winch issues or pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants