From f038ae6155e4a2588fc31659ae84320cc8444550 Mon Sep 17 00:00:00 2001 From: Vasileios Porpodas Date: Wed, 7 Aug 2024 16:52:09 -0700 Subject: [PATCH] [SandboxIR] Clean up tracking code with the help of emplaceIfTracking() 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`. --- llvm/include/llvm/SandboxIR/SandboxIR.h | 3 + llvm/include/llvm/SandboxIR/Tracker.h | 149 ++++++++++-------------- llvm/lib/SandboxIR/SandboxIR.cpp | 121 +++++++------------ llvm/lib/SandboxIR/Tracker.cpp | 82 +++++-------- 4 files changed, 130 insertions(+), 225 deletions(-) diff --git a/llvm/include/llvm/SandboxIR/SandboxIR.h b/llvm/include/llvm/SandboxIR/SandboxIR.h index 12ecd304ce8336..152b2add4f8de7 100644 --- a/llvm/include/llvm/SandboxIR/SandboxIR.h +++ b/llvm/include/llvm/SandboxIR/SandboxIR.h @@ -256,6 +256,9 @@ class Value { template friend class LLVMOpUserItToSBTy; Value(ClassID SubclassID, llvm::Value *Val, Context &Ctx); + /// Disable copies. + Value(const Value &) = delete; + Value &operator=(const Value &) = delete; public: virtual ~Value() = default; diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h index 8f2ac2bca9dec1..dea09f547b0be4 100644 --- a/llvm/include/llvm/SandboxIR/Tracker.h +++ b/llvm/include/llvm/SandboxIR/Tracker.h @@ -63,20 +63,15 @@ class AllocaInst; /// The base class for IR Change classes. class IRChangeBase { protected: - Tracker &Parent; + friend class Tracker; // For Parent. public: - IRChangeBase(Tracker &Parent); /// This runs when changes get reverted. - virtual void revert() = 0; + virtual void revert(Tracker &Tracker) = 0; /// This runs when changes get accepted. virtual void accept() = 0; virtual ~IRChangeBase() = default; #ifndef NDEBUG - /// \Returns the index of this change by iterating over all changes in the - /// tracker. This is only used for debugging. - unsigned getIdx() const; - void dumpCommon(raw_ostream &OS) const { OS << getIdx() << ". "; } virtual void dump(raw_ostream &OS) const = 0; LLVM_DUMP_METHOD virtual void dump() const = 0; friend raw_ostream &operator<<(raw_ostream &OS, const IRChangeBase &C) { @@ -92,15 +87,11 @@ class UseSet : public IRChangeBase { Value *OrigV = nullptr; public: - UseSet(const Use &U, Tracker &Tracker) - : IRChangeBase(Tracker), U(U), OrigV(U.get()) {} - void revert() final { U.set(OrigV); } + UseSet(const Use &U) : U(U), OrigV(U.get()) {} + void revert(Tracker &Tracker) final { U.set(OrigV); } void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "UseSet"; - } + void dump(raw_ostream &OS) const final { OS << "UseSet"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -115,14 +106,11 @@ class PHISetIncoming : public IRChangeBase { Value, Block, }; - PHISetIncoming(PHINode &PHI, unsigned Idx, What What, Tracker &Tracker); - void revert() final; + PHISetIncoming(PHINode &PHI, unsigned Idx, What What); + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "PHISetIncoming"; - } + void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -134,14 +122,11 @@ class PHIRemoveIncoming : public IRChangeBase { BasicBlock *RemovedBB; public: - PHIRemoveIncoming(PHINode &PHI, unsigned RemovedIdx, Tracker &Tracker); - void revert() final; + PHIRemoveIncoming(PHINode &PHI, unsigned RemovedIdx); + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "PHISetIncoming"; - } + void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -151,14 +136,11 @@ class PHIAddIncoming : public IRChangeBase { unsigned Idx; public: - PHIAddIncoming(PHINode &PHI, Tracker &Tracker); - void revert() final; + PHIAddIncoming(PHINode &PHI); + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "PHISetIncoming"; - } + void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -169,17 +151,14 @@ class UseSwap : public IRChangeBase { Use OtherUse; public: - UseSwap(const Use &ThisUse, const Use &OtherUse, Tracker &Tracker) - : IRChangeBase(Tracker), ThisUse(ThisUse), OtherUse(OtherUse) { + UseSwap(const Use &ThisUse, const Use &OtherUse) + : ThisUse(ThisUse), OtherUse(OtherUse) { assert(ThisUse.getUser() == OtherUse.getUser() && "Expected same user!"); } - void revert() final { ThisUse.swap(OtherUse); } + void revert(Tracker &Tracker) final { ThisUse.swap(OtherUse); } void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "UseSwap"; - } + void dump(raw_ostream &OS) const final { OS << "UseSwap"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -203,14 +182,11 @@ class EraseFromParent : public IRChangeBase { std::unique_ptr ErasedIPtr; public: - EraseFromParent(std::unique_ptr &&IPtr, Tracker &Tracker); - void revert() final; + EraseFromParent(std::unique_ptr &&IPtr); + void revert(Tracker &Tracker) final; void accept() final; #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "EraseFromParent"; - } + void dump(raw_ostream &OS) const final { OS << "EraseFromParent"; } LLVM_DUMP_METHOD void dump() const final; friend raw_ostream &operator<<(raw_ostream &OS, const EraseFromParent &C) { C.dump(OS); @@ -226,15 +202,12 @@ class RemoveFromParent : public IRChangeBase { PointerUnion NextInstrOrBB; public: - RemoveFromParent(Instruction *RemovedI, Tracker &Tracker); - void revert() final; + RemoveFromParent(Instruction *RemovedI); + void revert(Tracker &Tracker) final; void accept() final {}; Instruction *getInstruction() const { return RemovedI; } #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "RemoveFromParent"; - } + void dump(raw_ostream &OS) const final { OS << "RemoveFromParent"; } LLVM_DUMP_METHOD void dump() const final; #endif // NDEBUG }; @@ -265,15 +238,11 @@ class GenericSetter final : public IRChangeBase { SavedValT OrigVal; public: - GenericSetter(InstrT *I, Tracker &Tracker) - : IRChangeBase(Tracker), I(I), OrigVal((I->*GetterFn)()) {} - void revert() final { (I->*SetterFn)(OrigVal); } + GenericSetter(InstrT *I) : I(I), OrigVal((I->*GetterFn)()) {} + void revert(Tracker &Tracker) final { (I->*SetterFn)(OrigVal); } void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "GenericSetter"; - } + void dump(raw_ostream &OS) const final { OS << "GenericSetter"; } LLVM_DUMP_METHOD void dump() const final { dump(dbgs()); dbgs() << "\n"; @@ -287,14 +256,11 @@ class CallBrInstSetIndirectDest : public IRChangeBase { BasicBlock *OrigIndirectDest; public: - CallBrInstSetIndirectDest(CallBrInst *CallBr, unsigned Idx, Tracker &Tracker); - void revert() final; + CallBrInstSetIndirectDest(CallBrInst *CallBr, unsigned Idx); + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "CallBrInstSetIndirectDest"; - } + void dump(raw_ostream &OS) const final { OS << "CallBrInstSetIndirectDest"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -307,14 +273,11 @@ class MoveInstr : public IRChangeBase { PointerUnion NextInstrOrBB; public: - MoveInstr(sandboxir::Instruction *I, Tracker &Tracker); - void revert() final; + MoveInstr(sandboxir::Instruction *I); + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "MoveInstr"; - } + void dump(raw_ostream &OS) const final { OS << "MoveInstr"; } LLVM_DUMP_METHOD void dump() const final; #endif // NDEBUG }; @@ -323,14 +286,11 @@ class InsertIntoBB final : public IRChangeBase { Instruction *InsertedI = nullptr; public: - InsertIntoBB(Instruction *InsertedI, Tracker &Tracker); - void revert() final; + InsertIntoBB(Instruction *InsertedI); + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "InsertIntoBB"; - } + void dump(raw_ostream &OS) const final { OS << "InsertIntoBB"; } LLVM_DUMP_METHOD void dump() const final; #endif // NDEBUG }; @@ -339,15 +299,11 @@ class CreateAndInsertInst final : public IRChangeBase { Instruction *NewI = nullptr; public: - CreateAndInsertInst(Instruction *NewI, Tracker &Tracker) - : IRChangeBase(Tracker), NewI(NewI) {} - void revert() final; + CreateAndInsertInst(Instruction *NewI) : NewI(NewI) {} + void revert(Tracker &Tracker) final; void accept() final {} #ifndef NDEBUG - void dump(raw_ostream &OS) const final { - dumpCommon(OS); - OS << "CreateAndInsertInst"; - } + void dump(raw_ostream &OS) const final { OS << "CreateAndInsertInst"; } LLVM_DUMP_METHOD void dump() const final; #endif }; @@ -364,9 +320,6 @@ class Tracker { private: /// The list of changes that are being tracked. SmallVector> Changes; -#ifndef NDEBUG - friend unsigned IRChangeBase::getIdx() const; // For accessing `Changes`. -#endif /// The current state of the tracker. TrackerState State = TrackerState::Disabled; Context &Ctx; @@ -383,7 +336,29 @@ class Tracker { Context &getContext() const { return Ctx; } /// Record \p Change and take ownership. This is the main function used to /// track Sandbox IR changes. - void track(std::unique_ptr &&Change); + void track(std::unique_ptr &&Change) { + assert(State == TrackerState::Record && "The tracker should be tracking!"); +#ifndef NDEBUG + assert(!InMiddleOfCreatingChange && + "We are in the middle of creating another change!"); + if (isTracking()) + InMiddleOfCreatingChange = true; +#endif // NDEBUG + Changes.push_back(std::move(Change)); + +#ifndef NDEBUG + InMiddleOfCreatingChange = false; +#endif + } + /// A convenience wrapper for `track()` that constructs and tracks the Change + /// object if tracking is enabled. \Returns true if tracking is enabled. + template + bool emplaceIfTracking(ArgsT... Args) { + if (!isTracking()) + return false; + track(std::make_unique(Args...)); + return true; + } /// \Returns true if the tracker is recording changes. bool isTracking() const { return State == TrackerState::Record; } /// \Returns the current state of the tracker. diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp index 2cb76fc89d9b4d..65dc8b5c128827 100644 --- a/llvm/lib/SandboxIR/SandboxIR.cpp +++ b/llvm/lib/SandboxIR/SandboxIR.cpp @@ -18,18 +18,14 @@ using namespace llvm::sandboxir; Value *Use::get() const { return Ctx->getValue(LLVMUse->get()); } void Use::set(Value *V) { - auto &Tracker = Ctx->getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(*this, Tracker)); + Ctx->getTracker().emplaceIfTracking(*this); LLVMUse->set(V->Val); } unsigned Use::getOperandNo() const { return Usr->getUseOperandNo(*this); } void Use::swap(Use &OtherUse) { - auto &Tracker = Ctx->getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(*this, OtherUse, Tracker)); + Ctx->getTracker().emplaceIfTracking(*this, OtherUse); LLVMUse->swap(*OtherUse.LLVMUse); } @@ -152,9 +148,7 @@ void Value::replaceUsesWithIf( Use UseToReplace(&LLVMUse, DstU, Ctx); if (!ShouldReplace(UseToReplace)) return false; - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(UseToReplace, Tracker)); + Ctx.getTracker().emplaceIfTracking(UseToReplace); return true; }); } @@ -165,7 +159,7 @@ void Value::replaceAllUsesWith(Value *Other) { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) { for (auto Use : uses()) - Tracker.track(std::make_unique(Use, Tracker)); + Tracker.track(std::make_unique(Use)); } // We are delegating RAUW to LLVM IR's RAUW. Val->replaceAllUsesWith(Other->Val); @@ -257,9 +251,7 @@ bool User::classof(const Value *From) { void User::setOperand(unsigned OperandIdx, Value *Operand) { assert(isa(Val) && "No operands!"); - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(getOperandUse(OperandIdx), Tracker)); + Ctx.getTracker().emplaceIfTracking(getOperandUse(OperandIdx)); // We are delegating to llvm::User::setOperand(). cast(Val)->setOperand(OperandIdx, Operand->Val); } @@ -270,7 +262,7 @@ bool User::replaceUsesOfWith(Value *FromV, Value *ToV) { for (auto OpIdx : seq(0, getNumOperands())) { auto Use = getOperandUse(OpIdx); if (Use.get() == FromV) - Tracker.track(std::make_unique(Use, Tracker)); + Tracker.emplaceIfTracking(Use); } } // We are delegating RUOW to LLVM IR's RUOW. @@ -364,9 +356,7 @@ Instruction *Instruction::getPrevNode() const { } void Instruction::removeFromParent() { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(this, Tracker)); + Ctx.getTracker().emplaceIfTracking(this); // Detach all the LLVM IR instructions from their parent BB. for (llvm::Instruction *I : getLLVMInstrs()) @@ -380,8 +370,7 @@ void Instruction::eraseFromParent() { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) { - Tracker.track( - std::make_unique(std::move(Detached), Tracker)); + Tracker.track(std::make_unique(std::move(Detached))); // We don't actually delete the IR instruction, because then it would be // impossible to bring it back from the dead at the same memory location. // Instead we remove it from its BB and track its current location. @@ -403,9 +392,7 @@ void Instruction::moveBefore(BasicBlock &BB, const BBIterator &WhereIt) { // Destination is same as origin, nothing to do. return; - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(this, Tracker)); + Ctx.getTracker().emplaceIfTracking(this); auto *LLVMBB = cast(BB.Val); llvm::BasicBlock::iterator It; @@ -431,9 +418,7 @@ void Instruction::insertBefore(Instruction *BeforeI) { [](auto *I1, auto *I2) { return I1->comesBefore(I2); }) && "Expected program order!"); - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(this, Tracker)); + Ctx.getTracker().emplaceIfTracking(this); // Insert the LLVM IR Instructions in program order. for (llvm::Instruction *I : getLLVMInstrs()) @@ -459,9 +444,7 @@ void Instruction::insertInto(BasicBlock *BB, const BBIterator &WhereIt) { LLVMBeforeIt = LLVMBB->end(); } - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track(std::make_unique(this, Tracker)); + Ctx.getTracker().emplaceIfTracking(this); // Insert the LLVM IR Instructions in program order. for (llvm::Instruction *I : getLLVMInstrs()) @@ -625,12 +608,9 @@ void BranchInst::dump() const { #endif // NDEBUG void LoadInst::setVolatile(bool V) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) { - Tracker.track(std::make_unique< - GenericSetter<&LoadInst::isVolatile, &LoadInst::setVolatile>>( - this, Tracker)); - } + Ctx.getTracker() + .emplaceIfTracking< + GenericSetter<&LoadInst::isVolatile, &LoadInst::setVolatile>>(this); cast(Val)->setVolatile(V); } @@ -690,13 +670,9 @@ void LoadInst::dump() const { #endif // NDEBUG void StoreInst::setVolatile(bool V) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) { - Tracker.track( - std::make_unique< - GenericSetter<&StoreInst::isVolatile, &StoreInst::setVolatile>>( - this, Tracker)); - } + Ctx.getTracker() + .emplaceIfTracking< + GenericSetter<&StoreInst::isVolatile, &StoreInst::setVolatile>>(this); cast(Val)->setVolatile(V); } @@ -1048,19 +1024,13 @@ llvm::SmallVector CallBrInst::getIndirectDests() const { return BBs; } void CallBrInst::setDefaultDest(BasicBlock *BB) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) { - Tracker.track(std::make_unique>( - this, Tracker)); - } + Ctx.getTracker() + .emplaceIfTracking>(this); cast(Val)->setDefaultDest(cast(BB->Val)); } void CallBrInst::setIndirectDest(unsigned Idx, BasicBlock *BB) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) - Tracker.track( - std::make_unique(this, Idx, Tracker)); + Ctx.getTracker().emplaceIfTracking(this, Idx); cast(Val)->setIndirectDest(Idx, cast(BB->Val)); } @@ -1157,7 +1127,7 @@ void PHINode::setIncomingValue(unsigned Idx, Value *V) { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) Tracker.track(std::make_unique( - *this, Idx, PHISetIncoming::What::Value, Tracker)); + *this, Idx, PHISetIncoming::What::Value)); cast(Val)->setIncomingValue(Idx, V->Val); } @@ -1174,14 +1144,14 @@ void PHINode::setIncomingBlock(unsigned Idx, BasicBlock *BB) { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) Tracker.track(std::make_unique( - *this, Idx, PHISetIncoming::What::Block, Tracker)); + *this, Idx, PHISetIncoming::What::Block)); cast(Val)->setIncomingBlock(Idx, cast(BB->Val)); } void PHINode::addIncoming(Value *V, BasicBlock *BB) { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) - Tracker.track(std::make_unique(*this, Tracker)); + Tracker.track(std::make_unique(*this)); cast(Val)->addIncoming(V->Val, cast(BB->Val)); @@ -1189,7 +1159,7 @@ void PHINode::addIncoming(Value *V, BasicBlock *BB) { Value *PHINode::removeIncomingValue(unsigned Idx) { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) - Tracker.track(std::make_unique(*this, Idx, Tracker)); + Tracker.track(std::make_unique(*this, Idx)); llvm::Value *LLVMV = cast(Val)->removeIncomingValue(Idx, /*DeletePHIIfEmpty=*/false); @@ -1198,8 +1168,8 @@ Value *PHINode::removeIncomingValue(unsigned Idx) { Value *PHINode::removeIncomingValue(BasicBlock *BB) { auto &Tracker = Ctx.getTracker(); if (Tracker.isTracking()) - Tracker.track(std::make_unique( - *this, getBasicBlockIndex(BB), Tracker)); + Tracker.track( + std::make_unique(*this, getBasicBlockIndex(BB))); auto *LLVMBB = cast(BB->Val); llvm::Value *LLVMV = @@ -1304,35 +1274,24 @@ AllocaInst *AllocaInst::create(Type *Ty, unsigned AddrSpace, } void AllocaInst::setAllocatedType(Type *Ty) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) { - Tracker.track( - std::make_unique>( - this, Tracker)); - } + Ctx.getTracker() + .emplaceIfTracking>(this); cast(Val)->setAllocatedType(Ty); } void AllocaInst::setAlignment(Align Align) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) { - Tracker.track( - std::make_unique< - GenericSetter<&AllocaInst::getAlign, &AllocaInst::setAlignment>>( - this, Tracker)); - } + Ctx.getTracker() + .emplaceIfTracking< + GenericSetter<&AllocaInst::getAlign, &AllocaInst::setAlignment>>( + this); cast(Val)->setAlignment(Align); } void AllocaInst::setUsedWithInAlloca(bool V) { - auto &Tracker = Ctx.getTracker(); - if (Tracker.isTracking()) { - Tracker.track( - std::make_unique>( - this, Tracker)); - } + Ctx.getTracker() + .emplaceIfTracking>(this); cast(Val)->setUsedWithInAlloca(V); } @@ -1504,10 +1463,8 @@ Value *Context::registerValue(std::unique_ptr &&VPtr) { // Please note that we don't allow the creation of detached instructions, // meaning that the instructions need to be inserted into a block upon // creation. This is why the tracker class combines creation and insertion. - auto &Tracker = getTracker(); - if (Tracker.isTracking()) - if (auto *I = dyn_cast(VPtr.get())) - Tracker.track(std::make_unique(I, Tracker)); + if (auto *I = dyn_cast(VPtr.get())) + getTracker().emplaceIfTracking(I); Value *V = VPtr.get(); [[maybe_unused]] auto Pair = diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp index f0651967aae146..44fe0b7a212e03 100644 --- a/llvm/lib/SandboxIR/Tracker.cpp +++ b/llvm/lib/SandboxIR/Tracker.cpp @@ -15,22 +15,7 @@ using namespace llvm::sandboxir; -IRChangeBase::IRChangeBase(Tracker &Parent) : Parent(Parent) { #ifndef NDEBUG - assert(!Parent.InMiddleOfCreatingChange && - "We are in the middle of creating another change!"); - if (Parent.isTracking()) - Parent.InMiddleOfCreatingChange = true; -#endif // NDEBUG -} - -#ifndef NDEBUG -unsigned IRChangeBase::getIdx() const { - auto It = - find_if(Parent.Changes, [this](auto &Ptr) { return Ptr.get() == this; }); - return It - Parent.Changes.begin(); -} - void UseSet::dump() const { dump(dbgs()); dbgs() << "\n"; @@ -42,9 +27,8 @@ void UseSwap::dump() const { } #endif // NDEBUG -PHISetIncoming::PHISetIncoming(PHINode &PHI, unsigned Idx, What What, - Tracker &Tracker) - : IRChangeBase(Tracker), PHI(PHI), Idx(Idx) { +PHISetIncoming::PHISetIncoming(PHINode &PHI, unsigned Idx, What What) + : PHI(PHI), Idx(Idx) { switch (What) { case What::Value: OrigValueOrBB = PHI.getIncomingValue(Idx); @@ -55,7 +39,7 @@ PHISetIncoming::PHISetIncoming(PHINode &PHI, unsigned Idx, What What, } } -void PHISetIncoming::revert() { +void PHISetIncoming::revert(Tracker &Tracker) { if (auto *V = OrigValueOrBB.dyn_cast()) PHI.setIncomingValue(Idx, V); else @@ -69,14 +53,13 @@ void PHISetIncoming::dump() const { } #endif // NDEBUG -PHIRemoveIncoming::PHIRemoveIncoming(PHINode &PHI, unsigned RemovedIdx, - Tracker &Tracker) - : IRChangeBase(Tracker), PHI(PHI), RemovedIdx(RemovedIdx) { +PHIRemoveIncoming::PHIRemoveIncoming(PHINode &PHI, unsigned RemovedIdx) + : PHI(PHI), RemovedIdx(RemovedIdx) { RemovedV = PHI.getIncomingValue(RemovedIdx); RemovedBB = PHI.getIncomingBlock(RemovedIdx); } -void PHIRemoveIncoming::revert() { +void PHIRemoveIncoming::revert(Tracker &Tracker) { // Special case: if the PHI is now empty, as we don't need to care about the // order of the incoming values. unsigned NumIncoming = PHI.getNumIncomingValues(); @@ -105,10 +88,10 @@ void PHIRemoveIncoming::dump() const { } #endif // NDEBUG -PHIAddIncoming::PHIAddIncoming(PHINode &PHI, Tracker &Tracker) - : IRChangeBase(Tracker), PHI(PHI), Idx(PHI.getNumIncomingValues()) {} +PHIAddIncoming::PHIAddIncoming(PHINode &PHI) + : PHI(PHI), Idx(PHI.getNumIncomingValues()) {} -void PHIAddIncoming::revert() { PHI.removeIncomingValue(Idx); } +void PHIAddIncoming::revert(Tracker &Tracker) { PHI.removeIncomingValue(Idx); } #ifndef NDEBUG void PHIAddIncoming::dump() const { @@ -121,9 +104,8 @@ Tracker::~Tracker() { assert(Changes.empty() && "You must accept or revert changes!"); } -EraseFromParent::EraseFromParent(std::unique_ptr &&ErasedIPtr, - Tracker &Tracker) - : IRChangeBase(Tracker), ErasedIPtr(std::move(ErasedIPtr)) { +EraseFromParent::EraseFromParent(std::unique_ptr &&ErasedIPtr) + : ErasedIPtr(std::move(ErasedIPtr)) { auto *I = cast(this->ErasedIPtr.get()); auto LLVMInstrs = I->getLLVMInstrs(); // Iterate in reverse program order. @@ -151,7 +133,7 @@ void EraseFromParent::accept() { IData.LLVMI->deleteValue(); } -void EraseFromParent::revert() { +void EraseFromParent::revert(Tracker &Tracker) { // Place the bottom-most instruction first. auto [Operands, BotLLVMI] = InstrData[0]; if (auto *NextLLVMI = NextLLVMIOrBB.dyn_cast()) { @@ -170,7 +152,7 @@ void EraseFromParent::revert() { LLVMI->setOperand(OpNum, Op); BotLLVMI = LLVMI; } - Parent.getContext().registerValue(std::move(ErasedIPtr)); + Tracker.getContext().registerValue(std::move(ErasedIPtr)); } #ifndef NDEBUG @@ -180,15 +162,14 @@ void EraseFromParent::dump() const { } #endif // NDEBUG -RemoveFromParent::RemoveFromParent(Instruction *RemovedI, Tracker &Tracker) - : IRChangeBase(Tracker), RemovedI(RemovedI) { +RemoveFromParent::RemoveFromParent(Instruction *RemovedI) : RemovedI(RemovedI) { if (auto *NextI = RemovedI->getNextNode()) NextInstrOrBB = NextI; else NextInstrOrBB = RemovedI->getParent(); } -void RemoveFromParent::revert() { +void RemoveFromParent::revert(Tracker &Tracker) { if (auto *NextI = NextInstrOrBB.dyn_cast()) { RemovedI->insertBefore(NextI); } else { @@ -205,12 +186,11 @@ void RemoveFromParent::dump() const { #endif CallBrInstSetIndirectDest::CallBrInstSetIndirectDest(CallBrInst *CallBr, - unsigned Idx, - Tracker &Tracker) - : IRChangeBase(Tracker), CallBr(CallBr), Idx(Idx) { + unsigned Idx) + : CallBr(CallBr), Idx(Idx) { OrigIndirectDest = CallBr->getIndirectDest(Idx); } -void CallBrInstSetIndirectDest::revert() { +void CallBrInstSetIndirectDest::revert(Tracker &Tracker) { CallBr->setIndirectDest(Idx, OrigIndirectDest); } #ifndef NDEBUG @@ -220,15 +200,14 @@ void CallBrInstSetIndirectDest::dump() const { } #endif -MoveInstr::MoveInstr(Instruction *MovedI, Tracker &Tracker) - : IRChangeBase(Tracker), MovedI(MovedI) { +MoveInstr::MoveInstr(Instruction *MovedI) : MovedI(MovedI) { if (auto *NextI = MovedI->getNextNode()) NextInstrOrBB = NextI; else NextInstrOrBB = MovedI->getParent(); } -void MoveInstr::revert() { +void MoveInstr::revert(Tracker &Tracker) { if (auto *NextI = NextInstrOrBB.dyn_cast()) { MovedI->moveBefore(NextI); } else { @@ -244,10 +223,9 @@ void MoveInstr::dump() const { } #endif -void InsertIntoBB::revert() { InsertedI->removeFromParent(); } +void InsertIntoBB::revert(Tracker &Tracker) { InsertedI->removeFromParent(); } -InsertIntoBB::InsertIntoBB(Instruction *InsertedI, Tracker &Tracker) - : IRChangeBase(Tracker), InsertedI(InsertedI) {} +InsertIntoBB::InsertIntoBB(Instruction *InsertedI) : InsertedI(InsertedI) {} #ifndef NDEBUG void InsertIntoBB::dump() const { @@ -256,7 +234,7 @@ void InsertIntoBB::dump() const { } #endif -void CreateAndInsertInst::revert() { NewI->eraseFromParent(); } +void CreateAndInsertInst::revert(Tracker &Tracker) { NewI->eraseFromParent(); } #ifndef NDEBUG void CreateAndInsertInst::dump() const { @@ -265,22 +243,13 @@ void CreateAndInsertInst::dump() const { } #endif -void Tracker::track(std::unique_ptr &&Change) { - assert(State == TrackerState::Record && "The tracker should be tracking!"); - Changes.push_back(std::move(Change)); - -#ifndef NDEBUG - InMiddleOfCreatingChange = false; -#endif -} - void Tracker::save() { State = TrackerState::Record; } void Tracker::revert() { assert(State == TrackerState::Record && "Forgot to save()!"); State = TrackerState::Disabled; for (auto &Change : reverse(Changes)) - Change->revert(); + Change->revert(*this); Changes.clear(); } @@ -294,7 +263,8 @@ void Tracker::accept() { #ifndef NDEBUG void Tracker::dump(raw_ostream &OS) const { - for (const auto &ChangePtr : Changes) { + for (auto [Idx, ChangePtr] : enumerate(Changes)) { + OS << Idx << ". "; ChangePtr->dump(OS); OS << "\n"; }