From 9ddbb61b3ca7d51bcf125cca1c1a4126f4fb09cd Mon Sep 17 00:00:00 2001 From: Jeffrey Byrnes 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 ead6bbe1d5f64..f6c7cc117567b 100644 --- a/llvm/include/llvm/CodeGen/MachineInstr.h +++ b/llvm/include/llvm/CodeGen/MachineInstr.h @@ -45,6 +45,7 @@ class AAResults; template 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 getSpillSize(const TargetInstrInfo *TII) const; diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp index 2f17c04487de7..8df9eb77f12d9 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 941861da5c569..e5655eab1d0a8 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 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 f6c7cc117567b..d58e4168eaab7 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 getSpillSize(const TargetInstrInfo *TII) const; diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp index 8df9eb77f12d9..836a912a5e983 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 e5655eab1d0a8..2388d94aa7a5d 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,