[CodeGen] Refactor DeadMIElim isDead and GISel isTriviallyDead (#105956)

Merge GlobalISel's isTriviallyDead and DeadMachineInstructionElim's
isDead code and remove all unnecessary checks from the hot path by
looping over the operands before doing any other checks.

See #105950 for why DeadMIElim needs to remove LIFETIME markers even
though they probably shouldn't generally be considered dead.

x86 CTMark O3: -0.1%
AArch64 GlobalISel CTMark O0: -0.6%, O2: -0.2%

Author: tobias-stadler

llvm/include/llvm/CodeGen/MachineInstr.h

@@ -1323,6 +1323,11 @@ class MachineInstr
     return getOpcode() == TargetOpcode::ANNOTATION_LABEL;
   }
 
+  bool isLifetimeMarker() const {
+    return getOpcode() == TargetOpcode::LIFETIME_START ||
+           getOpcode() == TargetOpcode::LIFETIME_END;
+  }
+
   /// Returns true if the MachineInstr represents a label.
   bool isLabel() const {
     return isEHLabel() || isGCLabel() || isAnnotationLabel();
@@ -1727,6 +1732,10 @@ class MachineInstr
   /// the instruction's location and its intended destination.
   bool isSafeToMove(bool &SawStore) const;
 
+  /// Return true if this instruction would be trivially dead if all of its
+  /// defined registers were dead.
+  bool wouldBeTriviallyDead() const;
+
   /// Returns true if this instruction's memory access aliases the memory
   /// access of Other.
   //

llvm/lib/CodeGen/DeadMachineInstructionElim.cpp

@@ -80,23 +80,9 @@ INITIALIZE_PASS(DeadMachineInstructionElim, DEBUG_TYPE,
                 "Remove dead machine instructions", false, false)
 
 bool DeadMachineInstructionElimImpl::isDead(const MachineInstr *MI) 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 (MI->isInlineAsm())
-    return false;
-
-  // Don't delete frame allocation labels.
-  if (MI->getOpcode() == TargetOpcode::LOCAL_ESCAPE ||
-      MI->getOpcode() == TargetOpcode::FAKE_USE)
-    return false;
-
-  // Don't delete instructions with side effects.
-  bool SawStore = false;
-  if (!MI->isSafeToMove(SawStore) && !MI->isPHI())
-    return false;
-
-  // Examine each operand.
+  // 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 : MI->all_defs()) {
     Register Reg = MO.getReg();
     if (Reg.isPhysical()) {
@@ -120,8 +106,18 @@ bool DeadMachineInstructionElimImpl::isDead(const MachineInstr *MI) const {
     }
   }
 
-  // If there are no defs with uses, the instruction is dead.
-  return true;
+  // 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 (MI->isInlineAsm())
+    return false;
+
+  // FIXME: See issue #105950 for why LIFETIME markers are considered dead here.
+  if (MI->isLifetimeMarker())
+    return true;
+
+  // If there are no defs with uses, the instruction might be dead.
+  return MI->wouldBeTriviallyDead();
 }
 
 bool DeadMachineInstructionElimImpl::runImpl(MachineFunction &MF) {

llvm/lib/CodeGen/GlobalISel/Utils.cpp

@@ -221,34 +221,15 @@ bool llvm::canReplaceReg(Register DstReg, Register SrcReg,
 
 bool llvm::isTriviallyDead(const MachineInstr &MI,
                            const MachineRegisterInfo &MRI) {
-  // FIXME: This logical is mostly duplicated with
-  // DeadMachineInstructionElim::isDead. Why is LOCAL_ESCAPE not considered in
-  // MachineInstr::isLabel?
-
-  // Don't delete frame allocation labels.
-  if (MI.getOpcode() == TargetOpcode::LOCAL_ESCAPE)
-    return false;
-  // Don't delete fake uses.
-  if (MI.getOpcode() == TargetOpcode::FAKE_USE)
-    return false;
-  // LIFETIME markers should be preserved even if they seem dead.
-  if (MI.getOpcode() == TargetOpcode::LIFETIME_START ||
-      MI.getOpcode() == TargetOpcode::LIFETIME_END)
-    return false;
-
-  // If we can move an instruction, we can remove it.  Otherwise, it has
-  // a side-effect of some sort.
-  bool SawStore = false;
-  if (!MI.isSafeToMove(SawStore) && !MI.isPHI())
-    return false;
-
-  // Instructions without side-effects are dead iff they only define dead vregs.
+  // 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 auto &MO : MI.all_defs()) {
     Register Reg = MO.getReg();
     if (Reg.isPhysical() || !MRI.use_nodbg_empty(Reg))
       return false;
   }
-  return true;
+  return MI.wouldBeTriviallyDead();
 }
 
 static void reportGISelDiagnostic(DiagnosticSeverity Severity,

llvm/lib/CodeGen/MachineInstr.cpp

@@ -1323,6 +1323,28 @@ bool MachineInstr::isSafeToMove(bool &SawStore) const {
   return true;
 }
 
+bool MachineInstr::wouldBeTriviallyDead() const {
+  // Don't delete frame allocation labels.
+  // FIXME: Why is LOCAL_ESCAPE not considered in MachineInstr::isLabel?
+  if (getOpcode() == TargetOpcode::LOCAL_ESCAPE)
+    return false;
+
+  // Don't delete FAKE_USE.
+  // FIXME: Why is FAKE_USE not considered in MachineInstr::isPosition?
+  if (isFakeUse())
+    return false;
+
+  // LIFETIME markers should be preserved.
+  // FIXME: Why are LIFETIME markers not considered in MachineInstr::isPosition?
+  if (isLifetimeMarker())
+    return false;
+
+  // If we can move an instruction, we can remove it.  Otherwise, it has
+  // a side-effect of some sort.
+  bool SawStore = false;
+  return isPHI() || isSafeToMove(SawStore);
+}
+
 static bool MemOperandsHaveAlias(const MachineFrameInfo &MFI, AAResults *AA,
                                  bool UseTBAA, const MachineMemOperand *MMOa,
                                  const MachineMemOperand *MMOb) {