Cranelift: make all non-tail, non-indirect calls patchable, and rename patchable ABI to `preserve_all`. (#12160)

* Cranelift: make all non-tail, non-indirect calls patchable, and rename patchable AB

Cranelift: make all non-tail, non-indirect calls patchable, and rename patchable ABI to `preserve_all`. (#12160)

* Cranelift: make all non-tail, non-indirect calls patchable, and rename patchable ABI to `preserve_all`.

As discussed in this week's Cranelift meeting, we've discovered a need
to generalize the `patchable_call` mechanism and corresponding
`patchable` ABI slightly. In particular, we will need patchable
`try_call` callsites as well in order to allow breakpoint handlers to
throw exceptions (desirable functionality eventually) and have this work
in the presence of inlining. Also, it's just a nice generalization to
say that patchability is an orthogonal dimension to the call ABI and the
other restrictions we initially imposed, and works as long as the basic
requirement (no return values) is met.

This also renames the `patchable` ABI to `preserve_all`, to make it
clear that its purpose is actually orthogonal, and it can be used
independently of patchable callsites. It also deletes the `cold` ABI,
which never actually did anything and is misleading in the presence of
an actual cold-ish (subzero temperature, actually) ABI like
`preserve_all`.

* Review feedback.

show more ...

Update Wasmtime to the 2024 Rust Edition (#10806)

* Update Wasmtime to the 2024 Rust Edition

Now that our MSRV supports the 2024 edition it's possible to make this
switch. This commit moves Wasmtim

Update Wasmtime to the 2024 Rust Edition (#10806)

* Update Wasmtime to the 2024 Rust Edition

Now that our MSRV supports the 2024 edition it's possible to make this
switch. This commit moves Wasmtime to the 2024 Edition to keep
up-to-date with Rust idioms and access many of the edition features
exclusive to the 2024 edition.

prtest:full

* Reformat with the 2024 edition

show more ...

Enforce `uninlined_format_args` for the workspace (#9065)

* Enforce `uninlined_format_args` for the workspace

* fix: failing `Monolith Checks` job

* fix: formatting

Add more documentation to `ArgumentPurpose::StructArgument` (#8906)

I was a bit unclear on what exactly it was for. I thought the u32 field
could also use some documentation to clarify its meaning.

Remove ArgumentPurpose::StackLimit (#8700)

Decouple `serde` from its `derive` crate (#6917)

By not activating the `derive` feature on `serde`, the compilation speed
can be improved by a lot. This is because `serde` can then compile in
parall

Decouple `serde` from its `derive` crate (#6917)

By not activating the `derive` feature on `serde`, the compilation speed
can be improved by a lot. This is because `serde` can then compile in
parallel to `serde_derive`, allowing it to finish compilation possibly
even before `serde_derive`, unblocking all the crates waiting for
`serde` to start compiling much sooner.

As it turns out the main deciding factor for how long the compile time of a
project is, is primarly determined by the depth of dependencies rather
than the width. In other words, a crate's compile times aren't affected
by how many crates it depends on, but rather by the longest chain of
dependencies that it needs to wait on. In many cases `serde` is part of
that long chain, as it is part of a long chain if the `derive` feature
is active:

`proc-macro2` compile build script > `proc-macro2` run build script >
`proc-macro2` > `quote` > `syn` > `serde_derive` > `serde` >
`serde_json` (or any crate that depends on serde)

By decoupling it from `serde_derive`, the chain is shortened and compile
times get much better.

Check this issue for a deeper elaboration:
https://github.com/serde-rs/serde/issues/2584

For `wasmtime` I'm seeing a reduction from 24.75s to 22.45s when
compiling in `release` mode. This is because wasmtime through `gimli`
has a dependency on `indexmap` which can only start compiling when
`serde` is finished, which you want to happen as early as possible so
some of wasmtime's dependencies can start compiling.

To measure the full effect, the dependencies can't by themselves
activate the `derive` feature. I've upstreamed a patch for
`fxprof-processed-profile` which was the only dependency that activated
it for `wasmtime` (not yet published to crates.io). `wasmtime-cli` and
co. may need patches for their dependencies to see a similar
improvement.

show more ...

A bunch of minor cleanups (#6767)

* Remove DisplayFunctionAnnotations

It used to exist for printing the debuginfo value ranges with the clif
ir, but this no longer happens, so it is now useless.

*

A bunch of minor cleanups (#6767)

* Remove DisplayFunctionAnnotations

It used to exist for printing the debuginfo value ranges with the clif
ir, but this no longer happens, so it is now useless.

* Remove debug info collection from DummyEnvironment

There are no remaining users of it

* Remove ComparableSourceLoc

It is unused

* Move LabelValueLoc re-export out of the ir module

It encodes target specific information, so shouldn't be in the target
independent ir module.

* Remove RelocDistance dependency from ir::extfunc and ir::globalvalue

show more ...

Remove the unused sigid argument purpose (#5753)

Fix warnings in `cranelift-codegen` docs builds (#5292)

cranelift: Remove booleans (#5031)

Remove the boolean types from cranelift, and the associated instructions breduce, bextend, bconst, and bint. Standardize on using 1/0 for the return value from ins

cranelift: Remove booleans (#5031)

Remove the boolean types from cranelift, and the associated instructions breduce, bextend, bconst, and bint. Standardize on using 1/0 for the return value from instructions that produce scalar boolean results, and -1/0 for boolean vector elements.

Fixes #3205

Co-authored-by: Afonso Bordado <[email protected]>
Co-authored-by: Ulrich Weigand <[email protected]>
Co-authored-by: Chris Fallin <[email protected]>

show more ...

Implement an incremental compilation cache for Cranelift (#4551)

This is the implementation of https://github.com/bytecodealliance/wasmtime/issues/4155, using the "inverted API" approach suggested b

Implement an incremental compilation cache for Cranelift (#4551)

This is the implementation of https://github.com/bytecodealliance/wasmtime/issues/4155, using the "inverted API" approach suggested by @cfallin (thanks!) in Cranelift, and trait object to provide a backend for an all-included experience in Wasmtime.

After the suggestion of Chris, `Function` has been split into mostly two parts:

- on the one hand, `FunctionStencil` contains all the fields required during compilation, and that act as a compilation cache key: if two function stencils are the same, then the result of their compilation (`CompiledCodeBase<Stencil>`) will be the same. This makes caching trivial, as the only thing to cache is the `FunctionStencil`.
- on the other hand, `FunctionParameters` contain the... function parameters that are required to finalize the result of compilation into a `CompiledCode` (aka `CompiledCodeBase<Final>`) with proper final relocations etc., by applying fixups and so on.

Most changes are here to accomodate those requirements, in particular that `FunctionStencil` should be `Hash`able to be used as a key in the cache:

- most source locations are now relative to a base source location in the function, and as such they're encoded as `RelSourceLoc` in the `FunctionStencil`. This required changes so that there's no need to explicitly mark a `SourceLoc` as the base source location, it's automatically detected instead the first time a non-default `SourceLoc` is set.
- user-defined external names in the `FunctionStencil` (aka before this patch `ExternalName::User { namespace, index }`) are now references into an external table of `UserExternalNameRef -> UserExternalName`, present in the `FunctionParameters`, and must be explicitly declared using `Function::declare_imported_user_function`.
- some refactorings have been made for function names:
- `ExternalName` was used as the type for a `Function`'s name; while it thus allowed `ExternalName::Libcall` in this place, this would have been quite confusing to use it there. Instead, a new enum `UserFuncName` is introduced for this name, that's either a user-defined function name (the above `UserExternalName`) or a test case name.
- The future of `ExternalName` is likely to become a full reference into the `FunctionParameters`'s mapping, instead of being "either a handle for user-defined external names, or the thing itself for other variants". I'm running out of time to do this, and this is not trivial as it implies touching ISLE which I'm less familiar with.

The cache computes a sha256 hash of the `FunctionStencil`, and uses this as the cache key. No equality check (using `PartialEq`) is performed in addition to the hash being the same, as we hope that this is sufficient data to avoid collisions.

A basic fuzz target has been introduced that tries to do the bare minimum:

- check that a function successfully compiled and cached will be also successfully reloaded from the cache, and returns the exact same function.
- check that a trivial modification in the external mapping of `UserExternalNameRef -> UserExternalName` hits the cache, and that other modifications don't hit the cache.
- This last check is less efficient and less likely to happen, so probably should be rethought a bit.

Thanks to both @alexcrichton and @cfallin for your very useful feedback on Zulip.

Some numbers show that for a large wasm module we're using internally, this is a 20% compile-time speedup, because so many `FunctionStencil`s are the same, even within a single module. For a group of modules that have a lot of code in common, we get hit rates up to 70% when they're used together. When a single function changes in a wasm module, every other function is reloaded; that's still slower than I expect (between 10% and 50% of the overall compile time), so there's likely room for improvement.

Fixes #4155.

show more ...

Remove some dead code from the abi code (#4653)

These were originally used by the old backend framework as part of
legalizing function signatures for the respective ABI.

Cranellift: remove Baldrdash support and related features. (#4571)

* Cranellift: remove Baldrdash support and related features.

As noted in Mozilla's bugzilla bug 1781425 [1], the SpiderMonkey te

Cranellift: remove Baldrdash support and related features. (#4571)

* Cranellift: remove Baldrdash support and related features.

As noted in Mozilla's bugzilla bug 1781425 [1], the SpiderMonkey team
has recently determined that their current form of integration with
Cranelift is too hard to maintain, and they have chosen to remove it
from their codebase. If and when they decide to build updated support
for Cranelift, they will adopt different approaches to several details
of the integration.

In the meantime, after discussion with the SpiderMonkey folks, they
agree that it makes sense to remove the bits of Cranelift that exist
to support the integration ("Baldrdash"), as they will not need
them. Many of these bits are difficult-to-maintain special cases that
are not actually tested in Cranelift proper: for example, the
Baldrdash integration required Cranelift to emit function bodies
without prologues/epilogues, and instead communicate very precise
information about the expected frame size and layout, then stitched
together something post-facto. This was brittle and caused a lot of
incidental complexity ("fallthrough returns", the resulting special
logic in block-ordering); this is just one example. As another
example, one particular Baldrdash ABI variant processed stack args in
reverse order, so our ABI code had to support both traversal
orders. We had a number of other Baldrdash-specific settings as well
that did various special things.

This PR removes Baldrdash ABI support, the `fallthrough_return`
instruction, and pulls some threads to remove now-unused bits as a
result of those two, with the understanding that the SpiderMonkey folks
will build new functionality as needed in the future and we can perhaps
find cleaner abstractions to make it all work.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1781425

* Review feedback.

* Fix (?) DWARF debug tests: add `--disable-cache` to wasmtime invocations.

The debugger tests invoke `wasmtime` from within each test case under
the control of a debugger (gdb or lldb). Some of these tests started to
inexplicably fail in CI with unrelated changes, and the failures were
only inconsistently reproducible locally. It seems to be cache related:
if we disable cached compilation on the nested `wasmtime` invocations,
the tests consistently pass.

* Review feedback.

show more ...

Remove more old backend ISA concepts (#3402)

This also paves the way for unifying TargetIsa and MachBackend, since now they map one to one. In theory the two traits could be merged, which would be n

Remove more old backend ISA concepts (#3402)

This also paves the way for unifying TargetIsa and MachBackend, since now they map one to one. In theory the two traits could be merged, which would be nice to limit the number of total concepts. Also they have quite different responsibilities, so it might be fine to keep them separate.

Interestingly, this PR started as removing RegInfo from the TargetIsa trait since the adapter returned a dummy value there. From the fallout, noticed that all Display implementations didn't needed an ISA anymore (since these were only used to render ISA specific registers). Also the whole family of RegInfo / ValueLoc / RegUnit was exclusively used for the old backend, and these could be removed. Notably, some IR instructions needed to be removed, because they were using RegUnit too: this was the oddball of regfill / regmove / regspill / copy_special, which were IR instructions inserted by the old regalloc. Fare thee well!

show more ...

Add serde serialization support for the full clif ir

Do value-extensions at ABI boundaries only when ABI requires it.

There has been some confusion over the meaning of the "sign-extend"
(`sext`) and "zero-extend" (`uext`) attributes on parameters and

Do value-extensions at ABI boundaries only when ABI requires it.

There has been some confusion over the meaning of the "sign-extend"
(`sext`) and "zero-extend" (`uext`) attributes on parameters and return
values in signatures. According to the three implemented backends, these
attributes indicate that a value narrower than a full register should
always be extended in the way specified. However, they are much more
useful if they mean "extend in this way if the ABI requires extending":
only the ABI backend knows whether or not a particular ABI (e.g., x64
SysV vs. x64 Baldrdash) requires extensions, while only the frontend
(CLIF generator) knows whether or not a value is signed, so the two have
to work in concert.

This is the result of some very helpful discussion in #2354 (thanks to
@uweigand for raising the issue and @bjorn3 for helping to reason about
it).

This change respects the extension attributes in the above way, rather
than unconditionally extending, to avoid potential performance
degradation as we introduce more extension attributes on signatures.

show more ...

Support for SpiderMonkey's "Wasm ABI 2020".

As part of a Wasm JIT update, SpiderMonkey is changing its internal
WebAssembly function ABI. The new ABI's frame format includes "caller
TLS" and "callee

Support for SpiderMonkey's "Wasm ABI 2020".

As part of a Wasm JIT update, SpiderMonkey is changing its internal
WebAssembly function ABI. The new ABI's frame format includes "caller
TLS" and "callee TLS" slots. The details of where these come from are
not important; from Cranelift's point of view, the only relevant
requirement is that we have two on-stack args that are always present
(offsetting other on-stack args), and that we define special argument
purposes so that we can supply values for these slots.

Note that this adds a *new* ABI (a variant of the Baldrdash ABI) because
we do not want to tightly couple the landing of this PR to the landing
of the changes in SpiderMonkey; it's better if both the old and new
behavior remain available in Cranelift, so SpiderMonkey can continue to
vendor Cranelift even if it does not land (or backs out) the ABI change.

Furthermore, note that this needs to be a Cranelift-level change (i.e.
cannot be done purely from the translator environment implementation)
because the special TLS arguments must always go on the stack, which
would not otherwise happen with the usual argument-placement logic; and
there is no primitive to push a value directly in CLIF code (the notion
of a stack frame is a lower-level concept).

show more ...

Fix review comments

Implement SystemV struct argument passing

Make wasmtime_environ::Module serializable (#2005)

* Define WasmType/WasmFuncType in the Cranelift
* Make `Module` serializable

Fix doc warnings and link failures (#1948)

Also add configuration to CI to fail doc generation if any links are
broken. Unfortunately we can't blanket deny all warnings in rustdoc
since some are u

Fix doc warnings and link failures (#1948)

Also add configuration to CI to fail doc generation if any links are
broken. Unfortunately we can't blanket deny all warnings in rustdoc
since some are unconditional warnings, but for now this is hopefully
good enough.

Closes #1947

show more ...

Implement passing arguments by ref for win64 ABI

Fix long-range (non-colocated) aarch64 calls to not use Arm64Call reloc, and fix simplejit to use it.

Previously, every call was lowered on AArch64 to a `call` instruction, which
takes a signed 26-b

Fix long-range (non-colocated) aarch64 calls to not use Arm64Call reloc, and fix simplejit to use it.

Previously, every call was lowered on AArch64 to a `call` instruction, which
takes a signed 26-bit PC-relative offset. Including the 2-bit left shift, this
gives a range of +/- 128 MB. Longer-distance offsets would cause an impossible
relocation record to be emitted (or rather, a record that a more sophisticated
linker would fix up by inserting a shim/veneer).

This commit adds a notion of "relocation distance" in the MachInst backends,
and provides this information for every call target and symbol reference. The
intent is that backends on architectures like AArch64, where there are different
offset sizes / addressing strategies to choose from, can either emit a regular
call or a load-64-bit-constant / call-indirect sequence, as necessary. This
avoids the need to implement complex linking behavior.

The MachInst driver code provides this information based on the "colocated" bit
in the CLIF symbol references, which appears to have been designed for this
purpose, or at least a similar one. Combined with the `use_colocated_libcalls`
setting, this allows client code to ensure that library calls can link to
library code at any location in the address space.

Separately, the `simplejit` example did not handle `Arm64Call`; rather than doing
so, it appears all that is necessary to get its tests to pass is to set the
`use_colocated_libcalls` flag to false, to make use of the above change. This
fixes the `libcall_function` unit-test in this crate.

show more ...

Use `is_wasm_parameter` in translating wasm calls (#1352)

* Use `is_wasm_parameter` in translating wasm calls

Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters,

Use `is_wasm_parameter` in translating wasm calls (#1352)

* Use `is_wasm_parameter` in translating wasm calls

Added in #1329 it's now possible for multiple parameters to be non-wasm
parameters, so the previous `param_types` method is no longer suitable
for acquiring all wasm-related parameters, rather then `FuncEnvironment`
must be consulted. This removes usage of `param_types()` as a method
from the wasm translation and instead adds a custom method inline for
filtering the parameters based on `is_wasm_parameter`.

* Apply feedback

* Run rustfmt

* Don't require `mut`

* Run rustfmt

show more ...

Correctly count the number of wasm parameters. (#1337)

* Correctly count the number of wasm parameters.

Following up on #1329, this further replaces `num_normal_params` with a function
which cal

Correctly count the number of wasm parameters. (#1337)

* Correctly count the number of wasm parameters.

Following up on #1329, this further replaces `num_normal_params` with a function
which calls `is_wasm_parameter` to correctly count the number of wasm
parameters a function has.

* Move is_wasm_parameter's implementation into the trait.

show more ...