- Handle call instructions' clobbers with the clobbers API, using RA2's
clobbers bitmask (bytecodealliance/regalloc2#58) rather than clobbers
list;
- Pull in changes from bytecodealliance/regalloc2#59 for much more sane
edge-case behavior w.r.t. liverange splitting.
The previous `cls` code was producing wrong results when fed with a -1 i8.
The fix here is to sign extend instead of zero extending since we want
to keep the sign bit as one in order for it to be counted correctly
in the cls instruction
This also merges the interpreter only tests now that aarch64
correctly supports this instruction
* Upgrade to regalloc2 v0.2.3 to get bugfix from bytecodealliance/regalloc2#60.
* Update RELEASES.md.
* Update two compile tests based on slightly shifting regalloc output.
`idiv` on x86-64 only reads `rdx`/`edx`/`dx`/`dl` for divides with width
greater than 8 bits; for an 8-bit divide, it reads the whole 16-bit
divisor from `ax`, as our CISC ancestors intended. This PR fixes the
metadata to avoid a regalloc panic (due to undefined `rdx`) in this
case. Does not affect Wasmtime or other Wasm-frontend embedders.
This commit fixes a mistake in the `Swizzle` opcode implementation in
the x64 backend of Cranelift. Previously an input register was casted to
a writable register and then modified, which I believe instructions are
not supposed to do. This was discovered as part of my investigation
into #4315.
This commit fixes a bug in the previous codegen for the `select`
instruction when the operations of the `select` were of the `v128` type.
Previously teh `XmmCmove` instruction only stored an `OperandSize` of 32
or 64 for a 64 or 32-bit move, but this was also used for these 128-bit
types which meant that when used the wrong move instruction was
generated. The fix applied here is to store the whole `Type` being moved
so the 128-bit variant can be selected as well.
* cranelift: Fix `bint` implementation on interpreter
The interpreter was returning -1 instead of 1 for positive values.
This also extends the bint test suite to cover all types.
* cranelift: Restrict `bint` to scalar values only
This fixes a bug when the `cold` field would not be serialized, since
we're using a custom (de)serializer for `Layout`. This is now properly
handled by adding a boolean in the serialized stream.
This was caught during the work on #4155, as this would result in cache
mismatches between a function and itself.
This commit updates the wasm-tools family of crates, notably pulling in
the refactorings and updates from bytecodealliance/wasm-tools#621 for
the latest iteration of the component model. This commit additionally
updates all support for the component model for these changes, notably:
* Many bits and pieces of type information was refactored. Many
`FooTypeIndex` namings are now `TypeFooIndex`. Additionally there is
now `TypeIndex` as well as `ComponentTypeIndex` for the two type index
spaces in a component.
* A number of new sections are now processed to handle the core and
component variants.
* Internal maps were split such as the `funcs` map into
`component_funcs` and `funcs` (same for `instances`).
* Canonical options are now processed individually instead of one bulk
`into` definition.
Overall this was not a major update to the internals of handling the
component model in Wasmtime. Instead this was mostly a surface-level
refactoring to make sure that everything lines up with the new binary
format for components.
* All text syntax used in tests was updated to the new syntax.
Now the fiber implementation on AArch64 authenticates function
return addresses and includes the relevant BTI instructions, except
on macOS.
Also, change the locations of the saved FP and LR registers on the
fiber stack to make them compliant with the Procedure Call Standard
for the Arm 64-bit Architecture.
Copyright (c) 2022, Arm Limited.
In #4224 we saw that an SSE2-only x86-64 system somehow was still
detecting SSE3/SSSE3/SSE4.1/SSE4.2. It turns out that we enabled these
in the baseline `Flags` in #3816, because without that, a ton of other
things break: default flags no longer produce a compiler backend that
works with default Wasmtime settings. However the logic to set them
when detected (via `CPUID`-using feature-test macros) only does an "if
detected then set bit" step per feature; the bits are never *cleared*.
This PR fixes that.
The current lowering helper for `cmpxchg` returns the literal RealReg
`rax` as its result. However, this breaks a number of invariants, and
eventually causes a regalloc panic if used as a blockparam arg (pinned
vregs cannot be used in this way).
In general we have to return regular vregs, not a RealReg, as results of
instructions during lowering. However #4223 added a helper for
`x64_cmpxchg` that returns a literal `rax`.
Fortunately we can do the right thing here by just giving a fresh vreg
to the instruction; the regalloc constraints mean that this vreg is
constrained to `rax` at the instruction (at its def/late point), so the
generator of the instruction need not worry about `rax` here.
If an address expression is given to `to_amode` that is completely
constant (no registers at all), then it will produce an `Amode` that has
the resulting constant as an offset, and `(invalid_reg)` as the base.
This is a side-effect of the way we build up the amode step-by-step --
we're waiting to see a register and plug it into the base field. If we
never get a reg though, we need to generate a constant zero into a
register and use that as the base. This PR adds a `finalize_amode`
helper to do just that.
Fixes#4234.
* Update Cranelift-ISLE integration docs to reflect no more checked-in code.
In #4143, we removed the checked-in-generated-code aspect of the ISLE
build process, in order to simplify the development cycle and reduce
errors. However, I failed to update the docs at the same time. This PR
fixes that. Supersedes #4228 (thanks @jlb6740 for noticing this issue!).
* fix typo
Our README was starting to show its age; it did not reflect the current
status of Cranelift well with respect to production maturity, current
supported backends, or performance. This PR makes a pass over the
"Status" section to fix that. It also removes some old/out-of-date
details, like `no_std` support (which has bitrotted).
Rust 1.61 changed the way `Debug` output looks for strings with null
bytes in them, which broke some expected-panic error message matches.
This makes the expectations more generic while still capturing the
important part ("has a null byte").
This resolves an edge-case where mul.i128 with an input that continues
to be live after the instruction could cause an invalid regalloc
constraint (basically, the regalloc did not previously support an
instruction use and def both being constrained to the same physical reg;
and the "mul" variant used for mul.i128 on x64 was the only instance of
such operands in Cranelift).
Causes two extra move instructions in the mul.i128 filetest, but that's
the price to pay for the slightly more general (works in all cases)
handling of the constraints.
RA2 recently removed the need for a dedicated scratch register for
cyclic moves (bytecodealliance/regalloc2#51). This has moderate positive
performance impact on function bodies that were register-constrained, as
it means that one more register is available. In Sightglass, I measured
+5-8% on `blake3-scalar`, at least among current benchmarks.
* Remove unused srcloc in MachReloc
* Remove unused srcloc in MachTrap
* Use `into_iter` on array in bench code to suppress a warning
* Remove unused srcloc in MachCallSite
Previously, the pinned register (enabled by the `enable_pinned_reg`
Cranelift setting and used via the `get_pinned_reg` and `set_pinned_reg`
CLIF ops) was only used when Cranelift was embedded in SpiderMonkey, in
order to support a pinned heap register. SpiderMonkey has its own
calling convention in Cranelift (named after the integration layer,
"Baldrdash").
However, the feature is more general, and should be usable with the
default system calling convention too, e.g. SysV or Windows Fastcall.
This PR fixes the ABI code to properly treat the pinned register as a
globally allocated register -- and hence an implicit input and output to
every function, not saved/restored in the prologue/epilogue -- for SysV
on x86-64 and aarch64, and Fastcall on x86-64.
Fixes#4170.
This PR adds a basic *alias analysis*, and optimizations that use it.
This is a "mid-end optimization": it operates on CLIF, the
machine-independent IR, before lowering occurs.
The alias analysis (or maybe more properly, a sort of memory-value
analysis) determines when it can prove a particular memory
location is equal to a given SSA value, and when it can, it replaces any
loads of that location.
This subsumes two common optimizations:
* Redundant load elimination: when the same memory address is loaded two
times, and it can be proven that no intervening operations will write
to that memory, then the second load is *redundant* and its result
must be the same as the first. We can use the first load's result and
remove the second load.
* Store-to-load forwarding: when a load can be proven to access exactly
the memory written by a preceding store, we can replace the load's
result with the store's data operand, and remove the load.
Both of these optimizations rely on a "last store" analysis that is a
sort of coloring mechanism, split across disjoint categories of abstract
state. The basic idea is that every memory-accessing operation is put
into one of N disjoint categories; it is disallowed for memory to ever
be accessed by an op in one category and later accessed by an op in
another category. (The frontend must ensure this.)
Then, given this, we scan the code and determine, for each
memory-accessing op, when a single prior instruction is a store to the
same category. This "colors" the instruction: it is, in a sense, a
static name for that version of memory.
This analysis provides an important invariant: if two operations access
memory with the same last-store, then *no other store can alias* in the
time between that last store and these operations. This must-not-alias
property, together with a check that the accessed address is *exactly
the same* (same SSA value and offset), and other attributes of the
access (type, extension mode) are the same, let us prove that the
results are the same.
Given last-store info, we scan the instructions and build a table from
"memory location" key (last store, address, offset, type, extension) to
known SSA value stored in that location. A store inserts a new mapping.
A load may also insert a new mapping, if we didn't already have one.
Then when a load occurs and an entry already exists for its "location",
we can reuse the value. This will be either RLE or St-to-Ld depending on
where the value came from.
Note that this *does* work across basic blocks: the last-store analysis
is a full iterative dataflow pass, and we are careful to check dominance
of a previously-defined value before aliasing to it at a potentially
redundant load. So we will do the right thing if we only have a
"partially redundant" load (loaded already but only in one predecessor
block), but we will also correctly reuse a value if there is a store or
load above a loop and a redundant load of that value within the loop, as
long as no potentially-aliasing stores happen within the loop.
* Update the wasm-tools family of crates
This commit updates these crates as used by Wasmtime for the recently
published versions to pull in changes necessary to support the component
model. I've split this out from #4005 to make it clear what's impacted
here and #4005 can simply rebase on top of this to pick up the necessary
changes.
* More test fixes
A new version of rustc was released this morning and we have a few small
breakages on our CI which need fixing:
* A new warning was coming out of the c-api crate about an unneeded
`unsafe` block.
* The panic message of a task in `cranelift-object` needed updating
since the standard library changed how it formats strings with the nul
byte.
* Upgrade to regalloc2 0.1.3.
This pulls in bytecodealliance/regalloc2#49, which slightly improves
codegen in some cases where a safepoint (for reference-typed values)
occurs in the same liverange as a register-constrained use. For
example, in bytecodealliance/wasmtime#3785, an extra move instruction
appeared and a callee-save register was used (necessitating a more
expensive prologue) because of suboptimal splitting heuristics, which
this PR fixes. The updated RA2 heuristics appear to have no measured
downsides in existing benchmarks and improve the manually-observed
codegen issue.
* Update filetests where regalloc2 improvement altered behavior with reftypes.
In #4143 we made ISLE compilation part of the normal build flow again,
to avoid the issues with the checked-in source. To make this acceptably
fast, we cut down dependencies of the ISLE compiler, so the "fancy"
error printing is now optional. When not included, it just prints error
messages to stderr in a list. However, this did not include file
locations. It might be nice to have this without enabling the "fancy
printing" and waiting for that to build.
Fortunately most of the plumbing for this was already present (we had it
at one point before switching to miette). This PR adds back locations to
the basic error output. It now looks like:
```
Error building ISLE files: ISLE errors:
src/isa/aarch64/inst.isle:1:1: parse error: Unexpected token Symbol("asdf")
```
* Don't attempt to track the generated clif.isle in cargo
This causes the build script to rerun every time for me.
* Put build script debug messages on stderr instead of stdout
This keeps stdout reserved for cargo build script directives
Current codegen had a number of logic errors confusing
NAND with AND WITH COMPLEMENT, and NOR with OR WITH COMPLEMENT.
Add support for the missing z15 instructions and fix logic.
The HashMap implementation is significantly simpler than the BTreeMap
implementation. Because of this switching reduces compilation time of
cranelift-isle by about 10%.
# Before
$ hyperfine --prepare "cargo clean" "cargo build"
Benchmark 1: cargo build
Time (mean ± σ): 5.221 s ± 0.094 s [User: 10.659 s, System: 0.734 s]
Range (min … max): 5.151 s … 5.420 s 10 runs
# After
$ hyperfine --prepare "cargo clean" "cargo build"
Benchmark 1: cargo build
Time (mean ± σ): 4.746 s ± 0.150 s [User: 9.109 s, System: 0.721 s]
Range (min … max): 4.630 s … 5.144 s 10 runs
This PR fixes#4066: it modifies the Cranelift `build.rs` workflow to
invoke the ISLE DSL compiler on every compilation, rather than only
when the user specifies a special "rebuild ISLE" feature.
The main benefit of this change is that it vastly simplifies the mental
model required of developers, and removes a bunch of failure modes
we have tried to work around in other ways. There is now just one
"source of truth", the ISLE source itself, in the repository, and so there
is no need to understand a special "rebuild" step and how to handle
merge errors. There is no special process needed to develop the compiler
when modifying the DSL. And there is no "noise" in the git history produced
by constantly-regenerated files.
The two main downsides we discussed in #4066 are:
- Compile time could increase, by adding more to the "meta" step before the main build;
- It becomes less obvious where the source definitions are (everything becomes
more "magic"), which makes exploration and debugging harder.
This PR addresses each of these concerns:
1. To maintain reasonable compile time, it includes work to cut down the
dependencies of the `cranelift-isle` crate to *nothing* (only the Rust stdlib),
in the default build. It does this by putting the error-reporting bits
(`miette` crate) under an optional feature, and the logging (`log` crate) under
a feature-controlled macro, and manually writing an `Error` impl rather than
using `thiserror`. This completely avoids proc macros and the `syn` build slowness.
The user can still get nice errors out of `miette`: this is enabled by specifying
a Cargo feature `--features isle-errors`.
2. To allow the user to optionally inspect the generated source, which nominally
lives in a hard-to-find path inside `target/` now, this PR adds a feature `isle-in-source-tree`
that, as implied by the name, moves the target for ISLE generated source into
the source tree, at `cranelift/codegen/isle_generated_source/`. It seems reasonable
to do this when an explicit feature (opt-in) is specified because this is how ISLE regeneration
currently works as well. To prevent surprises, if the feature is *not* specified, the
build fails if this directory exists.
In #4104 we discussed whether it makes sense for the division and
remainder ops to support vector types. We concluded that because most
hardware doesn't support it directly, it probably is not ideal to force
all backends to polyfill it. In the future we can always reverse this
decision, perhaps with a platform-independent legalization.
This PR restricts the allowed types on the CLIF ops to integer types
only.
This test was added between the last CI run on #4088 and its merge to
main, and the changes in #4088 (use of constants directly in instruction
via load from constant pool, rather than from a register initialized by
a separate instruction) cause it to fail now.
This PR alters the test to be invariant to regalloc and argument
decisions during lowering, as the test is really checking (per the
comment) that we get two cmoves without an intervening move. As such, it
just matches the instruction opcodes, irrespective of the arguments.
