[llvm-dev] Ok with mismatch between dead-markings in BUNDLE and bundled instructions?

[Matthias Braun via llvm-dev]

> On Jun 27, 2017, at 4:55 AM, Mikael Holmén via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> Hi Quentin and llvm-dev,
> 
> I've got a regalloc-related question that you might have an opinion or answer about.
> 
> In our out-of-tree target we've been doing some bundling before register allocation for quite some time now, and last night a new problem popped up. What the fix should be depends on if this bundle is legal or not:
> 
>    BUNDLE %vreg39<imp-def,dead>
>      * %vreg39:hiAcc<def> = mv_ar16_ar16_lo16In32 %vreg37
>      [...]
> 
> %vreg39 isn't used after the bundle so the dead-marking in the BUNDLE is correct. However, the def in the actual bundled instruction defining %vreg39 is not marked with dead.
> 
> Is the above bundle ok or not?

As to whether the bundle is ok: The register allocator should view the whole bundle as a single instruction with all operands combined. That would conceptually give us:

    %vreg39<imp-def, dead>, %vreg39:hiAcc<def> = INST...

Of course having two defs for the same register is an uncommon situation. There's some interesting arguments to be had whether this example means the whole vreg39 is dead or just the parts of vreg39 that are not covered by the hiAcc subregister. I think in todays code it means that the whole vreg39 register is dead (though I cannot say off hand whether we do this consistently or whether it is actually documented somewhere).

> 
> 
> When the register allocator later tries to spill/reload %vreg39 it thinks that %vreg39 is live, so it inserts a reload after the bundle, but then the verifier pukes on it and says that there is a use of the register after it's dead.
> 
> The dead-marking on the BUNDLE is introduced by LiveIntervals::computeDeadValues which does
> 
>      // This is a dead def. Make sure the instruction knows.
>      MachineInstr *MI = getInstructionFromIndex(Def);
>      assert(MI && "No instruction defining live value");
>      MI->addRegisterDead(LI.reg, TRI);

I think the actual problem here is that MachineInstr::addRegisterDead only works on a single instruction and not on the whole bundle. 

- Matthias