| 49dd8fd7 | 16-May-2023 |
Alex Crichton <[email protected]> |
aarch64: Fix Ldr19 relocations being unresolvable (#6384)
* Add a cranelift setting for padding between basic blocks
Various relocations, jumps, and such require special handling in
`MachBuffer` w
aarch64: Fix Ldr19 relocations being unresolvable (#6384)
* Add a cranelift setting for padding between basic blocks
Various relocations, jumps, and such require special handling in
`MachBuffer` with respect to islands to ensure that everything gets
emitted correctly. This commit adds a setting to synthetically insert
padding at the end of every basic block to help stress this logic with
more minimal test cases. The setting is disabled by default but is
something that we should be able to turn on during fuzzing, for example.
* aarch64: Fix out-of-range `Ldr19` relocations
This commit fixes a bug in the AArch64 backend, and possibly others,
where constants were unconditionally forced to be at the end of the
function when they sometimes couldn't be. For example the `Ldr19`
relocation has a 512k range meaning that if an instruction near the
beginning of a function accesses a constant at the end of a function and
the function is >1M, then the relocation cannot be resolved. This is all
handled internally with `MachBuffer`'s handling of islands but the
problem with constants is that the labels (and the constant values)
weren't defined until the end of the function.
The first attempt at fixing this was to move the calls to
`defer_constant` to the beginning of emission. This would enable the
constants to get deferred as necessary. This was problematic, however,
because it only solved the forwards case (aka your constant was forced
to the end of the function which is too far away). The backwards case,
aka your constant is way too far behind you, was a new problem that
arose.
To fix all of these issues constants are now handled differently inside
of the `MachBuffer`. Previously constants were all pre-assigned a
label-per-constant and all references to the constant would use that
single label. Instead a new heuristic has been added where constants
record their size/alignment at the start of emission and labels are
lazily deferred. When a label for a constant is requested then a label
is lazily allocated or a previously-allocated label for this constant is
returned. When an island is emitted then all emitted constants get
their labels cleared. This intends to balance the previous functionality
of multiple uses of a constant only emit the constant once with fixing
this issue with simplicity as well. This means that constants may get
emitted multiple times, since each reference to a constant after an
island is generated will be guaranteed to generate a new label, even if
it's in-range to access. This can perhaps be fixed in the future with a
more clever API where the `LabelUse` is passed into the function which
converts a constant to a label, but that's left as a refactoring for a
future date.
This commit also moves an `alignment: u32` field into the
`MachBufferFinalized` itself since that's now a function of whatever
constants actually got emitted. Additionally note that constant
emission in the middle of a function doesn't actually emit anything,
instead recording markers of where constants need to go. Then when a
buffer is finalized the constants are passed in to get access to the
data which fills in everything as it's referenced.
* Fuzz the `bb_padding_log2` setting
This commit hooks up the previously-added setting to Cranelift to
Wasmtime's fuzzing infrastructure. This will automatically configure the
setting based on the fuzz input to add a bit of "chaos" to the emitted
code. This should hopefully help expose the issue fixed previously via
fuzzing which otherwise won't generate massive functions.
* Realign back to an instruction boundary
Otherwise misaligned instructions were getting emitted and tripping
various asserts.
* Fix riscv64 testing
* Rename codegen setting to bb_padding_log2_minus_one
Allow for inserting one byte of padding.
* Doc updates
* Thread through shared flags differently
Don't use `EmitInfo`, instead pass in to vcode emission
* Fix s390x tests
* Combine island calculations during vcode emission
Fixes an off-by-just-a-few error if the two island checks are done
separately after a basic block.
show more ...
|