[llvm] [SandboxIR] Clean up tracking code with the help of tryTrack() (PR #102406)

via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 7 17:30:50 PDT 2024


https://github.com/vporpo created https://github.com/llvm/llvm-project/pull/102406

This patch introduces Tracker::tryTrack(), a wrapper of Tracker::track() that will conditionally create the change object if tracking is enabled.

>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] [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 12ecd304ce833..152b2add4f8de 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 8f2ac2bca9dec..930b78eb236e0 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 2cb76fc89d9b4..2e337f7ba1629 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 f0651967aae14..b4aea23cfc64d 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() {



More information about the llvm-commits mailing list