Skip to content

Commit f7ad495

Browse files
author
vporpo
authored
[SandboxIR] Clean up tracking code with the help of emplaceIfTracking() (#102406)
This patch introduces Tracker::emplaceIfTracking(), a wrapper of Tracker::track() that will conditionally create the change object if tracking is enabled. This patch also removes the `Parent` member field of `IRChangeBase`.
1 parent 13fc914 commit f7ad495

File tree

4 files changed

+130
-225
lines changed

4 files changed

+130
-225
lines changed

‎llvm/include/llvm/SandboxIR/SandboxIR.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,9 @@ class Value {
258258
template <typename ItTy, typename SBTy> friend class LLVMOpUserItToSBTy;
259259

260260
Value(ClassID SubclassID, llvm::Value *Val, Context &Ctx);
261+
/// Disable copies.
262+
Value(const Value &) = delete;
263+
Value &operator=(const Value &) = delete;
261264

262265
public:
263266
virtual ~Value() = default;

‎llvm/include/llvm/SandboxIR/Tracker.h‎

Lines changed: 62 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,15 @@ class AllocaInst;
6363
/// The base class for IR Change classes.
6464
class IRChangeBase {
6565
protected:
66-
Tracker &Parent;
66+
friend class Tracker; // For Parent.
6767

6868
public:
69-
IRChangeBase(Tracker &Parent);
7069
/// This runs when changes get reverted.
71-
virtual void revert() = 0;
70+
virtual void revert(Tracker &Tracker) = 0;
7271
/// This runs when changes get accepted.
7372
virtual void accept() = 0;
7473
virtual ~IRChangeBase() = default;
7574
#ifndef NDEBUG
76-
/// \Returns the index of this change by iterating over all changes in the
77-
/// tracker. This is only used for debugging.
78-
unsigned getIdx() const;
79-
void dumpCommon(raw_ostream &OS) const { OS << getIdx() << ". "; }
8075
virtual void dump(raw_ostream &OS) const = 0;
8176
LLVM_DUMP_METHOD virtual void dump() const = 0;
8277
friend raw_ostream &operator<<(raw_ostream &OS, const IRChangeBase &C) {
@@ -92,15 +87,11 @@ class UseSet : public IRChangeBase {
9287
Value *OrigV = nullptr;
9388

9489
public:
95-
UseSet(const Use &U, Tracker &Tracker)
96-
: IRChangeBase(Tracker), U(U), OrigV(U.get()) {}
97-
void revert() final { U.set(OrigV); }
90+
UseSet(const Use &U) : U(U), OrigV(U.get()) {}
91+
void revert(Tracker &Tracker) final { U.set(OrigV); }
9892
void accept() final {}
9993
#ifndef NDEBUG
100-
void dump(raw_ostream &OS) const final {
101-
dumpCommon(OS);
102-
OS << "UseSet";
103-
}
94+
void dump(raw_ostream &OS) const final { OS << "UseSet"; }
10495
LLVM_DUMP_METHOD void dump() const final;
10596
#endif
10697
};
@@ -115,14 +106,11 @@ class PHISetIncoming : public IRChangeBase {
115106
Value,
116107
Block,
117108
};
118-
PHISetIncoming(PHINode &PHI, unsigned Idx, What What, Tracker &Tracker);
119-
void revert() final;
109+
PHISetIncoming(PHINode &PHI, unsigned Idx, What What);
110+
void revert(Tracker &Tracker) final;
120111
void accept() final {}
121112
#ifndef NDEBUG
122-
void dump(raw_ostream &OS) const final {
123-
dumpCommon(OS);
124-
OS << "PHISetIncoming";
125-
}
113+
void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; }
126114
LLVM_DUMP_METHOD void dump() const final;
127115
#endif
128116
};
@@ -134,14 +122,11 @@ class PHIRemoveIncoming : public IRChangeBase {
134122
BasicBlock *RemovedBB;
135123

136124
public:
137-
PHIRemoveIncoming(PHINode &PHI, unsigned RemovedIdx, Tracker &Tracker);
138-
void revert() final;
125+
PHIRemoveIncoming(PHINode &PHI, unsigned RemovedIdx);
126+
void revert(Tracker &Tracker) final;
139127
void accept() final {}
140128
#ifndef NDEBUG
141-
void dump(raw_ostream &OS) const final {
142-
dumpCommon(OS);
143-
OS << "PHISetIncoming";
144-
}
129+
void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; }
145130
LLVM_DUMP_METHOD void dump() const final;
146131
#endif
147132
};
@@ -151,14 +136,11 @@ class PHIAddIncoming : public IRChangeBase {
151136
unsigned Idx;
152137

153138
public:
154-
PHIAddIncoming(PHINode &PHI, Tracker &Tracker);
155-
void revert() final;
139+
PHIAddIncoming(PHINode &PHI);
140+
void revert(Tracker &Tracker) final;
156141
void accept() final {}
157142
#ifndef NDEBUG
158-
void dump(raw_ostream &OS) const final {
159-
dumpCommon(OS);
160-
OS << "PHISetIncoming";
161-
}
143+
void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; }
162144
LLVM_DUMP_METHOD void dump() const final;
163145
#endif
164146
};
@@ -169,17 +151,14 @@ class UseSwap : public IRChangeBase {
169151
Use OtherUse;
170152

171153
public:
172-
UseSwap(const Use &ThisUse, const Use &OtherUse, Tracker &Tracker)
173-
: IRChangeBase(Tracker), ThisUse(ThisUse), OtherUse(OtherUse) {
154+
UseSwap(const Use &ThisUse, const Use &OtherUse)
155+
: ThisUse(ThisUse), OtherUse(OtherUse) {
174156
assert(ThisUse.getUser() == OtherUse.getUser() && "Expected same user!");
175157
}
176-
void revert() final { ThisUse.swap(OtherUse); }
158+
void revert(Tracker &Tracker) final { ThisUse.swap(OtherUse); }
177159
void accept() final {}
178160
#ifndef NDEBUG
179-
void dump(raw_ostream &OS) const final {
180-
dumpCommon(OS);
181-
OS << "UseSwap";
182-
}
161+
void dump(raw_ostream &OS) const final { OS << "UseSwap"; }
183162
LLVM_DUMP_METHOD void dump() const final;
184163
#endif
185164
};
@@ -203,14 +182,11 @@ class EraseFromParent : public IRChangeBase {
203182
std::unique_ptr<sandboxir::Value> ErasedIPtr;
204183

205184
public:
206-
EraseFromParent(std::unique_ptr<sandboxir::Value> &&IPtr, Tracker &Tracker);
207-
void revert() final;
185+
EraseFromParent(std::unique_ptr<sandboxir::Value> &&IPtr);
186+
void revert(Tracker &Tracker) final;
208187
void accept() final;
209188
#ifndef NDEBUG
210-
void dump(raw_ostream &OS) const final {
211-
dumpCommon(OS);
212-
OS << "EraseFromParent";
213-
}
189+
void dump(raw_ostream &OS) const final { OS << "EraseFromParent"; }
214190
LLVM_DUMP_METHOD void dump() const final;
215191
friend raw_ostream &operator<<(raw_ostream &OS, const EraseFromParent &C) {
216192
C.dump(OS);
@@ -226,15 +202,12 @@ class RemoveFromParent : public IRChangeBase {
226202
PointerUnion<Instruction *, BasicBlock *> NextInstrOrBB;
227203

228204
public:
229-
RemoveFromParent(Instruction *RemovedI, Tracker &Tracker);
230-
void revert() final;
205+
RemoveFromParent(Instruction *RemovedI);
206+
void revert(Tracker &Tracker) final;
231207
void accept() final {};
232208
Instruction *getInstruction() const { return RemovedI; }
233209
#ifndef NDEBUG
234-
void dump(raw_ostream &OS) const final {
235-
dumpCommon(OS);
236-
OS << "RemoveFromParent";
237-
}
210+
void dump(raw_ostream &OS) const final { OS << "RemoveFromParent"; }
238211
LLVM_DUMP_METHOD void dump() const final;
239212
#endif // NDEBUG
240213
};
@@ -265,15 +238,11 @@ class GenericSetter final : public IRChangeBase {
265238
SavedValT OrigVal;
266239

267240
public:
268-
GenericSetter(InstrT *I, Tracker &Tracker)
269-
: IRChangeBase(Tracker), I(I), OrigVal((I->*GetterFn)()) {}
270-
void revert() final { (I->*SetterFn)(OrigVal); }
241+
GenericSetter(InstrT *I) : I(I), OrigVal((I->*GetterFn)()) {}
242+
void revert(Tracker &Tracker) final { (I->*SetterFn)(OrigVal); }
271243
void accept() final {}
272244
#ifndef NDEBUG
273-
void dump(raw_ostream &OS) const final {
274-
dumpCommon(OS);
275-
OS << "GenericSetter";
276-
}
245+
void dump(raw_ostream &OS) const final { OS << "GenericSetter"; }
277246
LLVM_DUMP_METHOD void dump() const final {
278247
dump(dbgs());
279248
dbgs() << "\n";
@@ -287,14 +256,11 @@ class CallBrInstSetIndirectDest : public IRChangeBase {
287256
BasicBlock *OrigIndirectDest;
288257

289258
public:
290-
CallBrInstSetIndirectDest(CallBrInst *CallBr, unsigned Idx, Tracker &Tracker);
291-
void revert() final;
259+
CallBrInstSetIndirectDest(CallBrInst *CallBr, unsigned Idx);
260+
void revert(Tracker &Tracker) final;
292261
void accept() final {}
293262
#ifndef NDEBUG
294-
void dump(raw_ostream &OS) const final {
295-
dumpCommon(OS);
296-
OS << "CallBrInstSetIndirectDest";
297-
}
263+
void dump(raw_ostream &OS) const final { OS << "CallBrInstSetIndirectDest"; }
298264
LLVM_DUMP_METHOD void dump() const final;
299265
#endif
300266
};
@@ -307,14 +273,11 @@ class MoveInstr : public IRChangeBase {
307273
PointerUnion<Instruction *, BasicBlock *> NextInstrOrBB;
308274

309275
public:
310-
MoveInstr(sandboxir::Instruction *I, Tracker &Tracker);
311-
void revert() final;
276+
MoveInstr(sandboxir::Instruction *I);
277+
void revert(Tracker &Tracker) final;
312278
void accept() final {}
313279
#ifndef NDEBUG
314-
void dump(raw_ostream &OS) const final {
315-
dumpCommon(OS);
316-
OS << "MoveInstr";
317-
}
280+
void dump(raw_ostream &OS) const final { OS << "MoveInstr"; }
318281
LLVM_DUMP_METHOD void dump() const final;
319282
#endif // NDEBUG
320283
};
@@ -323,14 +286,11 @@ class InsertIntoBB final : public IRChangeBase {
323286
Instruction *InsertedI = nullptr;
324287

325288
public:
326-
InsertIntoBB(Instruction *InsertedI, Tracker &Tracker);
327-
void revert() final;
289+
InsertIntoBB(Instruction *InsertedI);
290+
void revert(Tracker &Tracker) final;
328291
void accept() final {}
329292
#ifndef NDEBUG
330-
void dump(raw_ostream &OS) const final {
331-
dumpCommon(OS);
332-
OS << "InsertIntoBB";
333-
}
293+
void dump(raw_ostream &OS) const final { OS << "InsertIntoBB"; }
334294
LLVM_DUMP_METHOD void dump() const final;
335295
#endif // NDEBUG
336296
};
@@ -339,15 +299,11 @@ class CreateAndInsertInst final : public IRChangeBase {
339299
Instruction *NewI = nullptr;
340300

341301
public:
342-
CreateAndInsertInst(Instruction *NewI, Tracker &Tracker)
343-
: IRChangeBase(Tracker), NewI(NewI) {}
344-
void revert() final;
302+
CreateAndInsertInst(Instruction *NewI) : NewI(NewI) {}
303+
void revert(Tracker &Tracker) final;
345304
void accept() final {}
346305
#ifndef NDEBUG
347-
void dump(raw_ostream &OS) const final {
348-
dumpCommon(OS);
349-
OS << "CreateAndInsertInst";
350-
}
306+
void dump(raw_ostream &OS) const final { OS << "CreateAndInsertInst"; }
351307
LLVM_DUMP_METHOD void dump() const final;
352308
#endif
353309
};
@@ -364,9 +320,6 @@ class Tracker {
364320
private:
365321
/// The list of changes that are being tracked.
366322
SmallVector<std::unique_ptr<IRChangeBase>> Changes;
367-
#ifndef NDEBUG
368-
friend unsigned IRChangeBase::getIdx() const; // For accessing `Changes`.
369-
#endif
370323
/// The current state of the tracker.
371324
TrackerState State = TrackerState::Disabled;
372325
Context &Ctx;
@@ -383,7 +336,29 @@ class Tracker {
383336
Context &getContext() const { return Ctx; }
384337
/// Record \p Change and take ownership. This is the main function used to
385338
/// track Sandbox IR changes.
386-
void track(std::unique_ptr<IRChangeBase> &&Change);
339+
void track(std::unique_ptr<IRChangeBase> &&Change) {
340+
assert(State == TrackerState::Record && "The tracker should be tracking!");
341+
#ifndef NDEBUG
342+
assert(!InMiddleOfCreatingChange &&
343+
"We are in the middle of creating another change!");
344+
if (isTracking())
345+
InMiddleOfCreatingChange = true;
346+
#endif // NDEBUG
347+
Changes.push_back(std::move(Change));
348+
349+
#ifndef NDEBUG
350+
InMiddleOfCreatingChange = false;
351+
#endif
352+
}
353+
/// A convenience wrapper for `track()` that constructs and tracks the Change
354+
/// object if tracking is enabled. \Returns true if tracking is enabled.
355+
template <typename ChangeT, typename... ArgsT>
356+
bool emplaceIfTracking(ArgsT... Args) {
357+
if (!isTracking())
358+
return false;
359+
track(std::make_unique<ChangeT>(Args...));
360+
return true;
361+
}
387362
/// \Returns true if the tracker is recording changes.
388363
bool isTracking() const { return State == TrackerState::Record; }
389364
/// \Returns the current state of the tracker.

0 commit comments

Comments
 (0)