[llvm] [CodeGen] NFC: Move isDead to MachineInstr (PR #123531)
Jeffrey Byrnes via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 23 09:21:56 PST 2025
https://github.com/jrbyrnes updated https://github.com/llvm/llvm-project/pull/123531
>From 9ddbb61b3ca7d51bcf125cca1c1a4126f4fb09cd Mon Sep 17 00:00:00 2001
From: Jeffrey Byrnes <Jeffrey.Byrnes at amd.com>
Date: Mon, 6 Jan 2025 10:16:37 -0800
Subject: [PATCH 1/2] [CodeGen] NFC: Move isDead to MachineInstr
---
llvm/include/llvm/CodeGen/MachineInstr.h | 8 ++++
.../CodeGen/DeadMachineInstructionElim.cpp | 46 +------------------
llvm/lib/CodeGen/MachineInstr.cpp | 38 +++++++++++++++
3 files changed, 47 insertions(+), 45 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index ead6bbe1d5f641..f6c7cc117567bf 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -45,6 +45,7 @@ class AAResults;
template <typename T> class ArrayRef;
class DIExpression;
class DILocalVariable;
+class LiveRegUnits;
class MachineBasicBlock;
class MachineFunction;
class MachineRegisterInfo;
@@ -1786,6 +1787,13 @@ 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;
diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
index 2f17c04487de7c..8df9eb77f12d9a 100644
--- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
@@ -38,7 +38,6 @@ class DeadMachineInstructionElimImpl {
bool runImpl(MachineFunction &MF);
private:
- bool isDead(const MachineInstr *MI) const;
bool eliminateDeadMI(MachineFunction &MF);
};
@@ -79,47 +78,6 @@ char &llvm::DeadMachineInstructionElimID = DeadMachineInstructionElim::ID;
INITIALIZE_PASS(DeadMachineInstructionElim, DEBUG_TYPE,
"Remove dead machine instructions", false, false)
-bool DeadMachineInstructionElimImpl::isDead(const MachineInstr *MI) 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 : MI->all_defs()) {
- Register Reg = MO.getReg();
- if (Reg.isPhysical()) {
- // Don't delete live physreg defs, or any reserved register defs.
- if (!LivePhysRegs.available(Reg) || MRI->isReserved(Reg))
- return false;
- } else {
- if (MO.isDead()) {
-#ifndef NDEBUG
- // Basic check on the register. All of them should be 'undef'.
- for (auto &U : MRI->use_nodbg_operands(Reg))
- assert(U.isUndef() && "'Undef' use on a 'dead' register is found!");
-#endif
- continue;
- }
- for (const MachineInstr &Use : MRI->use_nodbg_instructions(Reg)) {
- if (&Use != MI)
- // 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 (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) {
MRI = &MF.getRegInfo();
@@ -146,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 (isDead(&MI)) {
+ 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
@@ -156,11 +114,9 @@ bool DeadMachineInstructionElimImpl::eliminateDeadMI(MachineFunction &MF) {
++NumDeletes;
continue;
}
-
LivePhysRegs.stepBackward(MI);
}
}
-
LivePhysRegs.clear();
return AnyChanges;
}
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 941861da5c5693..e5655eab1d0a8b 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -18,6 +18,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/Analysis/AliasAnalysis.h"
#include "llvm/Analysis/MemoryLocation.h"
+#include "llvm/CodeGen/LiveRegUnits.h"
#include "llvm/CodeGen/MachineBasicBlock.h"
#include "llvm/CodeGen/MachineFrameInfo.h"
#include "llvm/CodeGen/MachineFunction.h"
@@ -1592,6 +1593,43 @@ 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,
>From c145ffe20c09cf6cf7b3b279f8590d317fdcdd1d Mon Sep 17 00:00:00 2001
From: Jeffrey Byrnes <Jeffrey.Byrnes at amd.com>
Date: Wed, 22 Jan 2025 08:29:39 -0800
Subject: [PATCH 2/2] Early exit + review comments
Change-Id: I90e7829a0afc535760c08417cc50db71f46d9910
---
llvm/include/llvm/CodeGen/MachineInstr.h | 19 +++--
.../CodeGen/DeadMachineInstructionElim.cpp | 2 +-
llvm/lib/CodeGen/MachineInstr.cpp | 74 +++++++++----------
3 files changed, 50 insertions(+), 45 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index f6c7cc117567bf..d58e4168eaab78 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/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;
diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
index 8df9eb77f12d9a..836a912a5e9835 100644
--- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ b/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
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index e5655eab1d0a8b..2388d94aa7a5d1 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/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,
More information about the llvm-commits
mailing list