Make multiple defs of one vreg possible on one instruction.#38
Conversation
|
As a meta-comment, I tried my best to work around requiring this change in regalloc2 at first; e.g. deduplicating at the Cranelift level, or only using RealRegs (and deduplicating those), or ... but I kept getting a big mess. I think this is cleaner; it generalizes the class of programs we can accept and makes life easier for embedders. |
|
I'm not sure how I feel about this since the assert may help catch unintentional bugs in codegen. Could this be made opt-in in some way? Perhaps |
Currently, if this is done, overlapping liveranges are created, and we hit an assert that ensures our non-overlapping and built-in-reverse-order invariants during liverange construction. One could argue that multiple defs of a single vreg don't make a ton of sense -- which def's value is valid after the instruction? -- but if they all get the same alloc, then the answer is "whatever was put in that alloc", and this is just a case of an instruction being a bit over-eager when listing its registers. This can arise in practice when operand lists come from combinations or concatenations: for example, in Cranelift's s390x backend, there is a "Loop" pseudo-instruction, and the operands of the Loop are the operands of all the sub-instructions. It seems more logically cohesive overall to say that one can state an operand as many times as one likes; so this PR makes it so.
7388c60 to
4edc497
Compare
|
Sure, yep, that's a good idea; I've added that trait method with it defaulting to Longer-term, I think it should be possible to eliminate the need for this support: I'd like to do something about our macroinstructions in Cranelift in general, and solving that would make for a cleaner input where every inst is a simple one without a variable number of args (EDIT: except calls I guess but those we can handle without this) and/or duplicated PRegs. I'll file a followup issue on the Cranelift side once we merge initial regalloc2 integration. |
Currently, if this is done, overlapping liveranges are created, and we
hit an assert that ensures our non-overlapping and
built-in-reverse-order invariants during liverange construction.
One could argue that multiple defs of a single vreg don't make a ton of
sense -- which def's value is valid after the instruction? -- but if
they all get the same alloc, then the answer is "whatever was put in
that alloc", and this is just a case of an instruction being a bit
over-eager when listing its registers.
This can arise in practice when operand lists come from combinations or
concatenations: for example, in Cranelift's s390x backend, there is a
"Loop" pseudo-instruction, and the operands of the Loop are the operands
of all the sub-instructions.
It seems more logically cohesive overall to say that one can state an
operand as many times as one likes; so this PR makes it so.