[globalisel] Rename GISelChangeObserver's erasedInstr() to erasingInstr() and related nits. NFC
Summary:
There's little of interest that can be done to an already-erased instruction.
You can't inspect it, write it to a debug log, etc. It ought to be notification
that we're about to erase it. Rename the function to clarify the timing of the
event and reflect current usage.
Also fixed one case where we were trying to print an erased instruction.
Reviewers: aditya_nandakumar
Reviewed By: aditya_nandakumar
Subscribers: rovka, kristof.beyls, llvm-commits
Differential Revision: https://reviews.llvm.org/D55611
llvm-svn: 348976
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index 9f9a1de..26b1cd5 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -30,7 +30,6 @@
MachineRegisterInfo &MRI;
GISelChangeObserver &Observer;
- void eraseInstr(MachineInstr &MI);
void scheduleForVisit(MachineInstr &MI);
public:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
index f53cfb1..eed1f86 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h
@@ -18,7 +18,7 @@
namespace llvm {
/// Abstract class that contains various methods for clients to notify about
/// changes. This should be the preferred way for APIs to notify changes.
-/// Typically calling erasedInstr/createdInstr multiple times should not affect
+/// Typically calling erasingInstr/createdInstr multiple times should not affect
/// the result. The observer would likely need to check if it was already
/// notified earlier (consider using GISelWorkList).
class MachineInstr;
@@ -26,8 +26,8 @@
public:
virtual ~GISelChangeObserver() {}
- /// An instruction was erased.
- virtual void erasedInstr(MachineInstr &MI) = 0;
+ /// An instruction is about to be erased.
+ virtual void erasingInstr(MachineInstr &MI) = 0;
/// An instruction was created and inserted into the function.
virtual void createdInstr(MachineInstr &MI) = 0;
/// This instruction was mutated in some way.
diff --git a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
index 8f7431d..f944b5a 100644
--- a/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Combiner.cpp
@@ -43,7 +43,7 @@
WorkListMaintainer(WorkListTy &WorkList) : WorkList(WorkList) {}
virtual ~WorkListMaintainer() {}
- void erasedInstr(MachineInstr &MI) override {
+ void erasingInstr(MachineInstr &MI) override {
LLVM_DEBUG(dbgs() << "Erased: "; MI.print(dbgs()); dbgs() << "\n");
WorkList.remove(&MI);
}
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 179c073..6018c59 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -23,9 +23,6 @@
MachineIRBuilder &B)
: Builder(B), MRI(Builder.getMF().getRegInfo()), Observer(Observer) {}
-void CombinerHelper::eraseInstr(MachineInstr &MI) {
- Observer.erasedInstr(MI);
-}
void CombinerHelper::scheduleForVisit(MachineInstr &MI) {
Observer.createdInstr(MI);
}
@@ -299,8 +296,8 @@
Observer.createdInstr(*NewMI);
}
for (auto &EraseMI : ScheduleForErase) {
+ Observer.erasingInstr(*EraseMI);
EraseMI->eraseFromParent();
- Observer.erasedInstr(*EraseMI);
}
MI.getOperand(0).setReg(ChosenDstReg);
diff --git a/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp b/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
index 9f4d2aa..c4de312 100644
--- a/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Legalizer.cpp
@@ -94,7 +94,8 @@
LLVM_DEBUG(dbgs() << ".. .. New MI: " << MI;);
}
- void erasedInstr(MachineInstr &MI) override {
+ void erasingInstr(MachineInstr &MI) override {
+ LLVM_DEBUG(dbgs() << ".. .. Erasing: " << MI;);
InstList.remove(&MI);
ArtifactList.remove(&MI);
}
@@ -146,7 +147,7 @@
const LegalizerInfo &LInfo(Helper.getLegalizerInfo());
LegalizationArtifactCombiner ArtCombiner(Helper.MIRBuilder, MF.getRegInfo(), LInfo);
auto RemoveDeadInstFromLists = [&WorkListObserver](MachineInstr *DeadMI) {
- WorkListObserver.erasedInstr(*DeadMI);
+ WorkListObserver.erasingInstr(*DeadMI);
};
bool Changed = false;
do {
@@ -175,7 +176,7 @@
MachineInstr &MI = *ArtifactList.pop_back_val();
assert(isPreISelGenericOpcode(MI.getOpcode()) && "Expecting generic opcode");
if (isTriviallyDead(MI, MRI)) {
- LLVM_DEBUG(dbgs() << MI << "Is dead; erasing.\n");
+ LLVM_DEBUG(dbgs() << MI << "Is dead\n");
RemoveDeadInstFromLists(&MI);
MI.eraseFromParentAndMarkDBGValuesForRemoval();
continue;
@@ -183,7 +184,7 @@
SmallVector<MachineInstr *, 4> DeadInstructions;
if (ArtCombiner.tryCombineInstruction(MI, DeadInstructions)) {
for (auto *DeadMI : DeadInstructions) {
- LLVM_DEBUG(dbgs() << ".. Erasing Dead Instruction " << *DeadMI);
+ LLVM_DEBUG(dbgs() << *DeadMI << "Is dead\n");
RemoveDeadInstFromLists(DeadMI);
DeadMI->eraseFromParentAndMarkDBGValuesForRemoval();
}
diff --git a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
index 997bd92..56cf45e 100644
--- a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
@@ -15,7 +15,7 @@
public:
void changedInstr(MachineInstr &MI) override {}
void createdInstr(MachineInstr &MI) override {}
- void erasedInstr(MachineInstr &MI) override {}
+ void erasingInstr(MachineInstr &MI) override {}
};
// Test CTTZ expansion when CTTZ_ZERO_UNDEF is legal or custom,