Skip to content

Commit 0bf5846

Browse files
authored
InstructionSelect: Use GISelChangeObserver instead of MachineFunction::Delegate (#105725)
The main difference is that it's possible for multiple change observers to be installed at the same time whereas there can only be one MachineFunction delegate installed. This allows downstream targets to continue to use observers to recursively select. The target in question was selecting a gMIR instruction to a machine instruction plus some gMIR around it and relying on observers to ensure it correctly selected any gMIR it created before returning to the main loop.
1 parent 3d18cea commit 0bf5846

File tree

4 files changed

+45
-5
lines changed

4 files changed

+45
-5
lines changed

llvm/include/llvm/CodeGen/GlobalISel/GISelChangeObserver.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,5 +138,19 @@ class RAIIMFObsDelInstaller {
138138
~RAIIMFObsDelInstaller() = default;
139139
};
140140

141+
/// A simple RAII based Observer installer.
142+
/// Use this in a scope to install the Observer to the MachineFunction and reset
143+
/// it at the end of the scope.
144+
class RAIITemporaryObserverInstaller {
145+
public:
146+
RAIITemporaryObserverInstaller(GISelObserverWrapper &Observers,
147+
GISelChangeObserver &TemporaryObserver);
148+
~RAIITemporaryObserverInstaller();
149+
150+
private:
151+
GISelObserverWrapper &Observers;
152+
GISelChangeObserver &TemporaryObserver;
153+
};
154+
141155
} // namespace llvm
142156
#endif

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
#include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
1717

1818
namespace llvm {
19+
class GISelObserverWrapper;
20+
1921
class InstructionSelector : public GIMatchTableExecutor {
2022
public:
2123
virtual ~InstructionSelector();
@@ -36,6 +38,11 @@ class InstructionSelector : public GIMatchTableExecutor {
3638
const TargetPassConfig *TPC = nullptr;
3739

3840
MachineOptimizationRemarkEmitter *MORE = nullptr;
41+
42+
/// Note: InstructionSelect does not track changed instructions.
43+
/// changingInstr() and changedInstr() will never be called on these
44+
/// observers.
45+
GISelObserverWrapper *AllObservers = nullptr;
3946
};
4047
} // namespace llvm
4148

llvm/lib/CodeGen/GlobalISel/GISelChangeObserver.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,13 @@ RAIIMFObserverInstaller::RAIIMFObserverInstaller(MachineFunction &MF,
4646
}
4747

4848
RAIIMFObserverInstaller::~RAIIMFObserverInstaller() { MF.setObserver(nullptr); }
49+
50+
RAIITemporaryObserverInstaller::RAIITemporaryObserverInstaller(
51+
GISelObserverWrapper &Observers, GISelChangeObserver &TemporaryObserver)
52+
: Observers(Observers), TemporaryObserver(TemporaryObserver) {
53+
Observers.addObserver(&TemporaryObserver);
54+
}
55+
56+
RAIITemporaryObserverInstaller::~RAIITemporaryObserverInstaller() {
57+
Observers.removeObserver(&TemporaryObserver);
58+
}

llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,19 +75,25 @@ InstructionSelect::InstructionSelect(CodeGenOptLevel OL, char &PassID)
7575
/// a non-obvious limitation for selector implementers. Therefore, to allow
7676
/// deletion of arbitrary instructions, we detect this case and continue
7777
/// selection with the predecessor of the deleted instruction.
78-
class InstructionSelect::MIIteratorMaintainer
79-
: public MachineFunction::Delegate {
78+
class InstructionSelect::MIIteratorMaintainer : public GISelChangeObserver {
8079
#ifndef NDEBUG
8180
SmallSetVector<const MachineInstr *, 32> CreatedInstrs;
8281
#endif
8382
public:
8483
MachineBasicBlock::reverse_iterator MII;
8584

86-
void MF_HandleInsertion(MachineInstr &MI) override {
85+
void changingInstr(MachineInstr &MI) override {
86+
llvm_unreachable("InstructionSelect does not track changed instructions!");
87+
}
88+
void changedInstr(MachineInstr &MI) override {
89+
llvm_unreachable("InstructionSelect does not track changed instructions!");
90+
}
91+
92+
void createdInstr(MachineInstr &MI) override {
8793
LLVM_DEBUG(dbgs() << "Creating: " << MI; CreatedInstrs.insert(&MI));
8894
}
8995

90-
void MF_HandleRemoval(MachineInstr &MI) override {
96+
void erasingInstr(MachineInstr &MI) override {
9197
LLVM_DEBUG(dbgs() << "Erasing: " << MI; CreatedInstrs.remove(&MI));
9298
if (MII.getInstrIterator().getNodePtr() == &MI) {
9399
// If the iterator points to the MI that will be erased (i.e. the MI prior
@@ -190,8 +196,11 @@ bool InstructionSelect::selectMachineFunction(MachineFunction &MF) {
190196
// GISelChangeObserver, because we do not want notifications about changed
191197
// instructions. This prevents significant compile-time regressions from
192198
// e.g. constrainOperandRegClass().
199+
GISelObserverWrapper AllObservers;
193200
MIIteratorMaintainer MIIMaintainer;
194-
RAIIDelegateInstaller DelInstaller(MF, &MIIMaintainer);
201+
AllObservers.addObserver(&MIIMaintainer);
202+
RAIIDelegateInstaller DelInstaller(MF, &AllObservers);
203+
ISel->AllObservers = &AllObservers;
195204

196205
for (MachineBasicBlock *MBB : post_order(&MF)) {
197206
ISel->CurMBB = MBB;

0 commit comments

Comments
 (0)