Early exit + review comments

Change-Id: I90e7829a0afc535760c08417cc50db71f46d9910

Author: jrbyrnes

llvm/include/llvm/CodeGen/MachineInstr.h

@@ -1738,6 +1738,18 @@ class MachineInstr
   /// defined registers were dead.
   bool wouldBeTriviallyDead() const;
 
+  /// Check whether an MI is dead. If \p LivePhysRegs is provided, it is assumed
+  /// to be at the position of MI and will be used to check the Liveness of
+  /// physical register defs. If \p LivePhysRegs is not provided, this will
+  /// pessimistically assume any PhysReg def is live.
+  /// For trivially dead instructions (i.e. those without hard to model effects
+  /// / wouldBeTriviallyDead), this checks deadness by analyzing defs of the
+  /// MachineInstr. If the instruction wouldBeTriviallyDead, and  all the defs
+  /// either have dead flags or have no uses, then the instruction is said to be
+  /// dead.
+  bool isDead(const MachineRegisterInfo &MRI,
+              LiveRegUnits *LivePhysRegs = nullptr) const;
+
   /// Returns true if this instruction's memory access aliases the memory
   /// access of Other.
   //
@@ -1787,13 +1799,6 @@ class MachineInstr
   /// Return true if all the implicit defs of this instruction are dead.
   bool allImplicitDefsAreDead() const;
 
-  /// Check whether an MI is dead. If \p LivePhysRegs is provided, it is assumed
-  /// to be at the position of MI and will be used to check the Liveness of
-  /// physical register defs. If \p LivePhysRegs is not provided, this will
-  /// pessimistically assume any PhysReg def is live.
-  bool isDead(const MachineRegisterInfo *MRI,
-              LiveRegUnits *LivePhysRegs = nullptr) const;
-
   /// Return a valid size if the instruction is a spill instruction.
   std::optional<LocationSize> getSpillSize(const TargetInstrInfo *TII) const;
 

llvm/lib/CodeGen/DeadMachineInstructionElim.cpp

@@ -104,7 +104,7 @@ bool DeadMachineInstructionElimImpl::eliminateDeadMI(MachineFunction &MF) {
     // liveness as we go.
     for (MachineInstr &MI : make_early_inc_range(reverse(*MBB))) {
       // If the instruction is dead, delete it!
-      if (MI.isDead(MRI, &LivePhysRegs)) {
+      if (MI.isDead(*MRI, &LivePhysRegs)) {
         LLVM_DEBUG(dbgs() << "DeadMachineInstructionElim: DELETING: " << MI);
         // It is possible that some DBG_VALUE instructions refer to this
         // instruction. They will be deleted in the live debug variable

llvm/lib/CodeGen/MachineInstr.cpp

@@ -1351,6 +1351,43 @@ bool MachineInstr::wouldBeTriviallyDead() const {
   return isPHI() || isSafeToMove(SawStore);
 }
 
+bool MachineInstr::isDead(const MachineRegisterInfo &MRI,
+                          LiveRegUnits *LivePhysRegs) const {
+  // Technically speaking inline asm without side effects and no defs can still
+  // be deleted. But there is so much bad inline asm code out there, we should
+  // let them be.
+  if (isInlineAsm())
+    return false;
+
+  // If we suspect this instruction may have some side-effects, then we say
+  // this instruction cannot be dead.
+  // FIXME: See issue #105950 for why LIFETIME markers are considered dead here.
+  if (!isLifetimeMarker() && !wouldBeTriviallyDead())
+    return false;
+
+  // Instructions without side-effects are dead iff they only define dead regs.
+  // This function is hot and this loop returns early in the common case,
+  // so only perform additional checks before this if absolutely necessary.
+  for (const MachineOperand &MO : all_defs()) {
+    Register Reg = MO.getReg();
+    if (Reg.isPhysical()) {
+      // Don't delete live physreg defs, or any reserved register defs.
+      if (!LivePhysRegs || !LivePhysRegs->available(Reg) || MRI.isReserved(Reg))
+        return false;
+    } else {
+      if (MO.isDead())
+        continue;
+      for (const MachineInstr &Use : MRI.use_nodbg_instructions(Reg)) {
+        if (&Use != this)
+          // This def has a non-debug use. Don't delete the instruction!
+          return false;
+      }
+    }
+  }
+
+  return true;
+}
+
 static bool MemOperandsHaveAlias(const MachineFrameInfo &MFI, AAResults *AA,
                                  bool UseTBAA, const MachineMemOperand *MMOa,
                                  const MachineMemOperand *MMOb) {
@@ -1593,43 +1630,6 @@ bool MachineInstr::allImplicitDefsAreDead() const {
   return true;
 }
 
-bool MachineInstr::isDead(const MachineRegisterInfo *MRI,
-                          LiveRegUnits *LivePhysRegs) const {
-  // Instructions without side-effects are dead iff they only define dead regs.
-  // This function is hot and this loop returns early in the common case,
-  // so only perform additional checks before this if absolutely necessary.
-  for (const MachineOperand &MO : all_defs()) {
-    Register Reg = MO.getReg();
-    if (Reg.isPhysical()) {
-      // Don't delete live physreg defs, or any reserved register defs.
-      if (!LivePhysRegs || !LivePhysRegs->available(Reg) ||
-          MRI->isReserved(Reg))
-        return false;
-    } else {
-      if (MO.isDead())
-        continue;
-      for (const MachineInstr &Use : MRI->use_nodbg_instructions(Reg)) {
-        if (&Use != this)
-          // This def has a non-debug use. Don't delete the instruction!
-          return false;
-      }
-    }
-  }
-
-  // Technically speaking inline asm without side effects and no defs can still
-  // be deleted. But there is so much bad inline asm code out there, we should
-  // let them be.
-  if (isInlineAsm())
-    return false;
-
-  // FIXME: See issue #105950 for why LIFETIME markers are considered dead here.
-  if (isLifetimeMarker())
-    return true;
-
-  // If there are no defs with uses, the instruction might be dead.
-  return wouldBeTriviallyDead();
-}
-
 /// copyImplicitOps - Copy implicit register operands from specified
 /// instruction to this instruction.
 void MachineInstr::copyImplicitOps(MachineFunction &MF,