bytecodealliance / regalloc2

A new register allocator
Apache License 2.0
209 stars 38 forks source link

Allow fixed-early-def and fixed-late-use constraints #168

Closed Amanieu closed 9 months ago

Amanieu commented 10 months ago

These are useful to model fixed registers which should not be reused for other uses/defs. These were disallowed in #54, but this was too conservative.

Fundamentally, the only situation where a preg can be used in multiple fixed constraints within a single instruction is with an early-use and a late-def. Anything else is a user error because the live ranges will overlap.

As such this PR relaxes the operand rewrite from #54 to only apply in this specific situation. Fixed-late-use and fixed-early-def operands are left unchanged.

Amanieu commented 9 months ago

I personally don't have a use case for fixed-late-use, I only needed fixed-early-def in my code. I just allowed fixed-late-use for symmetry.

Specifically, what guarantees that we'd never try to generate a move in the middle of an instruction to satisfy a late-use? Currently we unconditionally use the CodeRange::from program point when inserting moves, so if the liverange containing a late-use got trimmed to only include that use, what prevents us from trying to insert the move at the late position?

Live range trimming/splitting will only split live ranges on instruction boundaries, never in the middle of an instruction. Since the value of a late use must necessarily come from a previous instruction, a move will simply be inserted before the instruction.

elliottt commented 9 months ago

Live range trimming/splitting will only split live ranges on instruction boundaries, never in the middle of an instruction. Since the value of a late use must necessarily come from a previous instruction, a move will simply be inserted before the instruction.

Ah right, thanks!

Separately, it sounds like @afonso360 has a use for fixed-late-use constraints in the riscv64 backend in cranelift.

afonso360 commented 9 months ago

I was looking for the PR where I first needed this, and it looks like it was https://github.com/bytecodealliance/wasmtime/pull/6568 but weirdly, I added a fixed-late-use operand, and it worked out fine? It's still in there.

Here's some of the discussion regarding this: https://github.com/bytecodealliance/wasmtime/pull/6568#discussion_r1227347838

Now that I'm looking at this again I'm not sure if a fixed-late-use operand is the same as a fixed-late use constraint?

In any case I'm going to run the SIMD testsuite with this PR since it sounds like these changes would be triggered with the existing code.


As an explanation of what is going on in that PR, it introduces the vslideup instruction, which has some special constraints, namely:

elliottt commented 9 months ago

It looks like the instruction is only generated with an (unmasked) argument for mask, which will not cause the fixed-late-use to be generated. I think that explains why we're not seeing the assertion failure for that case.

afonso360 commented 9 months ago

That makes sense. I checked the rest of the backend, and we currently only use masks with the vmerge instruction, which does not request a fixed-late-use operand on the mask.