[llvm] [CodeGen] Refactor DeadMIElim isDead and GISel isTriviallyDead (PR #105956)
Tobias Stadler via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 29 12:17:51 PDT 2024
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 1/4] [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) {
>From 419667913423953e3d53b1f97b2b1dc474a2eabd Mon Sep 17 00:00:00 2001
From: Tobias Stadler <mail at stadler-tobias.de>
Date: Tue, 27 Aug 2024 18:43:17 +0200
Subject: [PATCH 2/4] Rename function
---
llvm/include/llvm/CodeGen/MachineInstr.h | 6 +++---
llvm/lib/CodeGen/DeadMachineInstructionElim.cpp | 2 +-
llvm/lib/CodeGen/GlobalISel/Utils.cpp | 2 +-
llvm/lib/CodeGen/MachineInstr.cpp | 3 ++-
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 94a2679429be50..be7455ff3bb57c 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -1730,9 +1730,9 @@ 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;
+ /// 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 1e94b9ed356feb..2f17c04487de7c 100644
--- a/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
+++ b/llvm/lib/CodeGen/DeadMachineInstructionElim.cpp
@@ -117,7 +117,7 @@ bool DeadMachineInstructionElimImpl::isDead(const MachineInstr *MI) const {
return true;
// If there are no defs with uses, the instruction might be dead.
- return MI->canBeDead();
+ 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 5209a8638616c6..1713a582d5cfe5 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -229,7 +229,7 @@ bool llvm::isTriviallyDead(const MachineInstr &MI,
if (Reg.isPhysical() || !MRI.use_nodbg_empty(Reg))
return false;
}
- return MI.canBeDead();
+ return MI.wouldBeTriviallyDead();
}
static void reportGISelDiagnostic(DiagnosticSeverity Severity,
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 79bd2fb9585b09..05178ccaa0a4db 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -1323,13 +1323,14 @@ bool MachineInstr::isSafeToMove(bool &SawStore) const {
return true;
}
-bool MachineInstr::canBeDead() const {
+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;
// LIFETIME markers should be preserved.
+ // FIXME: Why are LIFETIME markers not considered in MachineInstr::isPosition?
if (isLifetimeMarker())
return false;
>From c3fda9c0b48213b580c04a4dc2fe72dca7cac906 Mon Sep 17 00:00:00 2001
From: Tobias Stadler <mail at stadler-tobias.de>
Date: Wed, 28 Aug 2024 23:53:20 +0200
Subject: [PATCH 3/4] Return expression
---
llvm/lib/CodeGen/MachineInstr.cpp | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 05178ccaa0a4db..c5fb2d93f5c543 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -1337,10 +1337,7 @@ bool MachineInstr::wouldBeTriviallyDead() const {
// 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;
+ return isSafeToMove(SawStore) || isPHI();
}
static bool MemOperandsHaveAlias(const MachineFrameInfo &MFI, AAResults *AA,
>From 5cf2459accc157e66d0e3cbab9b648b8fbe52aa6 Mon Sep 17 00:00:00 2001
From: Tobias Stadler <mail at stadler-tobias.de>
Date: Thu, 29 Aug 2024 21:15:11 +0200
Subject: [PATCH 4/4] phi check first
---
llvm/lib/CodeGen/MachineInstr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index c5fb2d93f5c543..53a47927fe92e7 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/lib/CodeGen/MachineInstr.cpp
@@ -1337,7 +1337,7 @@ bool MachineInstr::wouldBeTriviallyDead() const {
// If we can move an instruction, we can remove it. Otherwise, it has
// a side-effect of some sort.
bool SawStore = false;
- return isSafeToMove(SawStore) || isPHI();
+ return isPHI() || isSafeToMove(SawStore);
}
static bool MemOperandsHaveAlias(const MachineFrameInfo &MFI, AAResults *AA,
More information about the llvm-commits
mailing list