Skip to content

Commit

Permalink
[SandboxIR] Clean up tracking code with the help of tryTrack()
Browse files Browse the repository at this point in the history 
This patch introduces Tracker::tryTrack(), a wrapper of Tracker::track() that
will conditionally create the change object if tracking is enabled.
  • Loading branch information
@vporpo
vporpo committed Aug 8, 2024
1 parent 4fe33d0 commit a4e9a9f
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 83 deletions.
3 changes: 3 additions & 0 deletions llvm/include/llvm/SandboxIR/SandboxIR.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 16 additions & 1 deletion llvm/include/llvm/SandboxIR/Tracker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
104 changes: 31 additions & 73 deletions llvm/lib/SandboxIR/SandboxIR.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
});
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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.
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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;
Expand All @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 =
Expand Down
9 changes: 0 additions & 9 deletions llvm/lib/SandboxIR/Tracker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit a4e9a9f

Please sign in to comment.