Come up with a better scheme for fixed-register usages #3
Comments
cfallin
added a commit
to cfallin/regalloc2
that referenced
this issue
Jun 26, 2022
Right now, pinned vregs are a way of naming real registers (a
compatibility shim of sorts for Cranelift's `RealReg`s) and can be used
as sources and dests of moves. When the input program does so, regalloc2
converts these into "ghost" uses and defs on the other vreg of the move
(dest or source, respectively) with a fixed-register constraint. So
`move v128, v0` where `v0` is pinned to `p0` turns into a "ghost def"
on `v128` with constraint `fixed p0`.
There is some fancy manipulation of liveranges to make this all work
while properly recording where the preg's value must be preserved.
Unfortunately, there was an off-by-one in the location of the move and
transition of live-ranges which interacts poorly with the "implicit
live-in" of pinned vregs at function start. As a result, a function body
that starts like:
```
move v128, v0
def v9000
move v129, v1
```
might allocate `p1` (to which `v1` is pinned) for `v9000`. This clobbers
the original value.
Fortunately this only impacts the implicit live-in, and Cranelift's use
of regalloc2 is such that it will always copy all values out of pinned
vregs (creating ghost defs) without intervening defs, *except* in the
case of `sret` ("structure return") arguments. If a program does not use
`sret` arguments (and the `cranelift-wasm` frontend does not), then this
bug should not be reachable.
Long-term, we really need to kill pinned vregs with fire (bytecodealliance#3); the
special cases that arise from these, and from special handling of moves,
are too much incidental complexity. All of this can go away once
Cranelift migrates all fixed-register cases to operand constraints
instead. That will be a happy day.
Thanks to @bjorn3 for finding and reporting this issue!
cfallin
added a commit
that referenced
this issue
Jun 27, 2022
Right now, pinned vregs are a way of naming real registers (a
compatibility shim of sorts for Cranelift's `RealReg`s) and can be used
as sources and dests of moves. When the input program does so, regalloc2
converts these into "ghost" uses and defs on the other vreg of the move
(dest or source, respectively) with a fixed-register constraint. So
`move v128, v0` where `v0` is pinned to `p0` turns into a "ghost def"
on `v128` with constraint `fixed p0`.
There is some fancy manipulation of liveranges to make this all work
while properly recording where the preg's value must be preserved.
Unfortunately, there was an off-by-one in the location of the move and
transition of live-ranges which interacts poorly with the "implicit
live-in" of pinned vregs at function start. As a result, a function body
that starts like:
```
move v128, v0
def v9000
move v129, v1
```
might allocate `p1` (to which `v1` is pinned) for `v9000`. This clobbers
the original value.
Fortunately this only impacts the implicit live-in, and Cranelift's use
of regalloc2 is such that it will always copy all values out of pinned
vregs (creating ghost defs) without intervening defs, *except* in the
case of `sret` ("structure return") arguments. If a program does not use
`sret` arguments (and the `cranelift-wasm` frontend does not), then this
bug should not be reachable.
Long-term, we really need to kill pinned vregs with fire (#3); the
special cases that arise from these, and from special handling of moves,
are too much incidental complexity. All of this can go away once
Cranelift migrates all fixed-register cases to operand constraints
instead. That will be a happy day.
Thanks to @bjorn3 for finding and reporting this issue!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In many cases, a regalloc2 client will want to generate instructions that refer to fixed registers for uses/defs. There are multiple kinds of these register references: (i) uses/defs of a vreg that needs to be in a certain fixed register just at one instruction (e.g. for a call argument or return value), (ii) uses/defs of a machine register that is not allocatable but is used in an operand slot that can also take allocatable registers.
The former is well-covered by
OperandConstraint::FixedReg, but the latter is interesting; for now our answer is to use pinned vregs, but we should have a better way of "passing through" non-allocatable registers, and we should clearly define what the semantics are.The text was updated successfully, but these errors were encountered: