[llvm] [MachineLateInstrsCleanup] Handle multiple kills for a preceding definition. (PR #119132)

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 10 07:22:44 PST 2024


https://github.com/JonPsson1 updated https://github.com/llvm/llvm-project/pull/119132

>From 138859d620f7f32d992ae1e02aaa640f97d968ae Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Sat, 7 Dec 2024 16:55:46 -0500
Subject: [PATCH 1/3] Handle multiple kills for a preceding definition.

---
 llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp | 42 ++++++++-------
 .../SystemZ/machine-latecleanup-kills.mir     | 52 +++++++++++++++++++
 2 files changed, 76 insertions(+), 18 deletions(-)
 create mode 100644 llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir

diff --git a/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp b/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
index 6399e8a9523685e..5fefa2d8fbbab90 100644
--- a/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
+++ b/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
@@ -47,9 +47,9 @@ class MachineLateInstrsCleanup : public MachineFunctionPass {
       return MI && MI->isIdenticalTo(*ArgMI);
     }
   };
-
+  typedef SmallDenseMap<Register, TinyPtrVector<MachineInstr *>> Reg2MIVecMap;
   std::vector<Reg2MIMap> RegDefs;
-  std::vector<Reg2MIMap> RegKills;
+  std::vector<Reg2MIVecMap> RegKills;
 
   // Walk through the instructions in MBB and remove any redundant
   // instructions.
