[IfConversion] More simple, correct dead/kill liveness handling

[JesperAntonsson]

Summary

The MachineVerifier reports of issues with liveness after IfConversion in randomized testing for our out-of-tree target. This patch is replacing the complex handling of dead/kill-flags with a simpler, reusable subpass that recomputes a BB's dead/kill flags at the end of each transformation.

The recomputation may be slightly different and a few Hexagon MIR test were adjusted to reflect this.

[kparzysz]

lib/CodeGen/MachineBasicBlock.cpp

456 ↗

(On Diff #114310)

Are you seeing any cases where a "dead" flag would be removed? This is somewhat concerning. Whether a use is a kill or not depends on the ordering relative to other uses, which can change. Whether a def is dead or not is more closely related to the program's semantics: once a def is dead, new uses should not really appear.

[kparzysz]

This seems like a reasonable way of handling it, but I'd like Matthias and Kyle to comment on this as well.

lib/CodeGen/MachineBasicBlock.cpp

456 ↗

(On Diff #114310)

I thought about it for a bit---I guess that a dead def from a side A of a diamond can appear to "reach" a use from side B of the diamond after predication, if A precedes B in the predicated code.

[iteratee]

This looks fine overall to me.

lib/CodeGen/MachineBasicBlock.cpp

449 ↗

(On Diff #114310)

Would it be worth it to have an early exit from the loop when IsLive is true?

[MatzeB]

This is a very good idea!

lib/CodeGen/MachineBasicBlock.cpp

425 ↗

(On Diff #114310)

• I'd put this function in LivePhysRegs.h maybe next to computeLiveIns() and not make it a member of MachineBasicBlock.
• How about changing this to also add kill/dead flags (see below) and calling it recomputeLivenessFlags()?
• It may or may not be a good idea to break this up into a more flexible function: recomputeLivenessFlagsInRange(LivePhysRegs &LiveRegs, iterator_range<MachineBasicBlock::reverse_iterator> Range); so that you can apply it to arbitrary ranges inside a basic block given a LivePhysRegs with liveness behind the range. You could still have a convenience function that processes a whole block and uses this internally.

426 ↗

(On Diff #114310)

Try using references for things that cannot be nullptr.

429–434 ↗

(On Diff #114310)

Better use LiveRegs.addLiveOutsNoPristines() instead of doing the loop yourself. It will get some corner cases like return blocks right.

433 ↗

(On Diff #114310)

(writing MachineBasicBlock * instead of auto is friendlier to readers.)

437–438 ↗

(On Diff #114310)

Are DebugValues allowed to be inside instruction bundles? I'd probably go for if (MO->isDebug()) in the following loop instead of using MI.isDebugValue() here.

446 ↗

(On Diff #114310)

Maybe add an assert(TargetRegisterInfo::isPhysReg(Reg)) here to make things explicit?

447–449 ↗

(On Diff #114310)

This is expensive, and not necessary I think:

454–457 ↗

(On Diff #114310)

How about:

if (MO->isDef())
  MO->setIsDead(!LiveRegs.available(MRI, Reg));
if (MO->readsReg())
  MO->setIsKill(!LiveRegs.available(MRI, Reg));

[JesperAntonsson]

Thanks for the reviews, everybody! Here I'm submitting an updated patch based on Matthias' suggestions.

Hope I covered everything, shout if not. (The range-based method extraction might be a good idea, but I'm leaving that to anyone that has use for it.)

As far as I have understood, DebugValues are not allowed in bundles, but I moved the check to the operand anyway. But the question made me think about bundling... Do I need to go inside bundles to adjust as well? I'm only doing bundle heads here.

Since I'm now adding flags as well, I had to update a few tests more.

[kparzysz]

lib/CodeGen/LivePhysRegs.cpp

294 ↗

(On Diff #114596)

It doesn't look like this code going to add a kill flag to the use of r0 in the add.

r0 = add r0, r1
... = r0

[JesperAntonsson]

lib/CodeGen/LivePhysRegs.cpp

294 ↗

(On Diff #114596)

True, good catch. It would also remove pre-existing kill flags.

Seems I should first set dead flags like I do now, then do a partial stepBackward over (non-early-clobber?) defs only, before I loop over use-MOs to set kill flags, and then do the remaining stepBackward? Would that be a good approach?

[MatzeB]

lib/CodeGen/LivePhysRegs.cpp

294 ↗

(On Diff #114596)

Should be easy enough to factor out the def/use loops in stepBackward() into two functions that can be used individually.

[JesperAntonsson]

This is a new patch that will handle the issue Krzysztof pointed out. Some more Hexagon tests affected. Makes me wonder: Is it ok to say that a predicated instruction's use of a register is killed if it is defined by the same instruction?

[JesperAntonsson]

This new implementation doesn't run cleanly with our extended test suite. I have to look a bit more into it.

[kparzysz]

I just started a test run on a Hexagon suite, but I wonder if the anti-dependence breaker could be a problem. This is regarding the question of whether the implicit uses on predicated instructions should be killed or not. BTW, the same argument applies to tied uses. Even though the live ranges seem to end/start at that instruction, the register cannot be renamed in only one of them. This differentiates those instructions from a case of unrelated use/def coincidentally using the same register (as was in the example: r0 = add r0, r1).

[kparzysz]

To add to my previous comment---there is a plan to eliminate kill flags in the future (as I was told), so passes the rely on those will eventually need to be modified. If it's the presence of kill flags on predicated instructions (and tied uses) that is causing problems, maybe it would be a good thing to address that in the passes that are exploiting that to generate wrong code. It would imply more work, so I guess the answer depends on how much more work it would be.

[kparzysz]

Good news everyone... The Hexagon test suite passed.

[MatzeB]

In D37611#869543, @kparzysz wrote:

I just started a test run on a Hexagon suite, but I wonder if the anti-dependence breaker could be a problem. This is regarding the question of whether the implicit uses on predicated instructions should be killed or not. BTW, the same argument applies to tied uses. Even though the live ranges seem to end/start at that instruction, the register cannot be renamed in only one of them. This differentiates those instructions from a case of unrelated use/def coincidentally using the same register (as was in the example: r0 = add r0, r1).

For instruction that def/use the same vreg (tied or not) it shouldn't matter whether we add a kill flag. That is either way it is obvious that the def will overwrite the register. But given that it doesn't matter we can just as well do it.
While it feels slightly off for tied operands I don't think it matters there either as the no-partially-renaming semantics stems from the fact that the tied bit is set not from present or missing kill flags.

[MatzeB]

In D37611#869555, @kparzysz wrote:

To add to my previous comment---there is a plan to eliminate kill flags in the future (as I was told), so passes the rely on those will eventually need to be modified. If it's the presence of kill flags on predicated instructions (and tied uses) that is causing problems, maybe it would be a good thing to address that in the passes that are exploiting that to generate wrong code. It would imply more work, so I guess the answer depends on how much more work it would be.

Yes we are in the process of eliminating kill flags. Ideally you should be able to remove all kill flags from a machine function and still generate exactly the same code. I think we are already there for aarch64 and x86 (though I didn't do in-depth validation yet). However I think we should work towards that goal delibarately: When writing new code make sure it does not read/depend on kill flags, at the same time when changing existing code I'd avoid removing kill flags to avoid unexpected performance swings on targets that still read kill flags.

[MatzeB]

LGTM with nitpicks.

include/llvm/CodeGen/LivePhysRegs.h

111–113 ↗

(On Diff #115004)

Maybe call this removeDefs?

115–116 ↗

(On Diff #115004)

Call this addUses?

[JesperAntonsson]

I found and fixed the reason our randomized tests didn't run cleanly. It was essentially that stepBackwards removed defined regs from the set even if the instruction was predicated. That meant a dominating def could be marked dead, which is wrong. The reason I found this is that I use the new recompute function also in a later stage in our out-of-tree backend, and that went bad when I started to add faulty deads. If I understand it correctly, this is a bug in the current implementation of LivePhysRegs.

I fixed this by adding an early return to removeDefs() if the instruction is predicated. NB this does not recognize that a sequence of predicated instructions may combine to define a reg with certainty. Thus we might miss an opportunity to determine a reg isn't live just before such a sequence and even remove such information if it was there beforehand. I think this has low impact, though.

[kparzysz]

In D37611#870742, @JesperAntonsson wrote:

I found and fixed the reason our randomized tests didn't run cleanly. It was essentially that stepBackwards removed defined regs from the set even if the instruction was predicated.

It's supposed to do that. This is exactly the reason why we add implicit uses to predicated instructions if there is a live def reaching it.

[JesperAntonsson]

Ah, thanks. Then the fault is in our backend, that we somehow miss to add that imp-use in some circumstances. My previous patch is probably better, then. I'll reupload that with Matthias suggested name changes. :-)

[JesperAntonsson]

Reverted the changes in the previous patch, except for the name improvements. I guess we can land this, then. I don't have the privileges yet, so I'm grateful for help with that. :-)

[MatzeB]

In D37611#870745, @kparzysz wrote:

In D37611#870742, @JesperAntonsson wrote:

I found and fixed the reason our randomized tests didn't run cleanly. It was essentially that stepBackwards removed defined regs from the set even if the instruction was predicated.

It's supposed to do that. This is exactly the reason why we add implicit uses to predicated instructions if there is a live def reaching it.

Indeed, generic codegen in llvm has no notion of predication. That's why we do the ifconversion pass really late and then "fake" liveness with those extra operands to make sure everything that needs to be appears live even when the code has no notion of predication (conservatively correct). We should not need any tweaks for predication outside the ifconversion pass.

llvm/trunk/include/llvm/CodeGen/LivePhysRegs.h

public:
   /// addReg() always adds all sub-registers to the set as well.
   /// Note: Returns false if just some sub registers are live, use available()
   /// when searching a free register.
   bool contains(unsigned Reg) const { return LiveRegs.count(Reg); }
 
   /// Returns true if register \p Reg and no aliasing register is in the set.
   bool available(const MachineRegisterInfo &MRI, unsigned Reg) const;
 
+  /// Remove defined registers and regmask kills from the set.
+  void removeDefs(const MachineInstr &MI);
+
+  /// Add uses to the set.
+  void addUses(const MachineInstr &MI);
+
   /// Simulates liveness when stepping backwards over an instruction(bundle).
   /// Remove Defs, add uses. This is the recommended way of calculating
   /// liveness.
   void stepBackward(const MachineInstr &MI);
 
   /// Simulates liveness when stepping forward over an instruction(bundle).
   /// Remove killed-uses, add defs. This is the not recommended way, because it
   /// depends on accurate kill flags. If possible use stepBackward() instead of
inline raw_ostream &operator<<(raw_ostream &OS, const LivePhysRegs& LR) {
   return OS;
 }
 
 /// \brief Computes registers live-in to \p MBB assuming all of its successors
 /// live-in lists are up-to-date. Puts the result into the given LivePhysReg
 /// instance \p LiveRegs.
 void computeLiveIns(LivePhysRegs &LiveRegs, const MachineBasicBlock &MBB);
 
+/// Recomputes dead and kill flags in \p MBB.
+void recomputeLivenessFlags(MachineBasicBlock &MBB);
+
 /// Adds registers contained in \p LiveRegs to the block live-in list of \p MBB.
 /// Does not add reserved registers.
 void addLiveIns(MachineBasicBlock &MBB, const LivePhysRegs &LiveRegs);
 
 /// Convenience function combining computeLiveIns() and addLiveIns().
 void computeAndAddLiveIns(LivePhysRegs &LiveRegs,
                           MachineBasicBlock &MBB);
 
 } // end namespace llvm
 
 #endif // LLVM_CODEGEN_LIVEPHYSREGS_H

llvm/trunk/lib/CodeGen/IfConversion.cpp

class IfConverter : public MachineFunctionPass {
 
     const TargetLoweringBase *TLI;
     const TargetInstrInfo *TII;
     const TargetRegisterInfo *TRI;
     const MachineBranchProbabilityInfo *MBPI;
     MachineRegisterInfo *MRI;
 
     LivePhysRegs Redefs;
-    LivePhysRegs DontKill;
 
     bool PreRegAlloc;
     bool MadeChange;
     int FnNum;
     std::function<bool(const MachineFunction &)> PredicateFtor;
 
   public:
     static char ID;
while (!Tokens.empty()) {
                                       Token->TClobbersPred,
                                       Token->FClobbersPred);
         DEBUG(dbgs() << (RetVal ? "succeeded!" : "failed!") << "\n");
         if (RetVal) ++NumForkedDiamonds;
         break;
       }
       }
 
+      if (RetVal && MRI->tracksLiveness())
+        recomputeLivenessFlags(*BBI.BB);
+
       Change |= RetVal;
 
       NumIfCvts = NumSimple + NumSimpleFalse + NumTriangle + NumTriangleRev +
         NumTriangleFalse + NumTriangleFRev + NumDiamonds;
       if (IfCvtLimit != -1 && (int)NumIfCvts >= IfCvtLimit)
         break;
     }
 
if (Op.isRegMask()) {
       // We also need to add an implicit def of this register for the later
       // use to read from.
       // For the register allocator to have allocated a register clobbered
       // by the call which is used later, it must be the case that
       // the call doesn't return.
       MIB.addReg(Reg, RegState::Implicit | RegState::Define);
       continue;
     }
-    assert(Op.isReg() && "Register operand required");
-    if (Op.isDead()) {
-      // If we found a dead def, but it needs to be live, then remove the dead
-      // flag.
-      if (Redefs.contains(Op.getReg()))
-        Op.setIsDead(false);
-    }
     if (LiveBeforeMI.count(Reg))
       MIB.addReg(Reg, RegState::Implicit);
     else {
       bool HasLiveSubReg = false;
       for (MCSubRegIterator S(Reg, TRI); S.isValid(); ++S) {
         if (!LiveBeforeMI.count(*S))
           continue;
         HasLiveSubReg = true;
         break;
       }
       if (HasLiveSubReg)
         MIB.addReg(Reg, RegState::Implicit);
     }
   }
 }
 
-/// Remove kill flags from operands with a registers in the \p DontKill set.
-static void RemoveKills(MachineInstr &MI, const LivePhysRegs &DontKill) {
-  for (MIBundleOperands O(MI); O.isValid(); ++O) {
-    if (!O->isReg() || !O->isKill())
-      continue;
-    if (DontKill.contains(O->getReg()))
-      O->setIsKill(false);
-  }
-}
-
-/// Walks a range of machine instructions and removes kill flags for registers
-/// in the \p DontKill set.
-static void RemoveKills(MachineBasicBlock::iterator I,
-                        MachineBasicBlock::iterator E,
-                        const LivePhysRegs &DontKill,
-                        const MCRegisterInfo &MCRI) {
-  for (MachineInstr &MI : make_range(I, E))
-    RemoveKills(MI, DontKill);
-}
-
 /// If convert a simple (split, no rejoin) sub-CFG.
 bool IfConverter::IfConvertSimple(BBInfo &BBI, IfcvtKind Kind) {
   BBInfo &TrueBBI  = BBAnalysis[BBI.TrueBB->getNumber()];
   BBInfo &FalseBBI = BBAnalysis[BBI.FalseBB->getNumber()];
   BBInfo *CvtBBI = &TrueBBI;
   BBInfo *NextBBI = &FalseBBI;
 
   SmallVector<MachineOperand, 4> Cond(BBI.BrCond.begin(), BBI.BrCond.end());
if (CvtMBB.hasAddressTaken())
     // Conservatively abort if-conversion if BB's address is taken.
     return false;
 
   if (Kind == ICSimpleFalse)
     if (TII->reverseBranchCondition(Cond))
       llvm_unreachable("Unable to reverse branch condition!");
 
   Redefs.init(*TRI);
-  DontKill.init(*TRI);
 
   if (MRI->tracksLiveness()) {
     // Initialize liveins to the first BB. These are potentiall redefined by
     // predicated instructions.
     Redefs.addLiveIns(CvtMBB);
     Redefs.addLiveIns(NextMBB);
-    // Compute a set of registers which must not be killed by instructions in
-    // BB1: This is everything live-in to BB2.
-    DontKill.addLiveIns(NextMBB);
   }
 
   // Remove the branches from the entry so we can add the contents of the true
   // block to it.
   BBI.NonPredSize -= TII->removeBranch(*BBI.BB);
 
   if (CvtMBB.pred_size() > 1) {
     // Copy instructions in the true block, predicate them, and add them to
     // the entry block.
     CopyAndPredicateBlock(BBI, *CvtBBI, Cond);
 
     // Keep the CFG updated.
     BBI.BB->removeSuccessor(&CvtMBB, true);
   } else {
     // Predicate the instructions in the true block.
-    RemoveKills(CvtMBB.begin(), CvtMBB.end(), DontKill, *TRI);
     PredicateBlock(*CvtBBI, CvtMBB.end(), Cond);
 
     // Merge converted block into entry block. The BB to Cvt edge is removed
     // by MergeBlocks.
     MergeBlocks(BBI, *CvtBBI);
   }
 
   bool IterIfcvt = true;
bool IfConverter::IfConvertTriangle(BBInfo &BBI, IfcvtKind Kind) {
   // Initialize liveins to the first BB. These are potentially redefined by
   // predicated instructions.
   Redefs.init(*TRI);
   if (MRI->tracksLiveness()) {
     Redefs.addLiveIns(CvtMBB);
     Redefs.addLiveIns(NextMBB);
   }
 
-  DontKill.clear();
-
   bool HasEarlyExit = CvtBBI->FalseBB != nullptr;
   BranchProbability CvtNext, CvtFalse, BBNext, BBCvt;
 
   if (HasEarlyExit) {
     // Get probabilities before modifying CvtMBB and BBI.BB.
     CvtNext = MBPI->getEdgeProbability(&CvtMBB, &NextMBB);
     CvtFalse = MBPI->getEdgeProbability(&CvtMBB, CvtBBI->FalseBB);
     BBNext = MBPI->getEdgeProbability(BBI.BB, &NextMBB);
if (!DI1->isDebugValue())
       ++i;
   }
   while (NumDups1 != 0) {
     ++DI2;
     if (!DI2->isDebugValue())
       --NumDups1;
   }
 
-  // Compute a set of registers which must not be killed by instructions in BB1:
-  // This is everything used+live in BB2 after the duplicated instructions. We
-  // can compute this set by simulating liveness backwards from the end of BB2.
-  DontKill.init(*TRI);
   if (MRI->tracksLiveness()) {
-    for (const MachineInstr &MI : make_range(MBB2.rbegin(), ++DI2.getReverse()))
-      DontKill.stepBackward(MI);
-
     for (const MachineInstr &MI : make_range(MBB1.begin(), DI1)) {
       SmallVector<std::pair<unsigned, const MachineOperand*>, 4> Dummy;
       Redefs.stepForward(MI, Dummy);
     }
   }
-  // Kill flags in the true block for registers living into the false block
-  // must be removed. This should be done before extracting the common
-  // instructions from the beginning of the MBB1, since these instructions
-  // can actually differ between MBB1 and MBB2 in terms of <kill> flags.
-  RemoveKills(MBB1.begin(), MBB1.end(), DontKill, *TRI);
-
   BBI.BB->splice(BBI.BB->end(), &MBB1, MBB1.begin(), DI1);
   MBB2.erase(MBB2.begin(), DI2);
 
   // The branches have been checked to match, so it is safe to remove the branch
   // in BB1 and rely on the copy in BB2
 #ifndef NDEBUG
   // Unanalyzable branches must match exactly. Check that now.
   if (!BBI1->IsBrAnalyzable)

 #endif
         llvm_unreachable(nullptr);
       }
     }
 
     // If the predicated instruction now redefines a register as the result of
     // if-conversion, add an implicit kill.
     UpdatePredRedefs(*MI, Redefs);
-
-    // Some kill flags may not be correct anymore.
-    if (!DontKill.empty())
-      RemoveKills(*MI, DontKill);
   }
 
   if (!IgnoreBr) {
     std::vector<MachineBasicBlock *> Succs(FromMBB.succ_begin(),
                                            FromMBB.succ_end());
     MachineBasicBlock *NBB = getNextBlock(FromMBB);
     MachineBasicBlock *FallThrough = FromBBI.HasFallThrough ? NBB : nullptr;
 

llvm/trunk/lib/CodeGen/LivePhysRegs.cpp

if (MO.clobbersPhysReg(*LRI)) {
       if (Clobbers)
         Clobbers->push_back(std::make_pair(*LRI, &MO));
       LRI = LiveRegs.erase(LRI);
     } else
       ++LRI;
   }
 }
 
-/// Simulates liveness when stepping backwards over an instruction(bundle):
-/// Remove Defs, add uses. This is the recommended way of calculating liveness.
-void LivePhysRegs::stepBackward(const MachineInstr &MI) {
-  // Remove defined registers and regmask kills from the set.
+/// Remove defined registers and regmask kills from the set.
+void LivePhysRegs::removeDefs(const MachineInstr &MI) {
   for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
     if (O->isReg()) {
       if (!O->isDef())
         continue;
       unsigned Reg = O->getReg();
       if (!TargetRegisterInfo::isPhysicalRegister(Reg))
         continue;
       removeReg(Reg);
     } else if (O->isRegMask())
       removeRegsInMask(*O);
   }
+}
 
-  // Add uses to the set.
+/// Add uses to the set.
+void LivePhysRegs::addUses(const MachineInstr &MI) {
   for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
     if (!O->isReg() || !O->readsReg())
       continue;
     unsigned Reg = O->getReg();
     if (!TargetRegisterInfo::isPhysicalRegister(Reg))
       continue;
     addReg(Reg);
   }
 }
 
+/// Simulates liveness when stepping backwards over an instruction(bundle):
+/// Remove Defs, add uses. This is the recommended way of calculating liveness.
+void LivePhysRegs::stepBackward(const MachineInstr &MI) {
+  // Remove defined registers and regmask kills from the set.
+  removeDefs(MI);
+
+  // Add uses to the set.
+  addUses(MI);
+}
+
 /// Simulates liveness when stepping forward over an instruction(bundle): Remove
 /// killed-uses, add defs. This is the not recommended way, because it depends
 /// on accurate kill flags. If possible use stepBackward() instead of this
 /// function.
 void LivePhysRegs::stepForward(const MachineInstr &MI,
         SmallVectorImpl<std::pair<unsigned, const MachineOperand*>> &Clobbers) {
   // Remove killed registers from the set.
   for (ConstMIBundleOperands O(MI); O.isValid(); ++O) {
for (MCSuperRegIterator SReg(Reg, &TRI); SReg.isValid(); ++SReg) {
       }
     }
     if (ContainsSuperReg)
       continue;
     MBB.addLiveIn(Reg);
   }
 }
 
+void llvm::recomputeLivenessFlags(MachineBasicBlock &MBB) {
+  const MachineFunction &MF = *MBB.getParent();
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
+  const TargetRegisterInfo &TRI = *MRI.getTargetRegisterInfo();
+
+  // We walk through the block backwards and start with the live outs.
+  LivePhysRegs LiveRegs;
+  LiveRegs.init(TRI);
+  LiveRegs.addLiveOutsNoPristines(MBB);
+
+  for (MachineInstr &MI : make_range(MBB.rbegin(), MBB.rend())) {
+    // Recompute dead flags.
+    for (MIBundleOperands MO(MI); MO.isValid(); ++MO) {
+      if (!MO->isReg() || !MO->isDef() || MO->isDebug())
+        continue;
+
+      unsigned Reg = MO->getReg();
+      if (Reg == 0)
+        continue;
+      assert(TargetRegisterInfo::isPhysicalRegister(Reg));
+
+      bool IsNotLive = LiveRegs.available(MRI, Reg);
+      MO->setIsDead(IsNotLive);
+    }
+
+    // Step backward over defs.
+    LiveRegs.removeDefs(MI);
+
+    // Recompute kill flags.
+    for (MIBundleOperands MO(MI); MO.isValid(); ++MO) {
+      if (!MO->isReg() || !MO->readsReg() || MO->isDebug())
+        continue;
+
+      unsigned Reg = MO->getReg();
+      if (Reg == 0)
+        continue;
+      assert(TargetRegisterInfo::isPhysicalRegister(Reg));
+
+      bool IsNotLive = LiveRegs.available(MRI, Reg);
+      MO->setIsKill(IsNotLive);
+    }
+
+    // Complete the stepbackward.
+    LiveRegs.addUses(MI);
+  }
+}
+
 void llvm::computeAndAddLiveIns(LivePhysRegs &LiveRegs,
                                 MachineBasicBlock &MBB) {
   computeLiveIns(LiveRegs, MBB);
   addLiveIns(MBB, LiveRegs);
 }

llvm/trunk/test/CodeGen/Hexagon/branch-folder-hoist-kills.mir

 # then created code, where the first predicated instruction has incorrect
 # implicit use of r0:
 #
 # BB#0:
 #     Live Ins: %R0
 #         %R1<def> = A2_sxth %R0<kill>               ; hoisted, kills r0
 #         A2_nop %P0<imp-def>
 #         %R0<def> = C2_cmoveit %P0, 2, %R0<imp-use> ; predicated A2_tfrsi
-#         %R0<def> = C2_cmoveif %P0, 1, %R0<imp-use> ; predicated A2_tfrsi
+#         %R0<def> = C2_cmoveif killed %P0, 1, %R0<imp-use> ; predicated A2_tfrsi
 #         %R0<def> = A2_add %R0<kill>, %R1<kill>
 #         J2_jumpr %R31, %PC<imp-def,dead>
 #
 
 # CHECK: %r1 = A2_sxth killed %r0
 # CHECK: %r0 = C2_cmoveit %p0, 2
 # CHECK-NOT: implicit-def %r0
-# CHECK: %r0 = C2_cmoveif %p0, 1, implicit %r0
+# CHECK: %r0 = C2_cmoveif killed %p0, 1, implicit killed %r0
 
 ---
 name: fred
 tracksRegLiveness: true
 
 body: |
   bb.0:
     liveins: %r0

llvm/trunk/test/CodeGen/Hexagon/ifcvt-impuse-livein.mir

liveins: %r2
         J2_jump %bb.3, implicit-def %pc
   bb.2:
     successors: %bb.3
     liveins: %r0
     ; Even though r2 was not live on entry to this block, it was live across
     ; block bb.1 in the original diamond. After if-conversion, the diamond
     ; became a single block, and so r2 is now live on entry to the instructions
     ; originating from bb.2.
-    ; CHECK: %r2 = C2_cmoveit %p1, 1, implicit %r2
+    ; CHECK: %r2 = C2_cmoveit %p1, 1, implicit killed %r2
         %r2 = A2_tfrsi 1
   bb.3:
     liveins: %r0, %r2
         %r0 = A2_add %r0, %r2
         J2_jumpr %r31, implicit-def %pc
 ...
 

llvm/trunk/test/CodeGen/Hexagon/ifcvt-live-subreg.mir

 # RUN: llc -march=hexagon -run-pass if-converter -o - %s | FileCheck %s
 # Check that an implicit use is generated for a predicated instruction
 # when a subregister of the redefined register is live.
 
 # CHECK-LABEL: name: foo
 
 # Verify the predicated block:
 # CHECK-LABEL: bb.0:
 # CHECK: liveins: %r0, %r1, %p0, %d8
 # CHECK: %d8 = A2_combinew killed %r0, killed %r1
-# CHECK: %d8 = L2_ploadrdf_io %p0, %r29, 0, implicit %d8
-# CHECK: J2_jumprf %p0, killed %r31, implicit-def %pc, implicit-def %pc, implicit killed %d8
+# CHECK: %d8 = L2_ploadrdf_io %p0, %r29, 0, implicit killed %d8
+# CHECK: J2_jumprf killed %p0, %r31, implicit-def %pc, implicit-def %pc, implicit %d8
 
 --- |
   define void @foo() {
     ret void
   }
 ...
 
 

 body:             |
   bb.0:
     successors: %bb.1, %bb.2
     liveins: %r0, %r1, %p0, %d8
     %d8 = A2_combinew killed %r0, killed %r1
     J2_jumpf killed %p0, %bb.2, implicit-def %pc
 
   bb.1:
-    liveins: %d0, %r17
+    liveins: %r17
     %r0 = A2_tfrsi 0
     %r1 = A2_tfrsi 0
     A2_nop ; non-predicable
     J2_jumpr killed %r31, implicit-def dead %pc, implicit killed %d0
 
   bb.2:
     ; Predicate this block.
     %d8 = L2_loadrd_io %r29, 0
     J2_jumpr killed %r31, implicit-def dead %pc, implicit killed %d8
 
 ...
 

llvm/trunk/test/CodeGen/Hexagon/livephysregs-add-pristines.mir

 # RUN: llc -march=hexagon -run-pass if-converter -o - %s -verify-machineinstrs | FileCheck %s
 
 # The register r23 is live on the path bb.0->bb.2->bb.3. Make sure we add
 # an implicit use of r23 to the predicated redefinition:
-# CHECK: %r23 = A2_tfrt %p0, killed %r1, implicit %r23
+# CHECK: %r23 = A2_tfrt killed %p0, killed %r1, implicit killed %r23
 
 # LivePhysRegs::addPristines could accidentally remove a callee-saved
 # register, if it determined that it wasn't pristine. Doing that caused
 # r23 in this testcase to be dropped from the Redefs set, and subsequently
 # the necessary implicit use was not added for it.
 
 ---
 name: foo