[PowerPC] fix killed/dead flag after reg+reg to reg+imm transformation

[shchenz]

Summary

---
name: testKilled
tracksRegLiveness: true
body: |
  bb.0.entry:
    liveins: $x3, $f1, $x5
    $x3 = ADDI8 $x5, 100
    STD killed $x5, $x5, 100
    STFSX killed $f1, $zero8, killed $x3
    BLR8 implicit $lr8, implicit $rm

llc -mtriple=powerpc64le--linux-gnu -stop-after ppc-pre-emit-peephole t.mir -o - -verify-machineinstrs

# After PowerPC Pre-Emit Peephole
# Machine code for function testKill: NoPHIs, TracksLiveness, NoVRegs

bb.0.entry:
  liveins: $f1, $x3, $x5
  STD killed $x5, $x5, 100
  STFS killed $f1, 100, $x5
  BLR8 implicit $lr8, implicit $rm

# End machine code for function testKill.

*** Bad machine code: Using an undefined physical register ***
- function:    testKill
- basic block: %bb.0 entry (0x100313d8218)
- instruction: STFS killed $f1, 100, $x5
- operand 2:   $x5
LLVM ERROR: Found 1 machine code errors.

PowerPC Peephole will convert ADDI8 + STFSX to STFS. But currently it does not consider about killed/dead flag, so after transformation in the above case $x5 is used after killed.

[jsji]

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

1143 ↗

