[llvm] 2d338be - [CodeGen] Refactor DeadMIElim isDead and GISel isTriviallyDead (#105956)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 9 07:30:49 PDT 2024
Author: Tobias Stadler
Date: 2024-09-09T16:30:44+02:00
New Revision: 2d338bed00b2bba713bceb4915400063b95929b2
URL: https://github.com/llvm/llvm-project/commit/2d338bed00b2bba713bceb4915400063b95929b2
DIFF: https://github.com/llvm/llvm-project/commit/2d338bed00b2bba713bceb4915400063b95929b2.diff
LOG: [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%
Added:
Modified:
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
llvm/lib/CodeGen/GlobalISel/Utils.cpp
llvm/lib/CodeGen/MachineInstr.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 239387a81ab581..8d6df1026e53a3 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();
@@ -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.
//
diff --git a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
index 332ed37bd2b79f..2f17c04487de7c 100644
--- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ b/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) {
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index b1270e7aeb875c..1713a582d5cfe5 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/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,
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index a0f4040828383e..0d78c2cafbaf63 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/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) {
More information about the llvm-commits
mailing list