[CodeGen] Refactor DeadMIElim isDead and GISel isTriviallyDead

Merge GlobalISel's isTriviallyDead and DeadMachineInstructionElim's
isDead behavior and move all unnecessary checks out of the hot path.

Functional changes to ponder over:
1. isTriviallyDead now never removes inlineasm like DeadMIElim.
2. We loop over the operands before doing any other checks. Please
   especially double-check if this fine for the loop in DeadMIElim.

See #105950 for why the LIFETIME markers check can't be moved to
wouldBeTriviallyDead().

Author: tobias-stadler

llvm/include/llvm/CodeGen/MachineInstr.h

@@ -1725,6 +1725,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 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,22 +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)
-    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 very 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()) {
@@ -119,8 +106,8 @@ bool DeadMachineInstructionElimImpl::isDead(const MachineInstr *MI) const {
     }
   }
 
-  // If there are no defs with uses, the instruction is dead.
-  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,31 +221,22 @@ 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?
+  // Instructions without side-effects are dead iff they only define dead regs.
+  // This function is very 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;
+  }
 
-  // Don't delete frame allocation labels.
-  if (MI.getOpcode() == TargetOpcode::LOCAL_ESCAPE)
-    return false;
   // LIFETIME markers should be preserved even if they seem dead.
+  // FIXME: See issue #105950 for why this isn't in wouldBeTriviallyDead().
   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.
-  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,27 @@ bool MachineInstr::isSafeToMove(bool &SawStore) const {
   return true;
 }
 
+bool MachineInstr::wouldBeTriviallyDead() 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;
+
+  // Don't delete frame allocation labels.
+  // FIXME: Why is LOCAL_ESCAPE not considered in MachineInstr::isLabel?
+  if (getOpcode() == TargetOpcode::LOCAL_ESCAPE)
+    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 (!isSafeToMove(SawStore) && !isPHI())
+    return false;
+
+  return true;
+}
+
 static bool MemOperandsHaveAlias(const MachineFrameInfo &MFI, AAResults *AA,
                                  bool UseTBAA, const MachineMemOperand *MMOa,
                                  const MachineMemOperand *MMOb) {