(On Diff #187523)

This file was changed only by adding or removing whitespace. - Please avoid changing this in this patch.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

2421 ↗

(On Diff #187523)

We not only fixup Kill flag, but also set Dead flag, so maybe we should update the name?

2436 ↗

(On Diff #187523)

(MI<->DefMI)

->

(MI -> DefMI]

2439 ↗

(On Diff #187523)

Why It++; here? Can we add comments to explain?

2441 ↗

(On Diff #187523)

Is it possible that debug instruction will mess up the logic here? Do we need to skip debug instructions?

2444 ↗

(On Diff #187523)

If we don't set IsKillSet for MI in line 2433, is it by design to also allow updating kill/dead for RegNo here? eg: we call this function with a RegNo that is not used in MI, and last use before MI is not killed too?

2459 ↗

(On Diff #187523)

Is it possible that we have multiple same operands with killed flag set?

2487 ↗

(On Diff #187523)

Why ForwardOperandReg instead of ForwardingOperandReg here?

3353 ↗

(On Diff #187523)

Killed -> killed

3410 ↗

(On Diff #187523)

Should we dump the instruction after flag fix?

3476 ↗

(On Diff #187523)

Killed -> killed

llvm/lib/Target/PowerPC/PPCInstrInfo.h

414 ↗

(On Diff #187523)

Can we add some doxygen comments here ? Especially relationship between MI and RegNo.

414 ↗

(On Diff #187523)

Also, why this needs to be "Public" here? Is it intended for reuse somewhere else?

[shchenz]

address @jsji comments

[shchenz]

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

1143 ↗

(On Diff #187523)

The empty namespace is added by mistake in this patch's NFC patch https://reviews.llvm.org/rL354438.
But I am ok to remove this change if your insist.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

2439 ↗

(On Diff #187523)

done.

2441 ↗

(On Diff #187523)

debug instructions should not impact functionality of this function. findRegisterUseOperand/findRegisterDefOperand will return nullptr for debug instructions I think.

But it impacts perf, I have updated.

2444 ↗

(On Diff #187523)

Yes, this is by design. Before we call this function, we should make sure RegNo liveness is killed in MI.

2459 ↗

(On Diff #187523)

Yes, we maybe meet this issue. We should continue to clear not-last-use instruction's killed flag for RegNo.

// Fixup kill/dead flag after transformation.$
// Pattern 1:$
// x = op1 KilledFwdFeederReg, imm$
// n1 = opn1 KilledFwdFeederReg(killed), regn1$
// n2 = opn2 KilledFwdFeederReg(killed), regn2$
// y = op2 0, x$

3410 ↗

(On Diff #187523)

Updated. Should dump after fix.

llvm/lib/Target/PowerPC/PPCInstrInfo.h

414 ↗

(On Diff #187523)

This is by design. FixupKilledDeadFlags is designed to be a general interface for all PostRA transformations which will violate killed/flags semantics. So it is maybe called like TII->FixupKilledDeadFlags().

[shchenz]

Thanks for your comments. @jsji

Ready for new round review.

[jsji]

Thanks for quick update, some more comments.

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

1143 ↗

(On Diff #187523)

Then please use another NFC patch to correct that, don't try to squeeze into this one. Thanks.

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

2439 ↗

(On Diff #188100)

is handled at the beginning -> has been handled above?

2421 ↗

(On Diff #187523)

I don't like fixUpKilledDeadFlags, KilledDead looks confusing... and why fixUp not fixup?

How about fixupIsDeadOrKill ?

2441 ↗

(On Diff #187523)

Thanks for updating,
but I am not sure that`findRegisterUseOperand/findRegisterDefOperand will return nullptr for debug instructions `, any spec say that debug instruction can NOT have Reg operands?

2444 ↗

(On Diff #187523)

Can we assert on this assumption?

2459 ↗

(On Diff #187523)

Is it possible that we have multiple same operands with killed flag set in the same MI?

2459 ↗

(On Diff #187523)

// Fixup kill/dead flag after transformation.$
// Pattern 1:$
// x = op1 KilledFwdFeederReg, imm$
// n1 = opn1 KilledFwdFeederReg(killed), regn1$
// n2 = opn2 KilledFwdFeederReg(killed), regn2$
// y = op2 0, x$

Hmm, should we set killed for n1 MI here？

2459 ↗

(On Diff #187523)

Also, please add this situation into testcase?

llvm/lib/Target/PowerPC/PPCInstrInfo.h

418 ↗

(On Diff #188100)

StartMI -> \p StartMI
EndMI -> \p EndMI
RegNo -> \p RegNo

llvm/test/CodeGen/PowerPC/convert-rr-to-ri-instrs-kill-flag.mir

4 ↗

(On Diff #188100)

Can we please add comments about test point for all the cases here? It is not explicit to me without comparing them carefully.

26 ↗

(On Diff #188100)

This is copied from convert-rr-to-ri-instr-add.mir ? If this is already covered there, maybe we can skip it here?

80 ↗

(On Diff #188100)

This is copied from convert-rr-to-ri-instr-add.mir ? If this is already covered there, maybe we can skip it here?

[shchenz]

address comments

[shchenz]

address @jsji comments.

[jsji]

LGTM, except some minor update. Thanks for fixing!

llvm/lib/Target/PowerPC/PPCInstrInfo.cpp

2466 ↗

(On Diff #188495)

This is almost identical to line 2442 , and it is for a special case handling, can we make it a lamda and just add the call here?

[shchenz]

address comments

llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.h

class PPCInstrInfo : public PPCGenInstrInfo {
   void LoadRegFromStackSlot(MachineFunction &MF, const DebugLoc &DL,
                             unsigned DestReg, int FrameIdx,
                             const TargetRegisterClass *RC,
                             SmallVectorImpl<MachineInstr *> &NewMIs) const;
 
   // If the inst has imm-form and one of its operand is produced by a LI,
   // put the imm into the inst directly and remove the LI if possible.
   bool transformToImmFormFedByLI(MachineInstr &MI, const ImmInstrInfo &III,
-                                 unsigned ConstantOpNo, int64_t Imm) const;
+                                 unsigned ConstantOpNo, MachineInstr &DefMI,
+                                 int64_t Imm) const;
   // If the inst has imm-form and one of its operand is produced by an
   // add-immediate, try to transform it when possible.
   bool transformToImmFormFedByAdd(MachineInstr &MI, const ImmInstrInfo &III,
-                                  unsigned ConstantOpNo,
-                                  MachineInstr &DefMI,
+                                  unsigned ConstantOpNo, MachineInstr &DefMI,
                                   bool KillDefMI) const;
   // Try to find that, if the instruction 'MI' contains any operand that
   // could be forwarded from some inst that feeds it. If yes, return the
   // Def of that operand. And OpNoForForwarding is the operand index in
   // the 'MI' for that 'Def'. If we see another use of this Def between
   // the Def and the MI, SeenIntermediateUse becomes 'true'.
   MachineInstr *getForwardingDefMI(MachineInstr &MI,
                                    unsigned &OpNoForForwarding,
                                    bool &SeenIntermediateUse) const;
 
   // Can the user MI have it's source at index \p OpNoForForwarding
   // forwarded from an add-immediate that feeds it?
   bool isUseMIElgibleForForwarding(MachineInstr &MI, const ImmInstrInfo &III,
                                    unsigned OpNoForForwarding) const;
   bool isDefMIElgibleForForwarding(MachineInstr &DefMI,
                                    const ImmInstrInfo &III,
                                    MachineOperand *&ImmMO,
                                    MachineOperand *&RegMO) const;
   bool isImmElgibleForForwarding(const MachineOperand &ImmMO,
                                  const MachineInstr &DefMI,
                                  const ImmInstrInfo &III,
                                  int64_t &Imm) const;
   bool isRegElgibleForForwarding(const MachineOperand &RegMO,
                                  const MachineInstr &DefMI,
-                                 const MachineInstr &MI,
-                                 bool KillDefMI) const;
+                                 const MachineInstr &MI, bool KillDefMI,
+                                 bool &IsFwdFeederRegKilled) const;
   const unsigned *getStoreOpcodesForSpillArray() const;
   const unsigned *getLoadOpcodesForSpillArray() const;
   virtual void anchor();
 
 protected:
   /// Commutes the operands in the given instruction.
   /// The commutable operands are specified by their indices OpIdx1 and OpIdx2.
   ///
public:
   /// Return true if the output of the instruction is always zero-extended,
   /// i.e. 0 to 31-th bits are all zeros
   bool isZeroExtended(const MachineInstr &MI, const unsigned depth = 0) const {
    return isSignOrZeroExtended(MI, false, depth);
   }
 
   bool convertToImmediateForm(MachineInstr &MI,
                               MachineInstr **KilledDef = nullptr) const;
+
+  /// Fixup killed/dead flag for register \p RegNo between instructions [\p
+  /// StartMI, \p EndMI]. Some PostRA transformations may violate register
+  /// killed/dead flags semantics, this function can be called to fix up. Before
+  /// calling this function,
+  /// 1. Ensure that \p RegNo liveness is killed after instruction \p EndMI.
+  /// 2. Ensure that there is no new definition between (\p StartMI, \p EndMI)
+  ///    and possible definition for \p RegNo is \p StartMI or \p EndMI.
+  /// 3. Ensure that all instructions between [\p StartMI, \p EndMI] are in same
+  ///    basic block.
+  void fixupIsDeadOrKill(MachineInstr &StartMI, MachineInstr &EndMI,
+                         unsigned RegNo) const;
   void replaceInstrWithLI(MachineInstr &MI, const LoadImmediateInfo &LII) const;
   void replaceInstrOperandWithImm(MachineInstr &MI, unsigned OpNo,
                                   int64_t Imm) const;
 
   bool instrHasImmForm(const MachineInstr &MI, ImmInstrInfo &III,
                        bool PostRA) const;
 
   /// getRegNumForOperand - some operands use different numbering schemes

llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp

static const unsigned OpcodesForSpill[2][SOK_LastOpcodeSpill] = {
       {PPC::LWZ, PPC::LD, PPC::LFD, PPC::LFS, PPC::RESTORE_CR,
        PPC::RESTORE_CRBIT, PPC::LVX, PPC::LXV, PPC::DFLOADf64, PPC::DFLOADf32,
        PPC::RESTORE_VRSAVE, PPC::QVLFDX, PPC::QVLFSXs, PPC::QVLFDXb,
        PPC::SPILLTOVSR_LD}};
 
   return OpcodesForSpill[(Subtarget.hasP9Vector()) ? 1 : 0];
 }
 
+void PPCInstrInfo::fixupIsDeadOrKill(MachineInstr &StartMI, MachineInstr &EndMI,
+                                     unsigned RegNo) const {
+  const MachineRegisterInfo &MRI =
+      StartMI.getParent()->getParent()->getRegInfo();
+  if (MRI.isSSA())
+    return;
+
+  // Instructions between [StartMI, EndMI] should be in same basic block.
+  assert((StartMI.getParent() == EndMI.getParent()) &&
+         "Instructions are not in same basic block");
+
+  bool IsKillSet = false;
+
+  auto clearOperandKillInfo = [=] (MachineInstr &MI, unsigned Index) {
+    MachineOperand &MO = MI.getOperand(Index);
+    if (MO.isReg() && MO.isUse() && MO.isKill() &&
+        getRegisterInfo().regsOverlap(MO.getReg(), RegNo))
+      MO.setIsKill(false);
+  };
+
+  // Set killed flag for EndMI.
+  // No need to do anything if EndMI defines RegNo.
+  int UseIndex =
+      EndMI.findRegisterUseOperandIdx(RegNo, false, &getRegisterInfo());
+  if (UseIndex != -1) {
+    EndMI.getOperand(UseIndex).setIsKill(true);
+    IsKillSet = true;
+    // Clear killed flag for other EndMI operands related to RegNo. In some
+    // upexpected cases, killed may be set multiple times for same register
+    // operand in same MI.
+    for (int i = 0, e = EndMI.getNumOperands(); i != e; ++i)
+      if (i != UseIndex)
+        clearOperandKillInfo(EndMI, i);
+  }
+
+  // Walking the inst in reverse order (EndMI -> StartMI].
+  MachineBasicBlock::reverse_iterator It = EndMI;
+  MachineBasicBlock::reverse_iterator E = EndMI.getParent()->rend();
+  // EndMI has been handled above, skip it here.
+  It++;
+  MachineOperand *MO = nullptr;
+  for (; It != E; ++It) {
+    // Skip insturctions which could not be a def/use of RegNo.
+    if (It->isDebugInstr() || It->isPosition())
+      continue;
+
+    // Clear killed flag for all It operands related to RegNo. In some
+    // upexpected cases, killed may be set multiple times for same register
+    // operand in same MI.
+    for (int i = 0, e = It->getNumOperands(); i != e; ++i)
+        clearOperandKillInfo(*It, i);
+
+    // If killed is not set, set killed for its last use or set dead for its def
+    // if no use found.
+    if (!IsKillSet) {
+      if ((MO = It->findRegisterUseOperand(RegNo, false, &getRegisterInfo()))) {
+        // Use found, set it killed.
+        IsKillSet = true;
+        MO->setIsKill(true);
+        continue;
+      } else if ((MO = It->findRegisterDefOperand(RegNo, false, true,
+                                                  &getRegisterInfo()))) {
+        // No use found, set dead for its def.
+        assert(&*It == &StartMI && "No new def between StartMI and EndMI.");
+        MO->setIsDead(true);
+        break;
+      }
+    }
+
+    if ((&*It) == &StartMI)
+      break;
+  }
+  // Ensure RegMo liveness is killed after EndMI.
+  assert((IsKillSet || (MO && MO->isDead())) &&
+         "RegNo should be killed or dead");
+}
+
 // If this instruction has an immediate form and one of its operands is a
 // result of a load-immediate or an add-immediate, convert it to
 // the immediate form if the constant is in range.
 bool PPCInstrInfo::convertToImmediateForm(MachineInstr &MI,
                                           MachineInstr **KilledDef) const {
   MachineFunction *MF = MI.getParent()->getParent();
   MachineRegisterInfo *MRI = &MF->getRegInfo();
   bool PostRA = !MRI->isSSA();
   bool SeenIntermediateUse = true;
   unsigned ForwardingOperand = ~0U;
   MachineInstr *DefMI = getForwardingDefMI(MI, ForwardingOperand,
                                            SeenIntermediateUse);
   if (!DefMI)
     return false;
   assert(ForwardingOperand < MI.getNumOperands() &&
          "The forwarding operand needs to be valid at this point");
-  bool KillFwdDefMI = !SeenIntermediateUse &&
-    MI.getOperand(ForwardingOperand).isKill();
+  bool IsForwardingOperandKilled = MI.getOperand(ForwardingOperand).isKill();
+  bool KillFwdDefMI = !SeenIntermediateUse && IsForwardingOperandKilled;
+  unsigned ForwardingOperandReg = MI.getOperand(ForwardingOperand).getReg();
   if (KilledDef && KillFwdDefMI)
     *KilledDef = DefMI;
 
   ImmInstrInfo III;
   bool HasImmForm = instrHasImmForm(MI, III, PostRA);
   // If this is a reg+reg instruction that has a reg+imm form,
   // and one of the operands is produced by an add-immediate,
   // try to convert it.
-  if (HasImmForm && transformToImmFormFedByAdd(MI, III, ForwardingOperand,
-                                               *DefMI, KillFwdDefMI))
+  if (HasImmForm &&
+      transformToImmFormFedByAdd(MI, III, ForwardingOperand, *DefMI,
+                                 KillFwdDefMI))
     return true;
 
   if ((DefMI->getOpcode() != PPC::LI && DefMI->getOpcode() != PPC::LI8) ||
       !DefMI->getOperand(1).isImm())
     return false;
 
   int64_t Immediate = DefMI->getOperand(1).getImm();
   // Sign-extend to 64-bits.
   int64_t SExtImm = ((uint64_t)Immediate & ~0x7FFFuLL) != 0 ?
     (Immediate | 0xFFFFFFFFFFFF0000) : Immediate;
 
   // If this is a reg+reg instruction that has a reg+imm form,
   // and one of the operands is produced by LI, convert it now.
   if (HasImmForm)
-    return transformToImmFormFedByLI(MI, III, ForwardingOperand, SExtImm);
+    return transformToImmFormFedByLI(MI, III, ForwardingOperand, *DefMI, SExtImm);
 
   bool ReplaceWithLI = false;
   bool Is64BitLI = false;
   int64_t NewImm = 0;
   bool SetCR = false;
   unsigned Opc = MI.getOpcode();
   switch (Opc) {
   default: return false;
 
   // FIXME: Any branches conditional on such a comparison can be made
   // unconditional. At this time, this happens too infrequently to be worth
   // the implementation effort, but if that ever changes, we could convert
   // such a pattern here.
   case PPC::CMPWI:
   case PPC::CMPLWI:
   case PPC::CMPDI:
   case PPC::CMPLDI: {
     // Doing this post-RA would require dataflow analysis to reliably find uses
     // of the CR register set by the compare.
+    // No need to fixup killed/dead flag since this transformation is only valid
+    // before RA.
     if (PostRA)
       return false;
     // If a compare-immediate is fed by an immediate and is itself an input of
     // an ISEL (the most common case) into a COPY of the correct register.
     bool Changed = false;
     unsigned DefReg = MI.getOperand(0).getReg();
     int64_t Comparand = MI.getOperand(2).getImm();
     int64_t SExtComparand = ((uint64_t)Comparand & ~0x7FFFuLL) != 0 ?
if (ReplaceWithLI) {
     LII.Imm = NewImm;
     LII.Is64Bit = Is64BitLI;
     LII.SetCR = SetCR;
     // If we're setting the CR, the original load-immediate must be kept (as an
     // operand to ANDIo/ANDI8o).
     if (KilledDef && SetCR)
       *KilledDef = nullptr;
     replaceInstrWithLI(MI, LII);
+
+    // Fixup killed/dead flag after transformation.
+    // Pattern:
+    // ForwardingOperandReg = LI imm1
+    // y = op2 imm2, ForwardingOperandReg(killed)
+    if (IsForwardingOperandKilled)
+      fixupIsDeadOrKill(*DefMI, MI, ForwardingOperandReg);
+
     LLVM_DEBUG(dbgs() << "With:\n");
     LLVM_DEBUG(MI.dump());
     return true;
   }
   return false;
 }
 
 static bool isVFReg(unsigned Reg) {
bool PPCInstrInfo::isDefMIElgibleForForwarding(MachineInstr &DefMI,
   ImmMO = &DefMI.getOperand(2);
 
   // This DefMI is elgible for forwarding if it is:
   // 1. add inst
   // 2. one of the operands is Imm/CPI/Global.
   return isAnImmediateOperand(*ImmMO);
 }
 
-bool PPCInstrInfo::isRegElgibleForForwarding(const MachineOperand &RegMO,
-                                             const MachineInstr &DefMI,
-                                             const MachineInstr &MI,
-                                             bool KillDefMI
-                                             ) const {
+bool PPCInstrInfo::isRegElgibleForForwarding(
+    const MachineOperand &RegMO, const MachineInstr &DefMI,
+    const MachineInstr &MI, bool KillDefMI,
+    bool &IsFwdFeederRegKilled) const {
   // x = addi y, imm
   // ...
   // z = lfdx 0, x   -> z = lfd imm(y)
   // The Reg "y" can be forwarded to the MI(z) only when there is no DEF
   // of "y" between the DEF of "x" and "z".
   // The query is only valid post RA.
   const MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
   if (MRI.isSSA())
     return false;
 
   unsigned Reg = RegMO.getReg();
 
   // Walking the inst in reverse(MI-->DefMI) to get the last DEF of the Reg.
   MachineBasicBlock::const_reverse_iterator It = MI;
   MachineBasicBlock::const_reverse_iterator E = MI.getParent()->rend();
   It++;
   for (; It != E; ++It) {
     if (It->modifiesRegister(Reg, &getRegisterInfo()) && (&*It) != &DefMI)
       return false;
+    else if (It->killsRegister(Reg, &getRegisterInfo()) && (&*It) != &DefMI)
+      IsFwdFeederRegKilled = true;
     // Made it to DefMI without encountering a clobber.
     if ((&*It) == &DefMI)
       break;
   }
   assert((&*It) == &DefMI && "DefMI is missing");
 
   // If DefMI also defines the register to be forwarded, we can only forward it
   // if DefMI is being erased.
bool PPCInstrInfo::isImmElgibleForForwarding(const MachineOperand &ImmMO,
   // in ImmInstrInfo
   return true;
 }
 
 // If an X-Form instruction is fed by an add-immediate and one of its operands
 // is the literal zero, attempt to forward the source of the add-immediate to
 // the corresponding D-Form instruction with the displacement coming from
 // the immediate being added.
-bool PPCInstrInfo::transformToImmFormFedByAdd(MachineInstr &MI,
-                                              const ImmInstrInfo &III,
-                                              unsigned OpNoForForwarding,
-                                              MachineInstr &DefMI,
-                                              bool KillDefMI) const {
+bool PPCInstrInfo::transformToImmFormFedByAdd(
+    MachineInstr &MI, const ImmInstrInfo &III, unsigned OpNoForForwarding,
+    MachineInstr &DefMI, bool KillDefMI) const {
   //         RegMO ImmMO
   //           |    |
   // x = addi reg, imm  <----- DefMI
   // y = op    0 ,  x   <----- MI
   //                |
   //         OpNoForForwarding
   // Check if the MI meet the requirement described in the III.
   if (!isUseMIElgibleForForwarding(MI, III, OpNoForForwarding))
     return false;
 
   // Check if the DefMI meet the requirement
   // described in the III. If yes, set the ImmMO and RegMO accordingly.
   MachineOperand *ImmMO = nullptr;
   MachineOperand *RegMO = nullptr;
   if (!isDefMIElgibleForForwarding(DefMI, III, ImmMO, RegMO))
     return false;
   assert(ImmMO && RegMO && "Imm and Reg operand must have been set");
 
   // As we get the Imm operand now, we need to check if the ImmMO meet
   // the requirement described in the III. If yes set the Imm.
   int64_t Imm = 0;
   if (!isImmElgibleForForwarding(*ImmMO, DefMI, III, Imm))
     return false;
 
+  bool IsFwdFeederRegKilled = false;
   // Check if the RegMO can be forwarded to MI.
-  if (!isRegElgibleForForwarding(*RegMO, DefMI, MI, KillDefMI))
+  if (!isRegElgibleForForwarding(*RegMO, DefMI, MI, KillDefMI,
+                                 IsFwdFeederRegKilled))
     return false;
 
+  // Get killed info in case fixup needed after transformation.
+  unsigned ForwardKilledOperandReg = ~0U;
+  MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
+  bool PostRA = !MRI.isSSA();
+  if (PostRA && MI.getOperand(OpNoForForwarding).isKill())
+    ForwardKilledOperandReg = MI.getOperand(OpNoForForwarding).getReg();
+
   // We know that, the MI and DefMI both meet the pattern, and
   // the Imm also meet the requirement with the new Imm-form.
   // It is safe to do the transformation now.
   LLVM_DEBUG(dbgs() << "Replacing instruction:\n");
   LLVM_DEBUG(MI.dump());
   LLVM_DEBUG(dbgs() << "Fed by:\n");
   LLVM_DEBUG(DefMI.dump());
 
else {
     // Now add the rest back.
     for (auto &MO : MOps)
       MI.addOperand(MO);
   }
 
   // Update the opcode.
   MI.setDesc(get(III.ImmOpcode));
 
+  // Fix up killed/dead flag after transformation.
+  // Pattern 1:
+  // x = ADD KilledFwdFeederReg, imm
+  // n = opn KilledFwdFeederReg(killed), regn
+  // y = XOP 0, x
+  // Pattern 2:
+  // x = ADD reg(killed), imm
+  // y = XOP 0, x
+  if (IsFwdFeederRegKilled || RegMO->isKill())
+    fixupIsDeadOrKill(DefMI, MI, RegMO->getReg());
+  // Pattern 3:
+  // ForwardKilledOperandReg = ADD reg, imm
+  // y = XOP 0, ForwardKilledOperandReg(killed)
+  if (ForwardKilledOperandReg != ~0U)
+    fixupIsDeadOrKill(DefMI, MI, ForwardKilledOperandReg);
+
   LLVM_DEBUG(dbgs() << "With:\n");
   LLVM_DEBUG(MI.dump());
 
   return true;
 }
 
 bool PPCInstrInfo::transformToImmFormFedByLI(MachineInstr &MI,
                                              const ImmInstrInfo &III,
                                              unsigned ConstantOpNo,
+                                             MachineInstr &DefMI,
                                              int64_t Imm) const {
   MachineRegisterInfo &MRI = MI.getParent()->getParent()->getRegInfo();
   bool PostRA = !MRI.isSSA();
   // Exit early if we can't convert this.
   if ((ConstantOpNo != III.OpNoForForwarding) && !III.IsCommutative)
     return false;
   if (Imm % III.ImmMustBeMultipleOf)
     return false;
if (PostRA && III.ZeroIsSpecialOrig != III.ZeroIsSpecialNew) {
     if ((NewZeroReg == PPC::R0 || NewZeroReg == PPC::X0) &&
         ConstantOpNo != III.ZeroIsSpecialNew)
       return false;
     if ((OrigZeroReg == PPC::R0 || OrigZeroReg == PPC::X0) &&
         ConstantOpNo != PosForOrigZero)
       return false;
   }
 
+  // Get killed info in case fixup needed after transformation.
+  unsigned ForwardKilledOperandReg = ~0U;
+  if (PostRA && MI.getOperand(ConstantOpNo).isKill())
+    ForwardKilledOperandReg = MI.getOperand(ConstantOpNo).getReg();
+
   unsigned Opc = MI.getOpcode();
   bool SpecialShift32 =
     Opc == PPC::SLW || Opc == PPC::SLWo || Opc == PPC::SRW || Opc == PPC::SRWo;
   bool SpecialShift64 =
     Opc == PPC::SLD || Opc == PPC::SLDo || Opc == PPC::SRD || Opc == PPC::SRDo;
   bool SetCR = Opc == PPC::SLWo || Opc == PPC::SRWo ||
     Opc == PPC::SLDo || Opc == PPC::SRDo;
   bool RightShift =
if (III.ZeroIsSpecialNew) {
       if (TargetRegisterInfo::isVirtualRegister(RegToModify)) {
         const TargetRegisterClass *NewRC =
           MRI.getRegClass(RegToModify)->hasSuperClassEq(&PPC::GPRCRegClass) ?
           &PPC::GPRC_and_GPRC_NOR0RegClass : &PPC::G8RC_and_G8RC_NOX0RegClass;
         MRI.setRegClass(RegToModify, NewRC);
       }
     }
   }
+
+  // Fix up killed/dead flag after transformation.
+  // Pattern:
+  // ForwardKilledOperandReg = LI imm
+  // y = XOP reg, ForwardKilledOperandReg(killed)
+  if (ForwardKilledOperandReg != ~0U)
+    fixupIsDeadOrKill(DefMI, MI, ForwardKilledOperandReg);
   return true;
 }
 
 const TargetRegisterClass *
 PPCInstrInfo::updatedRC(const TargetRegisterClass *RC) const {
   if (Subtarget.hasVSX() && RC == &PPC::VRRCRegClass)
     return &PPC::VSRCRegClass;
   return RC;

llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instr-add.mir

 tracksRegLiveness: true
 body: |
   bb.0.entry:
     liveins: $x3, $f1, $x5
     $x3 = ADDI8 $x5, 100
     ; Following instruction $r3 also reads $x3, ADDI8 can not be erased
     ; CHECK: $x3 = ADDI8 $x5, 100, implicit-def $r3
     STW $r3, $x5, 100
-    ; CHECK: STW $r3, $x5, 100
+    ; CHECK: STW killed $r3, $x5, 100
     STFSX killed $f1, $zero8, $x3
     ; CHECK: STFS killed $f1, 100, $x5
     STD $x5, $x5, 100
     BLR8 implicit $lr8, implicit $rm
 ...

llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instrs-kill-flag.mir

+# RUN: llc -mtriple=powerpc64le--linux-gnu -stop-after ppc-pre-emit-peephole %s -o - -verify-machineinstrs | FileCheck %s
+
+---
+# LI + XFORM -> DFORM, no killed/dead flag fixup.
+name: testKillPassUpLI1
+#CHECK : name : testKillPassUpLI1
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = LI8 100
+    STFSX killed $f1, $x3, $x5
+    ; CHECK: STFS killed $f1, 100, $x5
+    STD killed $x3, killed $x5, 100
+    ; CHECK: STD killed $x3, killed $x5, 100
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# LI + XFORM -> DFORM, fixup killed/dead flag for $x3, find no use, set def as
+# dead(LI8 is deleted).
+name : testKillPassUpLI2
+# CHECK: name: testKillPassUpLI2
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = LI8 100
+    ; CHECK-NOT: LI8
+    STFSX killed $f1, killed $x3, killed $x5
+    ; CHECK: STFS killed $f1, 100, killed $x5
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# LI + XFORM -> DFORM, fixup killed/dead flag for $x3, find last use, set last
+# use as killed.
+name: testKillPassUpLI3
+# CHECK: name: testKillPassUpLI3
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = LI8 100
+    STD $x3, $x5, 100
+    ; CHECK: STD killed $x3, $x5, 100
+    STFSX killed $f1, killed $x3, $x5
+    ; CHECK: STFS killed $f1, 100, $x5
+    STD killed $x5, $x5, 100
+    ; CHECK: STD killed $x5, $x5, 100
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# LI + OP -> LI, fixup killed/dead flag for $x3, find last use, set last use as
+# killed.
+name: testKillPassUpLI4
+# CHECK: name: testKillPassUpLI4
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $x5
+    $x3 = LI8 100
+    STD $x3, $x5, 100
+    ; CHECK: STD killed $x3, killed $x5, 100
+    $x5 = ADDI8 killed $x3, 200
+    ; CHECK: $x5 = LI8 300
+    STD $x5, $x5, 100
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# ADD + XFORM -> DFORM, fixup killed/dead flag for $x3, find no use, set def as dead
+# (ADDI8 is deleted).
+name: testKillPassUpADD1
+# CHECK: name: testKillPassUpADD1
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = ADDI8 killed $x5, 100
+    ; CHECK-NOT: ADDI8
+    STFSX killed $f1, $zero8, killed $x3
+    ; CHECK: STFS killed $f1, 100, killed $x5
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# ADD + XFORM -> DFORM, fixup killed/dead flag for $x3, find last use, set last
+# use as killed.
+name: testKillPassUpADD2
+# CHECK: name: testKillPassUpADD2
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = ADDI8 $x5, 100
+    STD $x3, $x5, 100
+    ; CHECK: STD killed $x3, $x5, 100
+    STFSX killed $f1, $zero8, killed $x3
+    ; CHECK: STFS killed $f1, 100, $x5
+    STD killed $x5, $x5, 100
+    ; CHECK: STD killed $x5, $x5, 100
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# ADD + XFORM -> DFORM, fixup killed/dead flag for register $x5, DFORM
+# instruction uses $x5 and no other kill uses, set it as killed in
+# DFORM instruction.
+name: testKillPassDownADD1
+# CHECK: name: testKillPassDownADD1
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = ADDI8 killed $x5, 100
+    ; CHECK: $x3 = ADDI8 $x5, 100
+    STFSX killed $f1, $zero8, $x3
+    ; CHECK: STFS killed $f1, 100, killed $x5
+    STD killed $x3, $x3, 100
+    ; CHECK: STD killed $x3, $x3, 100
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# ADD + XFORM -> DFORM, fixup killed/dead flag for register $x5, DFORM
+# instruction uses $x5 and there is one kill use, set $x5 as killed in
+# DFORM instruction and clear the other kill use killed flag.
+name: testKillPassDownADD2
+# CHECK: name: testKillPassDownADD2
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = ADDI8 $x5, 100
+    STD killed $x5, $x5, 100
+    ; CHECK: STD $x5, $x5, 100
+    STFSX killed $f1, $zero8, killed $x3
+    ; CHECK: STFS killed $f1, 100, killed $x5
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# ADD + XFORM -> DFORM, fixup killed/dead flag for register $x3, DFORM
+# instruction defines $x3, do nothing for killed/dead flag.
+name: testKillPassDownADD3
+# CHECK: name: testKillPassDownADD3
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $x5
+    $x3 = ADDI8 $x5, 100
+    $x3 = LDX $zero8, killed $x3
+    ; CHECK: $x3 = LD 100, $x5
+    STD killed $x5, $x5, 100
+    ; CHECK: STD killed $x5, $x5, 100
+    STD killed $x3, $x3, 200
+    ; CHECK: STD killed $x3, $x3, 200
+    BLR8 implicit $lr8, implicit $rm
+
+...
+---
+# ADD + XFORM -> DFORM, fixup killed/dead flag for both register $x5 and $x3,
+# DFORM instruction uses $x5 and there is one kill use, set $x5 as killed in
+# DFORM instruction and clear the other kill use killed flag. Find last use for
+# $x3, set last use as killed.
+name: testKillPassDownADD4
+# CHECK: name: testKillPassDownADD4
+tracksRegLiveness: true
+body: |
+  bb.0.entry:
+    liveins: $x3, $f1, $x5
+    $x3 = ADDI8 $x5, 100
+    STD killed $x5, $x5, 100
+    ; CHECK: STD $x5, $x5, 100
+    STD $x3, $x3, 200
+    ; CHECK: STD killed $x3, $x3, 200
+    STFSX killed $f1, $zero8, killed $x3
+    ; CHECK: STFS killed $f1, 100, killed $x5
+    BLR8 implicit $lr8, implicit $rm
+
+...