PCC: remove proof-carrying code (for now?). (#12800)

In late 2023, we built out an experimental feature called
Proof-Carrying Code (PCC), where we attached "facts" to values in the
CLIF IR and built

PCC: remove proof-carrying code (for now?). (#12800)

In late 2023, we built out an experimental feature called
Proof-Carrying Code (PCC), where we attached "facts" to values in the
CLIF IR and built verification of these facts after lowering to
machine instructions. We also added "memory types" describing layout
of memory and a "checked" flag on memory operations such that we could
verify that any checked memory operation accessed valid memory (as
defined by memory types attached to pointer values via
facts). Wasmtime's Cranelift backend then put appropriate memory types
and facts in its IR such that all accesses to memory (aspirationally)
could be checked, taking the whole mid-end and lowering backend of
Cranelift out of the trusted core that enforces SFI.

This basically worked, at the time, for static memories; but never for
dynamic memories, and then work on the feature lost
prioritization (aka I had to work on other things) and I wasn't able
to complete it and put it in fuzzing/enable it as a production option.

Unfortunately since then it has bit-rotted significantly -- as we add
new backend optimizations and instruction lowerings we haven't kept
the PCC framework up to date.

Inspired by the discussion in #12497 I think it's time to delete
it (hopefully just "for now"?) unless/until we can build it again. And
when we do that, we should probably get it to the point of validating
robust operation on all combinations of memory configurations before
merging. (That implies a big experiment branch rather than a bunch of
eager PRs in-tree, but so it goes.) I still believe it is possible to
build this (and I have ideas on how to do it!) but not right now.

show more ...

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

Cranelift: add debug tag infrastructure. (#11768)

* Cranelift: add debug tag infrastructure.

This PR adds *debug tags*, a kind of metadata that can attach to CLIF
instructions and be lowered to VCo

Cranelift: add debug tag infrastructure. (#11768)

* Cranelift: add debug tag infrastructure.

This PR adds *debug tags*, a kind of metadata that can attach to CLIF
instructions and be lowered to VCode instructions and as metadata on
the produced compiled code. It also adds opaque descriptor blobs
carried with stackslots. Together, these two features allow decorating
IR with first-class debug instrumentation that is properly preserved
by the compiler, including across optimizations and
inlining. (Wasmtime's use of these features will come in followup
PRs.)

The key idea of a "debug tag" is to allow the Cranelift embedder to
express whatever information it needs to, in a format that is opaque
to Cranelift itself, except for the parts that need translation during
lowering. In particular, the `DebugTag::StackSlot` variant gets
translated to a physical offset into the stackframe in the compiled
metadata output. So, for example, the embedder can emit a tag
referring to a stackslot, and another describing an offset in that
stackslot.

The debug tags exist as a *sequence* on any given instruction; the
meaning of the sequence is known only to the embedder, *except* that
during inlining, the tags for the inlining call instruction are
prepended to the tags of inlined instructions. In this way, a
canonical use-case of tags as describing original source-language
frames can preserve the source-language view even when multiple
functions are inlined into one.

The descriptor on a stackslot may look a little odd at first, but its
purpose is to allow serializing some description of
stackslot-contained runtime user-program data, in a way that is firmly
attached to the stackslot. In particular, in the face of inlining,
this descriptor is copied into the inlining (parent) function from the
inlined function when the stackslot entity is copied; no other
metadata outside Cranelift needs to track the identity of stackslots
and know about that motion. This fits nicely with the ability of tags
to refer to stackslots; together, the embedder can annotate
instructions as having certain state in stackslots, and describe the
format of that state per stackslot.

This infrastructure is tested with some compile-tests now;
testing of the interpretation of the metadata output will come with
end-to-end debug instrumentation tests in a followup PR.

* Review feedback: add back sequence points and enforce tags only on sequence points or calls.

* Use Vecs for debug metadata in MachBuffer to avoid SmallVec size penalty in not-used case.

* Review feedback: switch from inlined stackslot descriptor blobs to u64 keys.

show more ...

Couple cleanups to the flags/settings handling in Cranelift (#11744)

* Remove unused shared flags

* Get rid of predicate settings

They were important in the old backend framework, but with the new

Couple cleanups to the flags/settings handling in Cranelift (#11744)

* Remove unused shared flags

* Get rid of predicate settings

They were important in the old backend framework, but with the new
backend framework if we need a combination of multiple settings, that
can just be done as a regular extractor doing &&. This simplifies the
settings implementation.

show more ...

Cranelift: add get_exception_handler_address. (#11629)

* Cranelift: add get_exception_handler_address.

This is designed to enable applications such as #11592 that use
alternative unwinding mechanis

Cranelift: add get_exception_handler_address. (#11629)

* Cranelift: add get_exception_handler_address.

This is designed to enable applications such as #11592 that use
alternative unwinding mechanisms that may not necessarily want to walk a
stack and look up exception tables. The idea is that whenever it would
be valid to resume to an exception handler that is active on the stack,
we can provide the same PC as a first-class runtime value that would be
found in the exception table for the given handler edge. A "custom"
resume step can then use this PC as a resume-point as long as it follows
the relevant exception ABI (i.e.: restore SP, FP, any other saved
registers that the exception ABI specifies, and provide appropriate
payload value(s)).

Handlers are associated with edges out of `try_call`s (or
`try_call_indirect`s); and edges specifically, not blocks, because there
could be multiple out-edges to one block. The instruction thus takes the
block that contains the try-call and an immediate that indexes its
exceptional edges.

This CLIF instruction required a bit of infrastructure to (i) allow
naming raw blocks, not just block calls, as instruction arguments, and
(ii) allow getting the MachLabel for any other lowered block during
lowering. But given that, the lowerings themselves are straightforward
uses of MachBuffer labels to fix-up PC-relative address-loading
instructions (e.g., `LEA` or `ADR` or `AUIPC`+`ADDI`).

* Review feedback.

* Review feedback: more tests.

show more ...

Cranelift: support dynamic contexts in exception-handler lists. (#11321)

In #11285, we realized that Wasm semantics require us to match on
dynamic instances of exception tags, rather than static tag

Cranelift: support dynamic contexts in exception-handler lists. (#11321)

In #11285, we realized that Wasm semantics require us to match on
dynamic instances of exception tags, rather than static tag types. This
fundamentally requires the unwinder to be able to resolve the current
Wasm instance for each Wasm frame on the stack that has any handlers,
and our frame format does not provide this today.

We discussed many options, some of which solve the more general problem
(Wasm vmctx for any frame), but ultimately landed on a notion of
"dynamic context for evaluating tags", specific to Cranelift's
exception-catch metadata; and storing that context and carrying it
through to a place that is named in the unwind metadata. The reasoning
is fairly straightforward: we cannot afford a more general approach that
stores vmctx in every frame (I measured this at 20% overhead for a
recursive-Fibonacci benchmark that is call-intensive); and inlining
means that we may have *multiple* contexts at any given program point,
each associated with a different slice of the handler tags; so we need a
mechanism that, *just for a try-call*, intersperses contexts with tags
(or puts a context on each tag) and stores these somewhere that the
exception-unwind ABI doesn't clobber (e.g., on the stack).

This PR implements "option 4" from that issue, namely, *dynamic
exception contexts*. The idea is that this is the dual to exception
payload: while payload lets the unwinder communicate state *to* the
catching code, context lets the unwinder take state *from* the catching
code that lets it decide whether the tag is a match. Because of
inlining, we need to either associate (optional) context with every tag,
or intersperse context-updates with handler tags. I've opted for the
latter for efficiency at the CLIF level (in most cases there will be
multiple tags per context), though they are isomorphic.

The new tag-matching semantics are: when walking up the stack, upon
reaching a `try_call`, evaluate catch-clauses in listed order. A
`context` clause sets the current context. A `tagN: block(...)` clause
attempts to match the throwing exception against `tagN`, *evaluated in
the current context*, and branches to the named block if it matches. A
`default: block(...)` always branches to the named block.

Note that this lets us assume less about tags than before, and this
particularly manifests in the changes to the inliner. Whereas before,
`tagN` is `tagN` and an inner handler for that tag shadows an outer
handler (that is, tags always alias if identical indices); and whereas
before, `tagN` is not `tagM` and so we can order the tags arbitrarily
(that is, tags never alias if non-identical indices); now any two static
tag indices may or may not alias depending on the dynamic context of
each. Or, even in the same context, two may alias, because we leave the
match-predicate as an unspecified (user-chosen) algorithm during
unwinding. (This mirrors the reality that, for example, a Wasm instance
may import two tags, and dynamically these tags may be equal or
different at runtime, even instantiation-to-instantiation.) Cranelift's
only job is to faithfully carry the list of contexts and tags through to
the compiled-code metadata; and to ensure that they remain in the order
they were specified in the CLIF.

This PR introduces the Cranelift-level feature, and it will be used in
a subsequent PR that introduces Wasm exception handling. Because of
that, I've opted not to update the clif-utils runtest "runtime" to read
out contexts and do something with them -- we will have plenty of test
coverage via a bunch of Wasm tests for corner cases such as the above.
This PR does include filetests that show that contexts are carried
through to spillslots and those appear in the metadata.

Fixes #11285.

show more ...

Enable the `useless_conversion` Clippy lint (#10838)

* Enable the `useless_conversion` Clippy lint

We've got lots of types in Wasmtime and convert between them quite a
lot, but often over time conv

Enable the `useless_conversion` Clippy lint (#10838)

* Enable the `useless_conversion` Clippy lint

We've got lots of types in Wasmtime and convert between them quite a
lot, but often over time conversions become unnecessary through
refactorings or similar. This will hopefully enable us to clean up some
conversions as they come up to try to have as few as possible ideally.

* Review comments

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

Add support for loading, storing and bitcasting small vectors on x64 and aarch64 (#10693)

Cranelift: initial try_call / try_call_indirect (exception) support. (#10510)

* Cranelift: initial try_call / try_call_indirect (exception) support.

This PR adds `try_call` and `try_call_indirect`

Cranelift: initial try_call / try_call_indirect (exception) support. (#10510)

* Cranelift: initial try_call / try_call_indirect (exception) support.

This PR adds `try_call` and `try_call_indirect` instructions, and
lowerings on four of five ISAs (x86-64, aarch64, riscv64, pulley; s390x
has its own non-shared ABI code that will need separate work).

It extends CLIF to support these instructions as new kinds of branches,
and extends block-calls to accept `retN` and `exnN` block-call args that
carry the normal return values or exception payloads (respectively) into
the appropriate successor blocks.

It wires up the "normal return path" so that it continues to work.
It updates the ABI so that unwinding is possible without an initial
register state at throw: specifically, as per our RFC, all registers are
clobbered. It also includes metadata in the `MachBuffer` that describes
exception-catch destinations. However, no unwinder exists to interpret
these catch-destinations yet, so they are untested.

* Add try_call_indirect lowering as well.

show more ...

Enforce `uninlined_format_args` for the workspace (#9065)

* Enforce `uninlined_format_args` for the workspace

* fix: failing `Monolith Checks` job

* fix: formatting

Enable a few more clippy lints by default (#9038)

Also set their default levels to `warn` instead of `deny` to assist with
using clippy locally to avoid compilation failures and collecting as
many a

Enable a few more clippy lints by default (#9038)

Also set their default levels to `warn` instead of `deny` to assist with
using clippy locally to avoid compilation failures and collecting as
many as possible in one run. Note that CI will still deny-by-default
through `-Dwarnings`.

show more ...

cranelift: Add `f16const` and `f128const` instructions (#8893)

* cranelift: Add `f16const` and `f128const` instructions

* cranelift: Add constant propagation for `f16` and `f128`

Initial `f16` and `f128` support (#8860)

clif: Fix parsing the `cold` calling convention (#8854)

The identifier for the `cold` calling convention overlaps with the
`cold` keyword for basic blocks so handle another kind of token when
parsin

clif: Fix parsing the `cold` calling convention (#8854)

The identifier for the `cold` calling convention overlaps with the
`cold` keyword for basic blocks so handle another kind of token when
parsing signatures.

show more ...

Remove leftover debugging `dbg!` in CLIF parser (#8836)

Cranelift: Allow CLIF frontends to define their own stack maps (#8728)

Tracking GC references and producing stack maps is a significant amount of
complexity in `regalloc2`.

At the same time, GC ref

Cranelift: Allow CLIF frontends to define their own stack maps (#8728)

Tracking GC references and producing stack maps is a significant amount of
complexity in `regalloc2`.

At the same time, GC reference value types are pretty annoying to deal with in
Cranelift itself. We know our `r64` is "actually" just an `i64` pointer, and we
want to do `i64`-y things with it, such as an `iadd` to compute a derived
pointer, but `iadd` only takes integer types and not `r64`s. We investigated
loosening that restriction and it was way too painful given the way that CLIF
type inference and its controlling type vars work. So to compute those derived
pointers, we have to first `bitcast` the `r64` into an `i64`. This is
unfortunate in two ways. First, because of arcane interactions between register
allocation constraints, stack maps, and ABIs this involves inserting unnecessary
register-to-register moves in our generated code which hurts binary size and
performance ever so slightly. Second, and much more seriously, this is a serious
footgun. If a GC reference isn't an `r64` right now, then it will not appear in
stack maps, and failure to record a live GC reference in a stack map means that
the collector could reclaim the object while you are still using it, leading to
use-after-free bugs! Very bad. And the mid-end needs to know
*not* to GVN these bitcasts or else we get similar bugs (see
https://github.com/bytecodealliance/wasmtime/pull/8317).

Overall GC references are a painful situation for us today.

This commit is the introduction of an alternative. (Note, though, that we aren't
quite ready to remove the old stack maps infrastructure just yet.)

Instead of preserving GC references all the way through the whole pipeline and
computing live GC references and inserting spills at safepoints for stack maps
all the way at the end of that pipeline in register allocation, the
CLIF-producing frontend explicitly generates its own stack slots and spills for
safepoints. The only thing the rest of the compiler pipeline needs to know is
the metadata required to produce the stack map for the associated safepoint. We
can completely remove `r32` and `r64` from Cranelift and just use plain `i32`
and `i64` values. Or `f64` if the runtime uses NaN-boxing, which the old stack
maps system did not support at all. Or 32-bit GC references on a 64-bit target,
which was also not supported by the old system. Furthermore, we *cannot* get
miscompiles due to GVN'ing bitcasts that shouldn't be GVN'd because there aren't
any bitcasts hiding GC references from stack maps anymore. And in the case of a
moving GC, we don't need to worry about the mid-end doing illegal code motion
across calls that could have triggered a GC that invalidated the moved GC
reference because frontends will reload their GC references from the stack slots
after the call, and that loaded value simply isn't a candidate for GVN with the
previous version. We don't have to worry about those bugs by construction.

So everything gets a lot easier under this new system.

But this commit doesn't mean we are 100% done and ready to transition to the new
system, so what is actually in here?

* CLIF producers can mark values as needing to be present in a stack map if they
are live across a safepoint in `cranelift-frontend`. This is the
`FunctionBuilder::declare_needs_stack_map` method.

* When we finalize the function we are building, we do a simple, single-pass
liveness analysis to determine the set of GC references that are live at each
safepoint, and then we insert spills to explicit stack slots just before the
safepoint. We intentionally trade away the precision of a fixed-point liveness
analysis for the speed and simplicity of a single-pass implementation.

* We annotate the safepoint with the metadata necessary to construct its
associated stack map. This is the new
`cranelift_codegen::ir::DataFlowGraph::append_user_stack_map_entry` method and
all that stuff.

* These stack map entries are part of the CLIF and can be roundtripped through
printing and parsing CLIF.

* Each stack map entry describes a GC-managed value that is on the stack and how
to locate it: its type, the stack slot it is located within, and the offset
within that stack slot where it resides. Different stack map entries for the
same safepoint may have different types or a different width from the target's
pointer.

Here is what is *not* handled yet, and left for future follow up commits:

* Lowering the stack map entries' locations from symbolic stack slot and offset
pairs to physical stack frame offsets after register allocation.

* Coalescing and aggregating the safepoints and their raw stack map entries into
a compact PC-to-stack-map table during emission.

* Supporting moving GCs. Right now we generate spills into stack slots for live
GC references just before safepoints, but we don't reload the GC references from
the stack upon their next use after the safepoint. This involves rewriting uses
of the old, spilled values which could be a little finicky, but we think we have
a good approach.

* Port Wasmtime over to using this new stack maps system.

* Removing the old stack map system, including `r{32,64}` from Cranelift and GC
reference handling from `regalloc2`. (For the time being, the new system
generally refers to "user stack maps" to disambiguate from the old system where
it might otherwise be confusing.) If we wanted to remove the old system now,
that would require us to also port Wasmtime to the new system now, and we'd end
up with a monolithic PR. Better to do this incrementally and temporarily have
the old and in-progress new system overlap for a short period of time.

Co-authored-by: Trevor Elliott <[email protected]>

show more ...

Enable rustc's `unused-lifetimes` lint (#8711)

* Enable rustc's `unused-lifetimes` lint

This is allow-by-default doesn't seem to have any false positives in
Wasmtime's codebase so enable it by defa

Enable rustc's `unused-lifetimes` lint (#8711)

* Enable rustc's `unused-lifetimes` lint

This is allow-by-default doesn't seem to have any false positives in
Wasmtime's codebase so enable it by default to help clean up vestiges of
old refactorings.

* Remove another unused lifetime

* Remove another unused lifetime

show more ...

Allow empty block param lists in clif files (#8698)

Fix some typos (#8641)

* occurred

* winch typos

* tests typos

* cli typos

* fuzz typos

* examples typos

* docs typos

* crates/wasmtime typos

* crates/environ typos

* crates/cranelift typos

Fix some typos (#8641)

* occurred

* winch typos

* tests typos

* cli typos

* fuzz typos

* examples typos

* docs typos

* crates/wasmtime typos

* crates/environ typos

* crates/cranelift typos

* crates/test-programs typos

* crates/c-api typos

* crates/cache typos

* crates other typos

* cranelift/codegen/src/isa typos

* cranelift/codegen/src other typos

* cranelift/codegen other typos

* cranelift other typos

* ci js typo

* .github workflows typo

* RELEASES typo

* Fix clang-format documentation line

---------

Co-authored-by: Andrew Brown <[email protected]>

show more ...

Cranelift: add alignment parameter to stack slots. (#8635)

* Cranelift: add alignment parameter to stack slots.

Fixes #6716.

Currently, stack slots on the stack are aligned only to a machine-word

Cranelift: add alignment parameter to stack slots. (#8635)

* Cranelift: add alignment parameter to stack slots.

Fixes #6716.

Currently, stack slots on the stack are aligned only to a machine-word
boundary. This is insufficient for some use-cases: for example, storing
SIMD data or structs that require a larger alignment.

This PR adds a parameter to the `StackSlotData` to specify alignment,
and the associated logic to the CLIF parser and printer. It updates the
shared ABI code to compute the stackslot layout taking the alignment
into account. In order to ensure the alignment is always a power of two,
it is stored as a shift amount (log2 of actual alignment) in the IR.

* Apply suggestions from code review

Co-authored-by: Trevor Elliott <[email protected]>

* Update filetest.

* Update alignment to ValRaw vector.

* Fix printer test.

* cargo-fmt from suggestion update.

---------

Co-authored-by: Trevor Elliott <[email protected]>

show more ...

Refactor the `MemFlags` internal representation (#8162)

* Refactor the `MemFlags` internal representation

This commit refactors the internal `MemFlags` bits to be not just flags.
Instead some bits

Refactor the `MemFlags` internal representation (#8162)

* Refactor the `MemFlags` internal representation

This commit refactors the internal `MemFlags` bits to be not just flags.
Instead some bits are now grouped together and interpreted as a unit.
This enables two primary API changes:

* First the `heap`/`table`/`vmctx` split is now represented as an "alias
region" which is set as an enum. This means that all `MemFlags` carry
an `Option<AliasRegion>` internally.

* Second trapping state is now represented as an `Option<TrapCode>`.
This means that `notrap` is no longer a flag and `tabletrap` is no
longer a flag. This does enable storing arbitrary trap codes though so
long as they aren't `TrapCode::User(_)`.

The main purpose of this commit is to enable using more trap codes with
`MemFlags` for when a segfault is detected. For example with #5291 we
want a segfault to indicate a call-to-null, which is not currently
covered by `MemFlags`.

* Remove individual alias region setters/getters from `MemFlags`

Instead use the `AliasRegion` enum instead to help emphasize that only
one region is possible on a `MemFlags`, not multiple.

* Update cranelift/codegen/src/ir/memflags.rs

Co-authored-by: Jamey Sharp <[email protected]>

---------

Co-authored-by: Jamey Sharp <[email protected]>

show more ...

cranelift: Remove support for WebAssembly tables (#8124)

Wasmtime no longer needs any of this infrastructure and neither should
anybody else.

This diff is nearly identical to @bjorn3's version of t

cranelift: Remove support for WebAssembly tables (#8124)

Wasmtime no longer needs any of this infrastructure and neither should
anybody else.

This diff is nearly identical to @bjorn3's version of the same change,
except I didn't remove Uimm64, which has started being used in other
places. I forgot bjorn3 had already tackled this part until after I was
already done, but it's reassuring that we both made the same changes.

https://github.com/bjorn3/wasmtime/commit/fb82ccb3948e949641a6d9581aa84472f68f97b8

Fixes #5532

show more ...

Update some CI dependencies (#7983)

* Update some CI dependencies

* Update to the latest nightly toolchain
* Update mdbook
* Update QEMU for cross-compiled testing
* Update `cargo nextest` for usag

Update some CI dependencies (#7983)

* Update some CI dependencies

* Update to the latest nightly toolchain
* Update mdbook
* Update QEMU for cross-compiled testing
* Update `cargo nextest` for usage with MIRI

prtest:full

* Remove lots of unnecessary imports

* Downgrade qemu as 8.2.1 seems to segfault

* Remove more imports

* Remove unused winch trait method

* Fix warnings about unused trait methods

* More unused imports

* More unused imports

show more ...

PCC: fully support dynamic and static memories in Wasmtime on x86-64 and aarch64. (#7468)

* PCC: refactor heap memory-type creation to allow for use of GVs in facts.

Co-authored-by: Nick Fitzgerald

PCC: fully support dynamic and static memories in Wasmtime on x86-64 and aarch64. (#7468)

* PCC: refactor heap memory-type creation to allow for use of GVs in facts.

Co-authored-by: Nick Fitzgerald <[email protected]>

* PCC: WIP: add some facts for dynamic memories.

Co-authored-by: Nick Fitzgerald <[email protected]>

* PCC: WIP: add fact to loaded length on dynamic memories.

Co-authored-by: Nick Fitzgerald <[email protected]>

* PCC: add facts for dynamic bounds-checks and extend PCC verification as needed.

This commit also adds an integration test to Wasmtime that checks all
dynamic and static memory cases on x86-64 and aarch64.

Test expected-output changes are due to a change from `iadd_imm(x, -k)`
to `isub(x, iconst(k))` in bounds-check code to facilitate facts on the
computation.

* turn off trace-log in cranelift-codegen.

* rustfmt

* Review feedback.

* Avoid unused-item warnings on s390x/riscv64 in pcc test.

* ignore PCC test in miri run.

---------

Co-authored-by: Nick Fitzgerald <[email protected]>

show more ...