[llvm-commits] [llvm] r112825 - in /llvm/trunk: lib/Target/ARM/ARMExpandPseudoInsts.cpp lib/Target/ARM/ARMISelDAGToDAG.cpp lib/Target/ARM/ARMInstrNEON.td lib/Target/ARM/NEONPreAllocPass.cpp test/CodeGen/ARM/reg_sequence.ll

[Jakob Stoklund Olesen]

Hi Bob,

On Sep 2, 2010, at 9:00 AM, Bob Wilson wrote:

> +  MIB.addReg(D0).addReg(D1);
> +  if (NumRegs > 2)
> +    MIB.addReg(D2);
> +  if (NumRegs > 3)
> +    MIB.addReg(D3);

I think you need to add <dead> flags here.

[...]

> +  // Add an implicit def for the super-reg.
> +  MIB.addReg(DstReg, (getDefRegState(true) | getDeadRegState(DstIsDead) |
> +                      getImplRegState(true)));

Just like you did here. (Also note that getDefRegState(true) | getImplRegState(true) can be spelled as RegState::ImplicitDefine)

This part is tricky, but a sub-register <def> takes precedence over a super-register <def,dead>, so the sub-register is resurrected. See the bottom of RegScavenger::forward():

  // Commit the changes.
  setUnused(KillRegs);
  setUnused(DeadRegs);
  setUsed(DefRegs);

Same here:

void MachineVerifier::visitMachineInstrAfter(const MachineInstr *MI) {
  BBInfo &MInfo = MBBInfoMap[MI->getParent()];
  set_union(MInfo.regsKilled, regsKilled);
  set_subtract(regsLive, regsKilled); regsKilled.clear();
  set_subtract(regsLive, regsDead);   regsDead.clear();
  set_union(regsLive, regsDefined);   regsDefined.clear();
}

In other words, <def> overrides <def,dead> and <kill>

This instruction:

  %D0<def>, %D1<def> = VLD ..., %Q0<def,dead>

Leaves %Q0 dead while subregs %D0 and D1 are live. You want:

  %D0<def,dead>, %D1<def,dead> = VLD ..., %Q0<def,dead>

To avoid any zombies.

Alternatively, you can use MachineInstr::addRegister{Defined,Dead}(%Q0, TRI, true) which should take care of this, including adding dead flags to the subreg operands.

/jakob