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

[Quentin Colombet via llvm-dev]

Hi Mikael,

Thanks for bringing that up.

> Le 27 juin 2017 à 04:55, Mikael Holmén <mikael.holmen at ericsson.com> a écrit :
> 
> 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?

Ultimately, I would like this to be the way we represent these situations, unfortunately the code base is not ready for that yet.

> 
> 
> 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);
> 
> and the reload after the BUNDLE is introduced by InlineSpiller::spillAroundUses which has determined that the register is live:
> 
>    // Rewrite instruction operands.
>    bool hasLiveDef = false;
>    for (const auto &OpPair : Ops) {
>      MachineOperand &MO = OpPair.first->getOperand(OpPair.second);
>      MO.setReg(NewVReg);
>      if (MO.isUse()) {
>        if (!OpPair.first->isRegTiedToDefOperand(OpPair.second))
>          MO.setIsKill();
>      } else {
>        if (!MO.isDead())
>          hasLiveDef = true;
>      }
> 
> 
> So I suppose we could either make LiveIntervals::computeDeadValues mark the individual defs dead as well,

I didn't get that.

> or we could change InlineSpiller::spillAroundUses so that if we only look at the BUNDLE instruction (if it exists) in case of bundled instructions when looking for defs.
> 
> Any opinions on what way to go?
> 

I believe we should teach the spiller about the subregs so that it understands a particular subregs is dead.


Cheers,
Q


> Thanks,
> Mikael