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

[Tobias Stadler via llvm-commits]

https://github.com/tobias-stadler updated https://github.com/llvm/llvm-project/pull/105956

>From 0f4e84a3df0d9ceb9a449cfedc9888245f65c9bd Mon Sep 17 00:00:00 2001
From: Tobias Stadler <mail at stadler-tobias.de>
Date: Sat, 24 Aug 2024 17:48:30 +0200
Subject: [PATCH] [CodeGen] Refactor DeadMIElim isDead and GISel
 isTriviallyDead

---
 llvm/include/llvm/CodeGen/MachineInstr.h      |  9 +++++
 .../CodeGen/DeadMachineInstructionElim.cpp    | 33 +++++++++----------
 llvm/lib/CodeGen/GlobalISel/Utils.cpp         | 24 +++-----------
 llvm/lib/CodeGen/MachineInstr.cpp             | 19 +++++++++++
 4 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 04c8144f2fe7af..94a2679429be50 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/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();
@@ -1725,6 +1730,10 @@ class MachineInstr
   /// the instruction's location and its intended destination.
   bool isSafeToMove(bool &SawStore) const;
 
+  /// Return true if this instruction can be considered dead if all of its
+  /// defined registers are dead.
+  bool canBeDead() const;
+
   /// Returns true if this instruction's memory access aliases the memory
   /// access of Other.
   //
diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
index 7fc25cd889a0df..1e94b9ed356feb 100644
--- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ b/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 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,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->canBeDead();
 }
 
 bool DeadMachineInstructionElimImpl::runImpl(MachineFunction &MF) {
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index cfdd9905c16fa6..5209a8638616c6 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -221,31 +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;
-  // 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.canBeDead();
 }
 
 static void reportGISelDiagnostic(DiagnosticSeverity Severity,
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 0f2acdb12389d4..79bd2fb9585b09 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -1323,6 +1323,25 @@ bool MachineInstr::isSafeToMove(bool &SawStore) const {
   return true;
 }
 
+bool MachineInstr::canBeDead() const {
+  // Don't delete frame allocation labels.
+  // FIXME: Why is LOCAL_ESCAPE not considered in MachineInstr::isLabel?
+  if (getOpcode() == TargetOpcode::LOCAL_ESCAPE)
+    return false;
+
+  // LIFETIME markers should be preserved.
+  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;
+  if (!isSafeToMove(SawStore) && !isPHI())
+    return false;
+
+  return true;
+}
+
 static bool MemOperandsHaveAlias(const MachineFrameInfo &MFI, AAResults *AA,
                                  bool UseTBAA, const MachineMemOperand *MMOa,
                                  const MachineMemOperand *MMOb) {