Cranelift: robustify timing infrastructure against mis-use and/or system clock bugs. (#12709)

In #12692, it was observed that the computation of time spent in
nested timing spans, minus child time,

Cranelift: robustify timing infrastructure against mis-use and/or system clock bugs. (#12709)

In #12692, it was observed that the computation of time spent in
nested timing spans, minus child time, was underflowing.

Correct operation of the handling of nested spans depends on the
invariant that the accumulated time for child spans is less than or
equal to a parent span once timing is completed. This property should
hold as long as the system clock is monotonic, and as long as timing
tokens are dropped in-order, so that the elapsed time of a parent
truly is computed after the elapsed time of a child ends.

The timing state may also temporarily violate this invariant whenever
a token is pending (still on stack and timing): the child time of any
completed child spans will be counted, but the parent has not yet
been. Hence, taking a snapshot of the state and computing "parent
minus children" on that snapshot may observe cases that yield
underflow.

This PR makes the infrastructure more robust along a few different
dimensions:

- It hardens the clock source we use to have a locally-ensured
guarantee of monotonicity, since we rely on this for logical
correctness. In particular, for each thread (since timing spans never
move between threads), we track the last `Instant` that was used by
the timing infrastructure, and use that value (zero time passed) if
the system clock moves backward.

- It hardens the assert about pass-timing token drop order from a
`debug_assert` to an `assert`. If this invariant is being violated,
we want to know about it noisily, rather than observing a
subtraction underflow or other inconsistency later.

- It adds an assert in `take_current()` to ensure that a snapshot is
never taken when any pass timing is in progress.

This should address any theoretically possible sources of #12692, as
far as I can tell.

show more ...

cranelift-codegen: rename most uses of std to core and alloc (#12237)

* rename most std uses to core and alloc

* cargo fmt

Update the `log` dependency (#11197)

* Update the `log` dependency

This enables getting warnings about formatting strings in the `log`
crate directives which are then additionally fixed here as wel

Update the `log` dependency (#11197)

* Update the `log` dependency

This enables getting warnings about formatting strings in the `log`
crate directives which are then additionally fixed here as well.

* Update dependency directive in `Cargo.toml`

show more ...

Remove `expect(clippy::allow_attributes_without_reason)` from cranelift-codegen (#11182)

* Remove `expect(clippy::allow_attributes_without_reason)` from cranelift-codegen

This commit gets around to

Remove `expect(clippy::allow_attributes_without_reason)` from cranelift-codegen (#11182)

* Remove `expect(clippy::allow_attributes_without_reason)` from cranelift-codegen

This commit gets around to migrating the `cranelift-codegen` crate to
require a reason on lint directives and additionally switch to
`#[expect]` where possible.

prtest:full

* Move x64-only item to x64 backend

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 ...

fix: `no_std` timing module (#9976)

* fix(cranelift): `no_std` timing module

* Update timing.rs

Enforce `uninlined_format_args` for the workspace (#9065)

* Enforce `uninlined_format_args` for the workspace

* fix: failing `Monolith Checks` job

* fix: formatting

cranelift: Delete redundant DCE optimization pass (#8227)

The egraph pass and the dead-code elimination pass both remove
instructions whose results are unused. If the optimization level is
"none", n

cranelift: Delete redundant DCE optimization pass (#8227)

The egraph pass and the dead-code elimination pass both remove
instructions whose results are unused. If the optimization level is
"none", neither pass runs, and if it's anything else both passes run. I
don't think we should do this work twice.

Note that the DCE pass is different than the "eliminate unreachable
code" pass, which removes entire blocks that are unreachable from the
entry block. That pass might still be necessary.

show more ...

Consolidate platform-specific definitions in Wasmtime (#7626)

* Gate more functionality behind `debug-builtins`

This commit extends the gating behavior of the preexisting
`debug-builtins` Cargo fea

Consolidate platform-specific definitions in Wasmtime (#7626)

* Gate more functionality behind `debug-builtins`

This commit extends the gating behavior of the preexisting
`debug-builtins` Cargo feature to cover more GDB-related functionality
associated with debugging. This can additionally slim down the set of
exposed symbols from Wasmtime over the default with them included.

* Move timing in Cranelift behind a Cargo feature

This commit adds a `timing` feature to the `cranelift-codegen` crate
which is enabled by default. This feature gates the timing functionality
in Cranelift to enable turning it off if desired. The goal of this
commit is to remove a system dependency on `Instant` for possibly
esoteric environments.

* Consolidate platform-specific definitions in Wasmtime

Prior to this commit Wasmtime did not not have a style or system for
containing platform-specific logic in files. The goal of this commit is
to consolidate all platform-specific functionality into one location to
make it easier to port Wasmtime to new systems. This commit creates a
`sys` module within the `wasmtime-runtime` crate which conditionally
defines all of the platform support that Wasmtime requires, namely
things related to virtual memory management and trap handling. Many of
the previous `unix.rs` files interspersed throughout the tree are now
all located in a single `unix` directory. This additionally helps split
out `miri`-specific functionality by pretending `miri` is its own
platform.

This change additionally goes through `#[cfg]` directives throughout
`wasmtime-runtime`, `wasmtime-jit`, and `wasmtime` itself to place all
of this target-specific functionality within this `sys` directory. The
end state is that there are two new top-level modules in the
`wasmtime-runtime` crate:

* `arch` - this conditionally defines architecture-specific logic such
as the state used by backtraces, libcalls, etc.
* `sys` - this conditionally defines OS or platform-specific logic such
as virtual memory management.

One secondary goal of this commit is to enable the ability to easily
add new platforms to Wasmtime, even if it's in a fork of Wasmtime.
Previously patches might have to touch a good number of locations where
now they'd ideally only have to touch `sys/mod.rs` to declare a new
platform and `sys/$platform/*.rs` to define all the functionality.

* Fix build on Windows

prtest:full

* Fix some build warnings

* Fix miri build

* Include debug-builtins when testing

* Try to fix Windows tests

* Fix warnings on miri

* Fix miri build

* Fix debug-builtins feature

* More feature fixes

* Rename `fd` field

* Fix speeling

* Review comments

show more ...

Cranelift: Log duration of timing passes (#6654)

Might as well, have it right there. Weird seeing all these start/end logs
without the duration itself, otherwise.

Support plugging external profilers into the Cranelift timing infrastructure (#5749)

* Remove no-std code for cranelift_codegen::timings

no-std mode isn't supported by Cranelift anymore

* Simplify

Support plugging external profilers into the Cranelift timing infrastructure (#5749)

* Remove no-std code for cranelift_codegen::timings

no-std mode isn't supported by Cranelift anymore

* Simplify define_passes macro

* Add egraph opt timings

* Replace the add_to_current api with PassTimes::add

* Omit a couple of unused time measurements

* Reduce divergence between run and run_passes a bit

* Introduce a Profiler trait

This allows plugging in external profilers into the Cranelift profiling
framework.

* Add Pass::description method

* Remove duplicate usage of the compile pass timing

* Rustfmt

show more ...

Remove IFLAGS/FFLAGS types (#5406)

All instructions using the CPU flags types (IFLAGS/FFLAGS) were already
removed. This patch completes the cleanup by removing all remaining
instructions that de

Remove IFLAGS/FFLAGS types (#5406)

All instructions using the CPU flags types (IFLAGS/FFLAGS) were already
removed. This patch completes the cleanup by removing all remaining
instructions that define values of CPU flags types, as well as the
types themselves.

Specifically, the following features are removed:
- The IFLAGS and FFLAGS types and the SpecialType category.
- Special handling of IFLAGS and FFLAGS in machinst/isle.rs and
machinst/lower.rs.
- The ifcmp, ifcmp_imm, ffcmp, iadd_ifcin, iadd_ifcout, iadd_ifcarry,
isub_ifbin, isub_ifbout, and isub_ifborrow instructions.
- The writes_cpu_flags instruction property.
- The flags verifier pass.
- Flags handling in the interpreter.

All of these features are currently unused; no functional change
intended by this patch.

This addresses https://github.com/bytecodealliance/wasmtime/issues/3249.

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 ...

[cranelift] Rejigger the `compile` API (#4540)

* Move `emit_to_memory` to `MachCompileResult`

This small refactoring makes it clearer to me that emitting to memory
doesn't require anything else

[cranelift] Rejigger the `compile` API (#4540)

* Move `emit_to_memory` to `MachCompileResult`

This small refactoring makes it clearer to me that emitting to memory
doesn't require anything else from the compilation `Context`. While it's
a trivial change, it's a small public API change that shouldn't cause
too much trouble, and doesn't seem RFC-worthy. Happy to hear different
opinions about this, though!

* hide the MachCompileResult behind a method

* Add a `CompileError` wrapper type that references a `Function`

* Rename MachCompileResult to CompiledCode

* Additionally remove the last unsafe API in cranelift-codegen

show more ...

Partially rewrite the constant-phi-nodes pass to make it more idiomatic (#4111)

* Add [must_use] to the timer functions

And remove unused vcode_post_ra function

* Make remove-constant-phis cod

Partially rewrite the constant-phi-nodes pass to make it more idiomatic (#4111)

* Add [must_use] to the timer functions

And remove unused vcode_post_ra function

* Make remove-constant-phis code more idiomatic

show more ...

Add support for running the regalloc2 checker. (#4043)

With these fixes, all this PR has to do is instantiate and run the
checker on the `regalloc2::Output`. This is off by default, and is
enabled

Add support for running the regalloc2 checker. (#4043)

With these fixes, all this PR has to do is instantiate and run the
checker on the `regalloc2::Output`. This is off by default, and is
enabled by setting the `regalloc_checker` Cranelift option.

This restores the old functionality provided by e.g. the
`backtracking_checked` regalloc algorithm setting rather than
`backtracking` when we were still on regalloc.rs.

show more ...

Use const-initialized thread locals (#3923)

This was a relatively recent feature added to the Rust standard library
which should help accelerate calls into WebAssembly slightly.

Add some debug logging for timing in module compiles (#3417)

* Add some debug logging for timing in module compiles

This is sometimes helpful when debugging slow compiles from fuzz bugs or
simil

Add some debug logging for timing in module compiles (#3417)

* Add some debug logging for timing in module compiles

This is sometimes helpful when debugging slow compiles from fuzz bugs or
similar.

* Fix total duration calculation to not double-count

show more ...

Remove no longer existing passes from timing.rs

Remove the CSSA verifier

The old register allocator required CSSA as intermediate step. The new
register allocator doesn't use SSA at all.

Remove references to the old `postopt` pass

cranelift: Move most debug-level logs to the trace level

Cranelift crates have historically been much more verbose with debug-level
logging than most other crates in the Rust ecosystem. We log thing

cranelift: Move most debug-level logs to the trace level

Cranelift crates have historically been much more verbose with debug-level
logging than most other crates in the Rust ecosystem. We log things like how
many parameters a basic block has, the color of virtual registers during
regalloc, etc. Even for Cranelift hackers, these things are largely only useful
when hacking specifically on Cranelift and looking at a particular test case,
not even when using some Cranelift embedding (such as Wasmtime).

Most of the time, when people want logging for their Rust programs, they do
something like:

RUST_LOG=debug cargo run

This means that they get all that mostly not useful debug logging out of
Cranelift. So they might want to disable logging for Cranelift, or change it to
a higher log level:

RUST_LOG=debug,cranelift=info cargo run

The problem is that this is already more annoying to type that `RUST_LOG=debug`,
and that Cranelift isn't one single crate, so you actually have to play
whack-a-mole with naming all the Cranelift crates off the top of your head,
something more like this:

RUST_LOG=debug,cranelift=info,cranelift_codegen=info,cranelift_wasm=info,...

Therefore, we're changing most of the `debug!` logs into `trace!` logs: anything
that is very Cranelift-internal, unlikely to be useful/meaningful to the
"average" Cranelift embedder, or prints a message for each instruction visited
during a pass. On the other hand, things that just report a one line statistic
for a whole pass, for example, are left as `debug!`. The more verbose the log
messages are, the higher the bar they must clear to be `debug!` rather than
`trace!`.

show more ...

Add timing for several new-backend stages.

This PR adds a bit more granularity to the output of e.g. `clif-util
compile -T`, indicating how much time is spent in VCode lowering and
various other new

Add timing for several new-backend stages.

This PR adds a bit more granularity to the output of e.g. `clif-util
compile -T`, indicating how much time is spent in VCode lowering and
various other new-backend-specific tasks.

show more ...

Add a transformation pass which removes phi nodes to which it can demonstrate
that only one value ever flows. Has been observed to improve generated code
run times by up to 8%. Compilation cost inc

Add a transformation pass which removes phi nodes to which it can demonstrate
that only one value ever flows. Has been observed to improve generated code
run times by up to 8%. Compilation cost increases by about 0.6%, but up to 7%
total cost has been observed to be saved; iow it can be a significant win in
terms of compilation time, overall.

show more ...

Fix clippy warnings.

This commit fixes the current set of (stable) clippy warnings in the repo.