@@ -57,7 +57,7 @@ class MachineLateInstrsCleanup : public MachineFunctionPass {
 
   void removeRedundantDef(MachineInstr *MI);
   void clearKillsForDef(Register Reg, MachineBasicBlock *MBB,
-                        BitVector &VisitedPreds);
+                        BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
 
 public:
   static char ID; // Pass identification, replacement for typeid
@@ -113,19 +113,25 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
 // definition.
 void MachineLateInstrsCleanup::clearKillsForDef(Register Reg,
                                                 MachineBasicBlock *MBB,
-                                                BitVector &VisitedPreds) {
+                                                BitVector &VisitedPreds,
+                                                MachineInstr *ToRemoveMI) {
   VisitedPreds.set(MBB->getNumber());
 
-  // Kill flag in MBB
-  if (MachineInstr *KillMI = RegKills[MBB->getNumber()].lookup(Reg)) {
-    KillMI->clearRegisterKills(Reg, TRI);
-    return;
-  }
+  // Clear kill flag(s) in MBB, that have been seen after the preceding
+  // definition. If Reg or one of its subregs was killed, it would actually
+  // be ok to stop after removing that (and any other) kill-flag, but it
+  // doesn't seem noticeably faster while it would be a bit more complicated.
+  Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
+  if (MBBKills.contains(Reg))
+    for (auto *KillMI : MBBKills[Reg])
+      KillMI->clearRegisterKills(Reg, TRI);
 
-  // Def in MBB (missing kill flag)
-  if (MachineInstr *DefMI = RegDefs[MBB->getNumber()].lookup(Reg))
-    if (DefMI->getParent() == MBB)
-      return;
+  // Definition in current MBB: done.
+  Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
+  MachineInstr *DefMI = MBBDefs[Reg];
+  assert(DefMI->isIdenticalTo(*ToRemoveMI) && "Previous def not identical?");
+  if (DefMI->getParent() == MBB)
+    return;
 
   // If an earlier def is not in MBB, continue in predecessors.
   if (!MBB->isLiveIn(Reg))
@@ -133,13 +139,13 @@ void MachineLateInstrsCleanup::clearKillsForDef(Register Reg,
   assert(!MBB->pred_empty() && "Predecessor def not found!");
   for (MachineBasicBlock *Pred : MBB->predecessors())
     if (!VisitedPreds.test(Pred->getNumber()))
-      clearKillsForDef(Reg, Pred, VisitedPreds);
+      clearKillsForDef(Reg, Pred, VisitedPreds, ToRemoveMI);
 }
 
 void MachineLateInstrsCleanup::removeRedundantDef(MachineInstr *MI) {
   Register Reg = MI->getOperand(0).getReg();
   BitVector VisitedPreds(MI->getMF()->getNumBlockIDs());
-  clearKillsForDef(Reg, MI->getParent(), VisitedPreds);
+  clearKillsForDef(Reg, MI->getParent(), VisitedPreds, MI);
   MI->eraseFromParent();
   ++NumRemoved;
 }
@@ -175,7 +181,7 @@ static bool isCandidate(const MachineInstr *MI, Register &DefedReg,
 bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
   bool Changed = false;
   Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
-  Reg2MIMap &MBBKills = RegKills[MBB->getNumber()];
+  Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
 
   // Find reusable definitions in the predecessor(s).
   if (!MBB->pred_empty() && !MBB->isEHPad() &&
@@ -225,8 +231,8 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
         MBBDefs.erase(Reg);
         MBBKills.erase(Reg);
       } else if (MI.findRegisterUseOperandIdx(Reg, TRI, true /*isKill*/) != -1)
-        // Keep track of register kills.
-        MBBKills[Reg] = &MI;
+        // Keep track of all instructions that fully or partially kills Reg.
+        MBBKills[Reg].push_back(&MI);
     }
 
     // Record this MI for potential later reuse.
diff --git a/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir b/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
new file mode 100644
index 000000000000000..51ae602fcd517c5
--- /dev/null
+++ b/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
@@ -0,0 +1,52 @@
+# RUN: llc -mtriple=s390x-linux-gnu -run-pass=machine-latecleanup %s -o - -mcpu=z16 \
+# RUN:   -verify-machineinstrs 2>&1 | FileCheck %s
+
+# Kill flag of $r0q (super-reg) needs to be removed, and $r0l needs to be added as live-in.
+# CHECK-LABEL: name: fun0
+# CHECK-LABEL: bb.0:
+# CHECK:         renamable $r0l = LHIMux -1
+# CHECK-NEXT:    J %bb.1
+# CHECK-LABEL: bb.1:
+# CHECK-NEXT:    liveins: $r0l
+# CHECK:         renamable $r1d = LGFI 0
+# CHECK-NEXT:    ST128 renamable $r0q, $r15d, 168, $noreg
+# CHECK-NEXT:    ST killed renamable $r0l, $r15d, 160, $noreg
+# CHECK-NEXT:    Return
+---
+name:            fun0
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    renamable $r0l = LHIMux -1
+    J %bb.1
+
+  bb.1:
+    renamable $r1d = LGFI 0
+    ST128 killed renamable $r0q, $r15d, 168, $noreg
+    renamable $r0l = LHIMux -1
+    ST killed renamable $r0l, $r15d, 160, $noreg
+    Return
+...
+
+# Kill flags of both $r1d and $r0q (super-reg) need to be removed.
+# CHECK-LABEL: name: fun1
+# CHECK-LABEL: bb.0:
+# CHECK-NEXT:    renamable $r1d = LLILL 1
+# CHECK-NEXT:    STG renamable $r1d, killed $r15d, 8, $noreg
+# CHECK-NEXT:    renamable $r0d = LLILL 0
+# CHECK-NEXT:    ST128 renamable $r0q, $r15d, 0, $noreg
+# CHECK-NEXT:    STG killed renamable $r1d, killed $r15d, 8, $noreg
+# CHECK-NEXT:    Return
+---
+name:            fun1
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    renamable $r1d = LLILL 1
+    STG killed renamable $r1d, killed $r15d, 8, $noreg
+    renamable $r0d = LLILL 0
+    ST128 killed renamable $r0q, $r15d, 0, $noreg
+    renamable $r1d = LLILL 1
+    STG killed renamable $r1d, killed $r15d, 8, $noreg
+    Return
+...

>From dc4ff43724f8e10ef98c83941fd3bdcb947f2af5 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Mon, 9 Dec 2024 14:09:08 -0500
Subject: [PATCH 2/3] Updates per review.

---
 llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp | 42 +++++++------------
 .../SystemZ/machine-latecleanup-kills.mir     | 40 ++++++++++--------
 2 files changed, 36 insertions(+), 46 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp b/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
index 5fefa2d8fbbab90..3abb0d7e8ad45f6 100644
--- a/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
+++ b/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
@@ -39,25 +39,24 @@ namespace {
 class MachineLateInstrsCleanup : public MachineFunctionPass {
   const TargetRegisterInfo *TRI = nullptr;
   const TargetInstrInfo *TII = nullptr;
+  const MachineRegisterInfo *MRI = nullptr;
 
-  // Data structures to map regs to their definitions and kills per MBB.
+  // Data structure to map regs to their definitions per MBB.
   struct Reg2MIMap : public SmallDenseMap<Register, MachineInstr *> {
     bool hasIdentical(Register Reg, MachineInstr *ArgMI) {
       MachineInstr *MI = lookup(Reg);
       return MI && MI->isIdenticalTo(*ArgMI);
     }
   };
-  typedef SmallDenseMap<Register, TinyPtrVector<MachineInstr *>> Reg2MIVecMap;
   std::vector<Reg2MIMap> RegDefs;
-  std::vector<Reg2MIVecMap> RegKills;
 
   // Walk through the instructions in MBB and remove any redundant
   // instructions.
   bool processBlock(MachineBasicBlock *MBB);
 
   void removeRedundantDef(MachineInstr *MI);
-  void clearKillsForDef(Register Reg, MachineBasicBlock *MBB,
-                        BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
+  void updateLiveInLists(Register Reg, MachineBasicBlock *MBB,
+                         BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
 
 public:
   static char ID; // Pass identification, replacement for typeid
@@ -94,11 +93,10 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
 
   TRI = MF.getSubtarget().getRegisterInfo();
   TII = MF.getSubtarget().getInstrInfo();
+  MRI = &MF.getRegInfo();
 
   RegDefs.clear();
   RegDefs.resize(MF.getNumBlockIDs());
-  RegKills.clear();
-  RegKills.resize(MF.getNumBlockIDs());
 
   // Visit all MBBs in an order that maximises the reuse from predecessors.
   bool Changed = false;
@@ -111,21 +109,12 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
 
 // Clear any preceding kill flag on Reg after removing a redundant
 // definition.
-void MachineLateInstrsCleanup::clearKillsForDef(Register Reg,
+void MachineLateInstrsCleanup::updateLiveInLists(Register Reg,
                                                 MachineBasicBlock *MBB,
                                                 BitVector &VisitedPreds,
                                                 MachineInstr *ToRemoveMI) {
   VisitedPreds.set(MBB->getNumber());
 
-  // Clear kill flag(s) in MBB, that have been seen after the preceding
-  // definition. If Reg or one of its subregs was killed, it would actually
-  // be ok to stop after removing that (and any other) kill-flag, but it
-  // doesn't seem noticeably faster while it would be a bit more complicated.
-  Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
-  if (MBBKills.contains(Reg))
-    for (auto *KillMI : MBBKills[Reg])
-      KillMI->clearRegisterKills(Reg, TRI);
-
   // Definition in current MBB: done.
   Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
   MachineInstr *DefMI = MBBDefs[Reg];
@@ -133,19 +122,23 @@ void MachineLateInstrsCleanup::clearKillsForDef(Register Reg,
   if (DefMI->getParent() == MBB)
     return;
 
-  // If an earlier def is not in MBB, continue in predecessors.
+  // If the earlier def is not in MBB, it has now become live in. Continue in
+  // predecessors until the defining MBB has been reached.
   if (!MBB->isLiveIn(Reg))
     MBB->addLiveIn(Reg);
   assert(!MBB->pred_empty() && "Predecessor def not found!");
   for (MachineBasicBlock *Pred : MBB->predecessors())
     if (!VisitedPreds.test(Pred->getNumber()))
-      clearKillsForDef(Reg, Pred, VisitedPreds, ToRemoveMI);
+      updateLiveInLists(Reg, Pred, VisitedPreds, ToRemoveMI);
 }
 
 void MachineLateInstrsCleanup::removeRedundantDef(MachineInstr *MI) {
   Register Reg = MI->getOperand(0).getReg();
+  // Clear any and all kill flags.
+  for (MCPhysReg SReg : TRI->superregs_inclusive(Reg))
+    MRI->clearKillFlags(SReg);
   BitVector VisitedPreds(MI->getMF()->getNumBlockIDs());
-  clearKillsForDef(Reg, MI->getParent(), VisitedPreds, MI);
+  updateLiveInLists(Reg, MI->getParent(), VisitedPreds, MI);
   MI->eraseFromParent();
   ++NumRemoved;
 }
@@ -181,7 +174,6 @@ static bool isCandidate(const MachineInstr *MI, Register &DefedReg,
 bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
   bool Changed = false;
   Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
-  Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
 
   // Find reusable definitions in the predecessor(s).
   if (!MBB->pred_empty() && !MBB->isEHPad() &&
@@ -208,7 +200,6 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
     // it) are valid.
     if (MI.modifiesRegister(FrameReg, TRI)) {
       MBBDefs.clear();
-      MBBKills.clear();
       continue;
     }
 
@@ -227,12 +218,8 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
     // Clear any entries in map that MI clobbers.
     for (auto DefI : llvm::make_early_inc_range(MBBDefs)) {
       Register Reg = DefI.first;
-      if (MI.modifiesRegister(Reg, TRI)) {
+      if (MI.modifiesRegister(Reg, TRI))
         MBBDefs.erase(Reg);
-        MBBKills.erase(Reg);
-      } else if (MI.findRegisterUseOperandIdx(Reg, TRI, true /*isKill*/) != -1)
-        // Keep track of all instructions that fully or partially kills Reg.
-        MBBKills[Reg].push_back(&MI);
     }
 
     // Record this MI for potential later reuse.
@@ -240,7 +227,6 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
       LLVM_DEBUG(dbgs() << "Found interesting instruction in "
                         << printMBBReference(*MBB) << ":  " << MI;);
       MBBDefs[DefedReg] = &MI;
-      assert(!MBBKills.count(DefedReg) && "Should already have been removed.");
     }
   }
 
diff --git a/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir b/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
index 51ae602fcd517c5..d0177f78c611684 100644
--- a/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
+++ b/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
@@ -1,21 +1,26 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
 # RUN: llc -mtriple=s390x-linux-gnu -run-pass=machine-latecleanup %s -o - -mcpu=z16 \
 # RUN:   -verify-machineinstrs 2>&1 | FileCheck %s
 
 # Kill flag of $r0q (super-reg) needs to be removed, and $r0l needs to be added as live-in.
-# CHECK-LABEL: name: fun0
-# CHECK-LABEL: bb.0:
-# CHECK:         renamable $r0l = LHIMux -1
-# CHECK-NEXT:    J %bb.1
-# CHECK-LABEL: bb.1:
-# CHECK-NEXT:    liveins: $r0l
-# CHECK:         renamable $r1d = LGFI 0
-# CHECK-NEXT:    ST128 renamable $r0q, $r15d, 168, $noreg
-# CHECK-NEXT:    ST killed renamable $r0l, $r15d, 160, $noreg
-# CHECK-NEXT:    Return
 ---
 name:            fun0
 tracksRegLiveness: true
 body:             |
+  ; CHECK-LABEL: name: fun0
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $r0l = LHIMux -1
+  ; CHECK-NEXT:   J %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   liveins: $r0l
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $r1d = LGFI 0
+  ; CHECK-NEXT:   ST128 renamable $r0q, $r15d, 168, $noreg
+  ; CHECK-NEXT:   ST renamable $r0l, $r15d, 160, $noreg
+  ; CHECK-NEXT:   Return
   bb.0:
     renamable $r0l = LHIMux -1
     J %bb.1
@@ -29,19 +34,18 @@ body:             |
 ...
 
 # Kill flags of both $r1d and $r0q (super-reg) need to be removed.
-# CHECK-LABEL: name: fun1
-# CHECK-LABEL: bb.0:
-# CHECK-NEXT:    renamable $r1d = LLILL 1
-# CHECK-NEXT:    STG renamable $r1d, killed $r15d, 8, $noreg
-# CHECK-NEXT:    renamable $r0d = LLILL 0
-# CHECK-NEXT:    ST128 renamable $r0q, $r15d, 0, $noreg
-# CHECK-NEXT:    STG killed renamable $r1d, killed $r15d, 8, $noreg
-# CHECK-NEXT:    Return
 ---
 name:            fun1
 tracksRegLiveness: true
 body:             |
   bb.0:
+    ; CHECK-LABEL: name: fun1
+    ; CHECK: renamable $r1d = LLILL 1
+    ; CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
+    ; CHECK-NEXT: renamable $r0d = LLILL 0
+    ; CHECK-NEXT: ST128 renamable $r0q, $r15d, 0, $noreg
+    ; CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
+    ; CHECK-NEXT: Return
     renamable $r1d = LLILL 1
     STG killed renamable $r1d, killed $r15d, 8, $noreg
     renamable $r0d = LLILL 0

>From c4e2f714cd43987fd85860e2d33bf21f6cadfa5f Mon Sep 17 00:00:00 2001
From: Jonas Paulsson <paulson1 at linux.ibm.com>
Date: Tue, 10 Dec 2024 10:16:58 -0500
Subject: [PATCH 3/3] Go back to clearing of specific kill-flags.

---
 llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp | 42 ++++++++++++-------
 .../SystemZ/machine-latecleanup-kills.mir     |  4 +-
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp b/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
index 3abb0d7e8ad45f6..5fefa2d8fbbab90 100644
--- a/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
+++ b/llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp
@@ -39,24 +39,25 @@ namespace {
 class MachineLateInstrsCleanup : public MachineFunctionPass {
   const TargetRegisterInfo *TRI = nullptr;
   const TargetInstrInfo *TII = nullptr;
-  const MachineRegisterInfo *MRI = nullptr;
 
-  // Data structure to map regs to their definitions per MBB.
+  // Data structures to map regs to their definitions and kills per MBB.
   struct Reg2MIMap : public SmallDenseMap<Register, MachineInstr *> {
     bool hasIdentical(Register Reg, MachineInstr *ArgMI) {
       MachineInstr *MI = lookup(Reg);
       return MI && MI->isIdenticalTo(*ArgMI);
     }
   };
+  typedef SmallDenseMap<Register, TinyPtrVector<MachineInstr *>> Reg2MIVecMap;
   std::vector<Reg2MIMap> RegDefs;
+  std::vector<Reg2MIVecMap> RegKills;
 
   // Walk through the instructions in MBB and remove any redundant
   // instructions.
   bool processBlock(MachineBasicBlock *MBB);
 
   void removeRedundantDef(MachineInstr *MI);
-  void updateLiveInLists(Register Reg, MachineBasicBlock *MBB,
-                         BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
+  void clearKillsForDef(Register Reg, MachineBasicBlock *MBB,
+                        BitVector &VisitedPreds, MachineInstr *ToRemoveMI);
 
 public:
   static char ID; // Pass identification, replacement for typeid
@@ -93,10 +94,11 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
 
   TRI = MF.getSubtarget().getRegisterInfo();
   TII = MF.getSubtarget().getInstrInfo();
-  MRI = &MF.getRegInfo();
 
   RegDefs.clear();
   RegDefs.resize(MF.getNumBlockIDs());
+  RegKills.clear();
+  RegKills.resize(MF.getNumBlockIDs());
 
   // Visit all MBBs in an order that maximises the reuse from predecessors.
   bool Changed = false;
@@ -109,12 +111,21 @@ bool MachineLateInstrsCleanup::runOnMachineFunction(MachineFunction &MF) {
 
 // Clear any preceding kill flag on Reg after removing a redundant
 // definition.
-void MachineLateInstrsCleanup::updateLiveInLists(Register Reg,
+void MachineLateInstrsCleanup::clearKillsForDef(Register Reg,
                                                 MachineBasicBlock *MBB,
                                                 BitVector &VisitedPreds,
                                                 MachineInstr *ToRemoveMI) {
   VisitedPreds.set(MBB->getNumber());
 
+  // Clear kill flag(s) in MBB, that have been seen after the preceding
+  // definition. If Reg or one of its subregs was killed, it would actually
+  // be ok to stop after removing that (and any other) kill-flag, but it
+  // doesn't seem noticeably faster while it would be a bit more complicated.
+  Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
+  if (MBBKills.contains(Reg))
+    for (auto *KillMI : MBBKills[Reg])
+      KillMI->clearRegisterKills(Reg, TRI);
+
   // Definition in current MBB: done.
   Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
   MachineInstr *DefMI = MBBDefs[Reg];
@@ -122,23 +133,19 @@ void MachineLateInstrsCleanup::updateLiveInLists(Register Reg,
   if (DefMI->getParent() == MBB)
     return;
 
-  // If the earlier def is not in MBB, it has now become live in. Continue in
-  // predecessors until the defining MBB has been reached.
+  // If an earlier def is not in MBB, continue in predecessors.
   if (!MBB->isLiveIn(Reg))
     MBB->addLiveIn(Reg);
   assert(!MBB->pred_empty() && "Predecessor def not found!");
   for (MachineBasicBlock *Pred : MBB->predecessors())
     if (!VisitedPreds.test(Pred->getNumber()))
-      updateLiveInLists(Reg, Pred, VisitedPreds, ToRemoveMI);
+      clearKillsForDef(Reg, Pred, VisitedPreds, ToRemoveMI);
 }
 
 void MachineLateInstrsCleanup::removeRedundantDef(MachineInstr *MI) {
   Register Reg = MI->getOperand(0).getReg();
-  // Clear any and all kill flags.
-  for (MCPhysReg SReg : TRI->superregs_inclusive(Reg))
-    MRI->clearKillFlags(SReg);
   BitVector VisitedPreds(MI->getMF()->getNumBlockIDs());
-  updateLiveInLists(Reg, MI->getParent(), VisitedPreds, MI);
+  clearKillsForDef(Reg, MI->getParent(), VisitedPreds, MI);
   MI->eraseFromParent();
   ++NumRemoved;
 }
@@ -174,6 +181,7 @@ static bool isCandidate(const MachineInstr *MI, Register &DefedReg,
 bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
   bool Changed = false;
   Reg2MIMap &MBBDefs = RegDefs[MBB->getNumber()];
+  Reg2MIVecMap &MBBKills = RegKills[MBB->getNumber()];
 
   // Find reusable definitions in the predecessor(s).
   if (!MBB->pred_empty() && !MBB->isEHPad() &&
@@ -200,6 +208,7 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
     // it) are valid.
     if (MI.modifiesRegister(FrameReg, TRI)) {
       MBBDefs.clear();
+      MBBKills.clear();
       continue;
     }
 
@@ -218,8 +227,12 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
     // Clear any entries in map that MI clobbers.
     for (auto DefI : llvm::make_early_inc_range(MBBDefs)) {
       Register Reg = DefI.first;
-      if (MI.modifiesRegister(Reg, TRI))
+      if (MI.modifiesRegister(Reg, TRI)) {
         MBBDefs.erase(Reg);
+        MBBKills.erase(Reg);
+      } else if (MI.findRegisterUseOperandIdx(Reg, TRI, true /*isKill*/) != -1)
+        // Keep track of all instructions that fully or partially kills Reg.
+        MBBKills[Reg].push_back(&MI);
     }
 
     // Record this MI for potential later reuse.
@@ -227,6 +240,7 @@ bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
       LLVM_DEBUG(dbgs() << "Found interesting instruction in "
                         << printMBBReference(*MBB) << ":  " << MI;);
       MBBDefs[DefedReg] = &MI;
+      assert(!MBBKills.count(DefedReg) && "Should already have been removed.");
     }
   }
 
diff --git a/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir b/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
index d0177f78c611684..09a2b389344b649 100644
--- a/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
+++ b/llvm/test/CodeGen/SystemZ/machine-latecleanup-kills.mir
@@ -19,7 +19,7 @@ body:             |
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   renamable $r1d = LGFI 0
   ; CHECK-NEXT:   ST128 renamable $r0q, $r15d, 168, $noreg
-  ; CHECK-NEXT:   ST renamable $r0l, $r15d, 160, $noreg
+  ; CHECK-NEXT:   ST killed renamable $r0l, $r15d, 160, $noreg
   ; CHECK-NEXT:   Return
   bb.0:
     renamable $r0l = LHIMux -1
@@ -44,7 +44,7 @@ body:             |
     ; CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
     ; CHECK-NEXT: renamable $r0d = LLILL 0
     ; CHECK-NEXT: ST128 renamable $r0q, $r15d, 0, $noreg
-    ; CHECK-NEXT: STG renamable $r1d, killed $r15d, 8, $noreg
+    ; CHECK-NEXT: STG killed renamable $r1d, killed $r15d, 8, $noreg
     ; CHECK-NEXT: Return
     renamable $r1d = LLILL 1
     STG killed renamable $r1d, killed $r15d, 8, $noreg



More information about the llvm-commits mailing list