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

[Mikael Holmén via llvm-dev]

Hi,

On 06/28/2017 07:34 AM, Mikael Holmén via llvm-dev wrote:
 > [...]
> On 06/27/2017 11:10 PM, Matthias Braun wrote:
>  >
>  > [...]
>  > I think the actual problem here is that MachineInstr::addRegisterDead 
>  > only works on a single instruction and not on the whole bundle.
> 
> Maybe we should rather make MachineInstr::addRegisterDead mark the 
> individual instruction defs as dead then?
> 

I tried the below in MachineInstr::addRegisterDead rather than changing 
InlineSpiller::spillAroundUses, and this also solves my problem (and all 
tests under test/ still pass), but I suppose it's not actually as simple 
as this given the existance of "internal reads" in bundles? (Also I'm 
not sure if the return value is handled correctly.)

@@ -2186,10 +2186,23 @@ void MachineInstr::clearRegisterKills(unsigned Reg,
  }

  bool MachineInstr::addRegisterDead(unsigned Reg,
                                     const TargetRegisterInfo *RegInfo,
                                     bool AddIfNotFound) {
+  bool RetVal = false;
+
+  if (isBundle()) {
+    // Mark reg dead in all the bundled instructions as well.
+    MachineBasicBlock::instr_iterator MIE = getParent()->instr_end();
+    MachineBasicBlock::instr_iterator MII = getIterator();
+    ++MII;
+    while ((MII != MIE) && MII->isBundledWithPred()) {
+      RetVal = MII->addRegisterDead(Reg, RegInfo, AddIfNotFound) || RetVal;
+      ++MII;
+    }
+  }
+
    bool isPhysReg = TargetRegisterInfo::isPhysicalRegister(Reg);
    bool hasAliases = isPhysReg &&
      MCRegAliasIterator(Reg, RegInfo, false).isValid();
    bool Found = false;
    SmallVector<unsigned,4> DeadOps;
@@ -2225,11 +2238,11 @@ bool MachineInstr::addRegisterDead(unsigned Reg,
    }

    // If not found, this means an alias of one of the operands is dead. 
Add a
    // new implicit operand if required.
    if (Found || !AddIfNotFound)
-    return Found;
+    return RetVal || Found;

    addOperand(MachineOperand::CreateReg(Reg,
                                         true  /*IsDef*/,
                                         true  /*IsImp*/,
                                         false /*IsKill*/,

Regards,
Mikael