[llvm] [SandboxIR] Clean up tracking code with the help of tryTrack() (PR #102406)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 9 12:02:48 PDT 2024
https://github.com/vporpo updated https://github.com/llvm/llvm-project/pull/102406
>From a4e9a9f36db514c1dab8d8153d11e1957012e700 Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vporpodas at google.com>
Date: Wed, 7 Aug 2024 16:52:09 -0700
Subject: [PATCH 1/3] [SandboxIR] Clean up tracking code with the help of
tryTrack()
This patch introduces Tracker::tryTrack(), a wrapper of Tracker::track() that
will conditionally create the change object if tracking is enabled.
---
llvm/include/llvm/SandboxIR/SandboxIR.h | 3 +
llvm/include/llvm/SandboxIR/Tracker.h | 17 +++-
llvm/lib/SandboxIR/SandboxIR.cpp | 104 +++++++-----------------
llvm/lib/SandboxIR/Tracker.cpp | 9 --
4 files changed, 50 insertions(+), 83 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 <typename ItTy, typename SBTy> 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..930b78eb236e04 100644
--- a/llvm/include/llvm/SandboxIR/Tracker.h
+++ b/llvm/include/llvm/SandboxIR/Tracker.h
@@ -383,7 +383,22 @@ 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<IRChangeBase> &&Change);
+ void track(std::unique_ptr<IRChangeBase> &&Change) {
+ assert(State == TrackerState::Record && "The tracker should be tracking!");
+ 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.
+ template <typename ChangeT, typename... ArgsT>
+ void tryTrack(ArgsT... ChangeArgs) {
+ if (!isTracking())
+ return;
+ track(std::make_unique<ChangeT>(ChangeArgs..., *this));
+ }
/// \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..2e337f7ba16296 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<UseSet>(*this, Tracker));
+ Ctx->getTracker().tryTrack<UseSet>(*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<UseSwap>(*this, OtherUse, Tracker));
+ Ctx->getTracker().tryTrack<UseSwap>(*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<UseSet>(UseToReplace, Tracker));
+ Ctx.getTracker().tryTrack<UseSet>(UseToReplace);
return true;
});
}
@@ -257,9 +251,7 @@ bool User::classof(const Value *From) {
void User::setOperand(unsigned OperandIdx, Value *Operand) {
assert(isa<llvm::User>(Val) && "No operands!");
- auto &Tracker = Ctx.getTracker();
- if (Tracker.isTracking())
- Tracker.track(std::make_unique<UseSet>(getOperandUse(OperandIdx), Tracker));
+ Ctx.getTracker().tryTrack<UseSet>(getOperandUse(OperandIdx));
// We are delegating to llvm::User::setOperand().
cast<llvm::User>(Val)->setOperand(OperandIdx, Operand->Val);
}
@@ -270,7 +262,7 @@ bool User::replaceUsesOfWith(Value *FromV, Value *ToV) {
for (auto OpIdx : seq<unsigned>(0, getNumOperands())) {
auto Use = getOperandUse(OpIdx);
if (Use.get() == FromV)
- Tracker.track(std::make_unique<UseSet>(Use, Tracker));
+ Tracker.tryTrack<UseSet>(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<RemoveFromParent>(this, Tracker));
+ Ctx.getTracker().tryTrack<RemoveFromParent>(this);
// Detach all the LLVM IR instructions from their parent BB.
for (llvm::Instruction *I : getLLVMInstrs())
@@ -403,9 +393,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<MoveInstr>(this, Tracker));
+ Ctx.getTracker().tryTrack<MoveInstr>(this);
auto *LLVMBB = cast<llvm::BasicBlock>(BB.Val);
llvm::BasicBlock::iterator It;
@@ -431,9 +419,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<InsertIntoBB>(this, Tracker));
+ Ctx.getTracker().tryTrack<InsertIntoBB>(this);
// Insert the LLVM IR Instructions in program order.
for (llvm::Instruction *I : getLLVMInstrs())
@@ -459,9 +445,7 @@ void Instruction::insertInto(BasicBlock *BB, const BBIterator &WhereIt) {
LLVMBeforeIt = LLVMBB->end();
}
- auto &Tracker = Ctx.getTracker();
- if (Tracker.isTracking())
- Tracker.track(std::make_unique<InsertIntoBB>(this, Tracker));
+ Ctx.getTracker().tryTrack<InsertIntoBB>(this);
// Insert the LLVM IR Instructions in program order.
for (llvm::Instruction *I : getLLVMInstrs())
@@ -625,12 +609,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()
+ .tryTrack<GenericSetter<&LoadInst::isVolatile, &LoadInst::setVolatile>>(
+ this);
cast<llvm::LoadInst>(Val)->setVolatile(V);
}
@@ -690,13 +671,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()
+ .tryTrack<GenericSetter<&StoreInst::isVolatile, &StoreInst::setVolatile>>(
+ this);
cast<llvm::StoreInst>(Val)->setVolatile(V);
}
@@ -1048,19 +1025,13 @@ llvm::SmallVector<BasicBlock *, 16> CallBrInst::getIndirectDests() const {
return BBs;
}
void CallBrInst::setDefaultDest(BasicBlock *BB) {
- auto &Tracker = Ctx.getTracker();
- if (Tracker.isTracking()) {
- Tracker.track(std::make_unique<GenericSetter<&CallBrInst::getDefaultDest,
- &CallBrInst::setDefaultDest>>(
- this, Tracker));
- }
+ Ctx.getTracker()
+ .tryTrack<GenericSetter<&CallBrInst::getDefaultDest,
+ &CallBrInst::setDefaultDest>>(this);
cast<llvm::CallBrInst>(Val)->setDefaultDest(cast<llvm::BasicBlock>(BB->Val));
}
void CallBrInst::setIndirectDest(unsigned Idx, BasicBlock *BB) {
- auto &Tracker = Ctx.getTracker();
- if (Tracker.isTracking())
- Tracker.track(
- std::make_unique<CallBrInstSetIndirectDest>(this, Idx, Tracker));
+ Ctx.getTracker().tryTrack<CallBrInstSetIndirectDest>(this, Idx);
cast<llvm::CallBrInst>(Val)->setIndirectDest(Idx,
cast<llvm::BasicBlock>(BB->Val));
}
@@ -1304,35 +1275,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<GenericSetter<&AllocaInst::getAllocatedType,
- &AllocaInst::setAllocatedType>>(
- this, Tracker));
- }
+ Ctx.getTracker()
+ .tryTrack<GenericSetter<&AllocaInst::getAllocatedType,
+ &AllocaInst::setAllocatedType>>(this);
cast<llvm::AllocaInst>(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()
+ .tryTrack<
+ GenericSetter<&AllocaInst::getAlign, &AllocaInst::setAlignment>>(
+ this);
cast<llvm::AllocaInst>(Val)->setAlignment(Align);
}
void AllocaInst::setUsedWithInAlloca(bool V) {
- auto &Tracker = Ctx.getTracker();
- if (Tracker.isTracking()) {
- Tracker.track(
- std::make_unique<GenericSetter<&AllocaInst::isUsedWithInAlloca,
- &AllocaInst::setUsedWithInAlloca>>(
- this, Tracker));
- }
+ Ctx.getTracker()
+ .tryTrack<GenericSetter<&AllocaInst::isUsedWithInAlloca,
+ &AllocaInst::setUsedWithInAlloca>>(this);
cast<llvm::AllocaInst>(Val)->setUsedWithInAlloca(V);
}
@@ -1504,10 +1464,8 @@ Value *Context::registerValue(std::unique_ptr<Value> &&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<Instruction>(VPtr.get()))
- Tracker.track(std::make_unique<CreateAndInsertInst>(I, Tracker));
+ if (auto *I = dyn_cast<Instruction>(VPtr.get()))
+ getTracker().tryTrack<CreateAndInsertInst>(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..b4aea23cfc64d8 100644
--- a/llvm/lib/SandboxIR/Tracker.cpp
+++ b/llvm/lib/SandboxIR/Tracker.cpp
@@ -265,15 +265,6 @@ void CreateAndInsertInst::dump() const {
}
#endif
-void Tracker::track(std::unique_ptr<IRChangeBase> &&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() {
>From 9c5e648bf8a1ce1a0d301f63764add192db95d33 Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vporpodas at google.com>
Date: Thu, 8 Aug 2024 15:08:43 -0700
Subject: [PATCH 2/3] fixup! [SandboxIR] Clean up tracking code with the help
of tryTrack()
---
llvm/include/llvm/SandboxIR/Tracker.h | 7 +++--
llvm/lib/SandboxIR/SandboxIR.cpp | 44 +++++++++++++--------------
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h
index 930b78eb236e04..c95af75cfa5a2b 100644
--- a/llvm/include/llvm/SandboxIR/Tracker.h
+++ b/llvm/include/llvm/SandboxIR/Tracker.h
@@ -392,12 +392,13 @@ class Tracker {
#endif
}
/// A convenience wrapper for `track()` that constructs and tracks the Change
- /// object if tracking is enabled.
+ /// object if tracking is enabled. \Returns true if tracking is enabled.
template <typename ChangeT, typename... ArgsT>
- void tryTrack(ArgsT... ChangeArgs) {
+ bool emplaceIfTracking(ArgsT... ChangeArgs) {
if (!isTracking())
- return;
+ return false;
track(std::make_unique<ChangeT>(ChangeArgs..., *this));
+ return true;
}
/// \Returns true if the tracker is recording changes.
bool isTracking() const { return State == TrackerState::Record; }
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index 2e337f7ba16296..6fddd132fe9932 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -18,14 +18,14 @@ using namespace llvm::sandboxir;
Value *Use::get() const { return Ctx->getValue(LLVMUse->get()); }
void Use::set(Value *V) {
- Ctx->getTracker().tryTrack<UseSet>(*this);
+ Ctx->getTracker().emplaceIfTracking<UseSet>(*this);
LLVMUse->set(V->Val);
}
unsigned Use::getOperandNo() const { return Usr->getUseOperandNo(*this); }
void Use::swap(Use &OtherUse) {
- Ctx->getTracker().tryTrack<UseSwap>(*this, OtherUse);
+ Ctx->getTracker().emplaceIfTracking<UseSwap>(*this, OtherUse);
LLVMUse->swap(*OtherUse.LLVMUse);
}
@@ -148,7 +148,7 @@ void Value::replaceUsesWithIf(
Use UseToReplace(&LLVMUse, DstU, Ctx);
if (!ShouldReplace(UseToReplace))
return false;
- Ctx.getTracker().tryTrack<UseSet>(UseToReplace);
+ Ctx.getTracker().emplaceIfTracking<UseSet>(UseToReplace);
return true;
});
}
@@ -251,7 +251,7 @@ bool User::classof(const Value *From) {
void User::setOperand(unsigned OperandIdx, Value *Operand) {
assert(isa<llvm::User>(Val) && "No operands!");
- Ctx.getTracker().tryTrack<UseSet>(getOperandUse(OperandIdx));
+ Ctx.getTracker().emplaceIfTracking<UseSet>(getOperandUse(OperandIdx));
// We are delegating to llvm::User::setOperand().
cast<llvm::User>(Val)->setOperand(OperandIdx, Operand->Val);
}
@@ -262,7 +262,7 @@ bool User::replaceUsesOfWith(Value *FromV, Value *ToV) {
for (auto OpIdx : seq<unsigned>(0, getNumOperands())) {
auto Use = getOperandUse(OpIdx);
if (Use.get() == FromV)
- Tracker.tryTrack<UseSet>(Use);
+ Tracker.emplaceIfTracking<UseSet>(Use);
}
}
// We are delegating RUOW to LLVM IR's RUOW.
@@ -356,7 +356,7 @@ Instruction *Instruction::getPrevNode() const {
}
void Instruction::removeFromParent() {
- Ctx.getTracker().tryTrack<RemoveFromParent>(this);
+ Ctx.getTracker().emplaceIfTracking<RemoveFromParent>(this);
// Detach all the LLVM IR instructions from their parent BB.
for (llvm::Instruction *I : getLLVMInstrs())
@@ -393,7 +393,7 @@ void Instruction::moveBefore(BasicBlock &BB, const BBIterator &WhereIt) {
// Destination is same as origin, nothing to do.
return;
- Ctx.getTracker().tryTrack<MoveInstr>(this);
+ Ctx.getTracker().emplaceIfTracking<MoveInstr>(this);
auto *LLVMBB = cast<llvm::BasicBlock>(BB.Val);
llvm::BasicBlock::iterator It;
@@ -419,7 +419,7 @@ void Instruction::insertBefore(Instruction *BeforeI) {
[](auto *I1, auto *I2) { return I1->comesBefore(I2); }) &&
"Expected program order!");
- Ctx.getTracker().tryTrack<InsertIntoBB>(this);
+ Ctx.getTracker().emplaceIfTracking<InsertIntoBB>(this);
// Insert the LLVM IR Instructions in program order.
for (llvm::Instruction *I : getLLVMInstrs())
@@ -445,7 +445,7 @@ void Instruction::insertInto(BasicBlock *BB, const BBIterator &WhereIt) {
LLVMBeforeIt = LLVMBB->end();
}
- Ctx.getTracker().tryTrack<InsertIntoBB>(this);
+ Ctx.getTracker().emplaceIfTracking<InsertIntoBB>(this);
// Insert the LLVM IR Instructions in program order.
for (llvm::Instruction *I : getLLVMInstrs())
@@ -610,8 +610,8 @@ void BranchInst::dump() const {
void LoadInst::setVolatile(bool V) {
Ctx.getTracker()
- .tryTrack<GenericSetter<&LoadInst::isVolatile, &LoadInst::setVolatile>>(
- this);
+ .emplaceIfTracking<
+ GenericSetter<&LoadInst::isVolatile, &LoadInst::setVolatile>>(this);
cast<llvm::LoadInst>(Val)->setVolatile(V);
}
@@ -672,8 +672,8 @@ void LoadInst::dump() const {
void StoreInst::setVolatile(bool V) {
Ctx.getTracker()
- .tryTrack<GenericSetter<&StoreInst::isVolatile, &StoreInst::setVolatile>>(
- this);
+ .emplaceIfTracking<
+ GenericSetter<&StoreInst::isVolatile, &StoreInst::setVolatile>>(this);
cast<llvm::StoreInst>(Val)->setVolatile(V);
}
@@ -1026,12 +1026,12 @@ llvm::SmallVector<BasicBlock *, 16> CallBrInst::getIndirectDests() const {
}
void CallBrInst::setDefaultDest(BasicBlock *BB) {
Ctx.getTracker()
- .tryTrack<GenericSetter<&CallBrInst::getDefaultDest,
- &CallBrInst::setDefaultDest>>(this);
+ .emplaceIfTracking<GenericSetter<&CallBrInst::getDefaultDest,
+ &CallBrInst::setDefaultDest>>(this);
cast<llvm::CallBrInst>(Val)->setDefaultDest(cast<llvm::BasicBlock>(BB->Val));
}
void CallBrInst::setIndirectDest(unsigned Idx, BasicBlock *BB) {
- Ctx.getTracker().tryTrack<CallBrInstSetIndirectDest>(this, Idx);
+ Ctx.getTracker().emplaceIfTracking<CallBrInstSetIndirectDest>(this, Idx);
cast<llvm::CallBrInst>(Val)->setIndirectDest(Idx,
cast<llvm::BasicBlock>(BB->Val));
}
@@ -1276,14 +1276,14 @@ AllocaInst *AllocaInst::create(Type *Ty, unsigned AddrSpace,
void AllocaInst::setAllocatedType(Type *Ty) {
Ctx.getTracker()
- .tryTrack<GenericSetter<&AllocaInst::getAllocatedType,
- &AllocaInst::setAllocatedType>>(this);
+ .emplaceIfTracking<GenericSetter<&AllocaInst::getAllocatedType,
+ &AllocaInst::setAllocatedType>>(this);
cast<llvm::AllocaInst>(Val)->setAllocatedType(Ty);
}
void AllocaInst::setAlignment(Align Align) {
Ctx.getTracker()
- .tryTrack<
+ .emplaceIfTracking<
GenericSetter<&AllocaInst::getAlign, &AllocaInst::setAlignment>>(
this);
cast<llvm::AllocaInst>(Val)->setAlignment(Align);
@@ -1291,8 +1291,8 @@ void AllocaInst::setAlignment(Align Align) {
void AllocaInst::setUsedWithInAlloca(bool V) {
Ctx.getTracker()
- .tryTrack<GenericSetter<&AllocaInst::isUsedWithInAlloca,
- &AllocaInst::setUsedWithInAlloca>>(this);
+ .emplaceIfTracking<GenericSetter<&AllocaInst::isUsedWithInAlloca,
+ &AllocaInst::setUsedWithInAlloca>>(this);
cast<llvm::AllocaInst>(Val)->setUsedWithInAlloca(V);
}
@@ -1465,7 +1465,7 @@ Value *Context::registerValue(std::unique_ptr<Value> &&VPtr) {
// meaning that the instructions need to be inserted into a block upon
// creation. This is why the tracker class combines creation and insertion.
if (auto *I = dyn_cast<Instruction>(VPtr.get()))
- getTracker().tryTrack<CreateAndInsertInst>(I);
+ getTracker().emplaceIfTracking<CreateAndInsertInst>(I);
Value *V = VPtr.get();
[[maybe_unused]] auto Pair =
>From 569feab0f415d187b778e2e42354258adf6f550d Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vporpodas at google.com>
Date: Fri, 9 Aug 2024 12:02:05 -0700
Subject: [PATCH 3/3] fixup! fixup! [SandboxIR] Clean up tracking code with the
help of tryTrack()
---
llvm/include/llvm/SandboxIR/Tracker.h | 135 +++++++++-----------------
llvm/lib/SandboxIR/SandboxIR.cpp | 17 ++--
llvm/lib/SandboxIR/Tracker.cpp | 73 +++++---------
3 files changed, 81 insertions(+), 144 deletions(-)
diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h
index c95af75cfa5a2b..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<sandboxir::Value> ErasedIPtr;
public:
- EraseFromParent(std::unique_ptr<sandboxir::Value> &&IPtr, Tracker &Tracker);
- void revert() final;
+ EraseFromParent(std::unique_ptr<sandboxir::Value> &&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<Instruction *, BasicBlock *> 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<Instruction *, BasicBlock *> 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<std::unique_ptr<IRChangeBase>> Changes;
-#ifndef NDEBUG
- friend unsigned IRChangeBase::getIdx() const; // For accessing `Changes`.
-#endif
/// The current state of the tracker.
TrackerState State = TrackerState::Disabled;
Context &Ctx;
@@ -385,6 +338,12 @@ class Tracker {
/// track Sandbox IR changes.
void track(std::unique_ptr<IRChangeBase> &&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
@@ -394,10 +353,10 @@ class Tracker {
/// A convenience wrapper for `track()` that constructs and tracks the Change
/// object if tracking is enabled. \Returns true if tracking is enabled.
template <typename ChangeT, typename... ArgsT>
- bool emplaceIfTracking(ArgsT... ChangeArgs) {
+ bool emplaceIfTracking(ArgsT... Args) {
if (!isTracking())
return false;
- track(std::make_unique<ChangeT>(ChangeArgs..., *this));
+ track(std::make_unique<ChangeT>(Args...));
return true;
}
/// \Returns true if the tracker is recording changes.
diff --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index 6fddd132fe9932..65dc8b5c128827 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -159,7 +159,7 @@ void Value::replaceAllUsesWith(Value *Other) {
auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking()) {
for (auto Use : uses())
- Tracker.track(std::make_unique<UseSet>(Use, Tracker));
+ Tracker.track(std::make_unique<UseSet>(Use));
}
// We are delegating RAUW to LLVM IR's RAUW.
Val->replaceAllUsesWith(Other->Val);
@@ -370,8 +370,7 @@ void Instruction::eraseFromParent() {
auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking()) {
- Tracker.track(
- std::make_unique<EraseFromParent>(std::move(Detached), Tracker));
+ Tracker.track(std::make_unique<EraseFromParent>(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.
@@ -1128,7 +1127,7 @@ void PHINode::setIncomingValue(unsigned Idx, Value *V) {
auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking())
Tracker.track(std::make_unique<PHISetIncoming>(
- *this, Idx, PHISetIncoming::What::Value, Tracker));
+ *this, Idx, PHISetIncoming::What::Value));
cast<llvm::PHINode>(Val)->setIncomingValue(Idx, V->Val);
}
@@ -1145,14 +1144,14 @@ void PHINode::setIncomingBlock(unsigned Idx, BasicBlock *BB) {
auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking())
Tracker.track(std::make_unique<PHISetIncoming>(
- *this, Idx, PHISetIncoming::What::Block, Tracker));
+ *this, Idx, PHISetIncoming::What::Block));
cast<llvm::PHINode>(Val)->setIncomingBlock(Idx,
cast<llvm::BasicBlock>(BB->Val));
}
void PHINode::addIncoming(Value *V, BasicBlock *BB) {
auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking())
- Tracker.track(std::make_unique<PHIAddIncoming>(*this, Tracker));
+ Tracker.track(std::make_unique<PHIAddIncoming>(*this));
cast<llvm::PHINode>(Val)->addIncoming(V->Val,
cast<llvm::BasicBlock>(BB->Val));
@@ -1160,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<PHIRemoveIncoming>(*this, Idx, Tracker));
+ Tracker.track(std::make_unique<PHIRemoveIncoming>(*this, Idx));
llvm::Value *LLVMV =
cast<llvm::PHINode>(Val)->removeIncomingValue(Idx,
/*DeletePHIIfEmpty=*/false);
@@ -1169,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<PHIRemoveIncoming>(
- *this, getBasicBlockIndex(BB), Tracker));
+ Tracker.track(
+ std::make_unique<PHIRemoveIncoming>(*this, getBasicBlockIndex(BB)));
auto *LLVMBB = cast<llvm::BasicBlock>(BB->Val);
llvm::Value *LLVMV =
diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp
index b4aea23cfc64d8..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<Value *>())
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<sandboxir::Value> &&ErasedIPtr,
- Tracker &Tracker)
- : IRChangeBase(Tracker), ErasedIPtr(std::move(ErasedIPtr)) {
+EraseFromParent::EraseFromParent(std::unique_ptr<sandboxir::Value> &&ErasedIPtr)
+ : ErasedIPtr(std::move(ErasedIPtr)) {
auto *I = cast<Instruction>(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<llvm::Instruction *>()) {
@@ -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<Instruction *>()) {
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<Instruction *>()) {
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 {
@@ -271,7 +249,7 @@ 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();
}
@@ -285,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";
}
More information about the llvm-commits
mailing list