[llvm] 8c5d76a - [SandboxIR][Tracker][NFC] GenericSetterWithIdx (#104615)

via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 16 13:16:47 PDT 2024


Author: vporpo
Date: 2024-08-16T13:16:43-07:00
New Revision: 8c5d76ac508ece0b41cfd33247d1f0551c80a9e8

URL: https://github.com/llvm/llvm-project/commit/8c5d76ac508ece0b41cfd33247d1f0551c80a9e8
DIFF: https://github.com/llvm/llvm-project/commit/8c5d76ac508ece0b41cfd33247d1f0551c80a9e8.diff

LOG: [SandboxIR][Tracker][NFC] GenericSetterWithIdx (#104615)

This patch adds a generic change tracker class, similar to
GenericSetter, but for getter/setter functions that also take an index
argument. For example: `Foo:get(Idx)` and `Foo::set(Idx, Val)`. These
setter/getter patterns are common enough that using a common
implementation seems beneficial.

Added: 
    

Modified: 
    llvm/include/llvm/SandboxIR/Tracker.h
    llvm/lib/SandboxIR/SandboxIR.cpp
    llvm/lib/SandboxIR/Tracker.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h
index 80efbf9e8f6e08..2b4dfafa3b0e5d 100644
--- a/llvm/include/llvm/SandboxIR/Tracker.h
+++ b/llvm/include/llvm/SandboxIR/Tracker.h
@@ -96,25 +96,6 @@ class UseSet : public IRChangeBase {
 #endif
 };
 
-class PHISetIncoming : public IRChangeBase {
-  PHINode *PHI;
-  unsigned Idx;
-  PointerUnion<Value *, BasicBlock *> OrigValueOrBB;
-
-public:
-  enum class What {
-    Value,
-    Block,
-  };
-  PHISetIncoming(PHINode *PHI, unsigned Idx, What What);
-  void revert(Tracker &Tracker) final;
-  void accept() final {}
-#ifndef NDEBUG
-  void dump(raw_ostream &OS) const final { OS << "PHISetIncoming"; }
-  LLVM_DUMP_METHOD void dump() const final;
-#endif
-};
-
 class PHIRemoveIncoming : public IRChangeBase {
   PHINode *PHI;
   unsigned RemovedIdx;
@@ -250,18 +231,33 @@ class GenericSetter final : public IRChangeBase {
 #endif
 };
 
-class CallBrInstSetIndirectDest : public IRChangeBase {
-  CallBrInst *CallBr;
+/// Similar to GenericSetter but the setters/getters have an index as their
+/// first argument. This is commont in cases like: getOperand(unsigned Idx)
+template <auto GetterFn, auto SetterFn>
+class GenericSetterWithIdx final : public IRChangeBase {
+  /// Helper for getting the class type from the getter
+  template <typename ClassT, typename RetT>
+  static ClassT getClassTypeFromGetter(RetT (ClassT::*Fn)(unsigned) const);
+  template <typename ClassT, typename RetT>
+  static ClassT getClassTypeFromGetter(RetT (ClassT::*Fn)(unsigned));
+
+  using InstrT = decltype(getClassTypeFromGetter(GetterFn));
+  using SavedValT = std::invoke_result_t<decltype(GetterFn), InstrT, unsigned>;
+  InstrT *I;
+  SavedValT OrigVal;
   unsigned Idx;
-  BasicBlock *OrigIndirectDest;
 
 public:
-  CallBrInstSetIndirectDest(CallBrInst *CallBr, unsigned Idx);
-  void revert(Tracker &Tracker) final;
+  GenericSetterWithIdx(InstrT *I, unsigned Idx)
+      : I(I), OrigVal((I->*GetterFn)(Idx)), Idx(Idx) {}
+  void revert(Tracker &Tracker) final { (I->*SetterFn)(Idx, OrigVal); }
   void accept() final {}
 #ifndef NDEBUG
-  void dump(raw_ostream &OS) const final { OS << "CallBrInstSetIndirectDest"; }
-  LLVM_DUMP_METHOD void dump() const final;
+  void dump(raw_ostream &OS) const final { OS << "GenericSetterWithIdx"; }
+  LLVM_DUMP_METHOD void dump() const final {
+    dump(dbgs());
+    dbgs() << "\n";
+  }
 #endif
 };
 

diff  --git a/llvm/lib/SandboxIR/SandboxIR.cpp b/llvm/lib/SandboxIR/SandboxIR.cpp
index 96c5af807d1001..559d9ae7c00e7a 100644
--- a/llvm/lib/SandboxIR/SandboxIR.cpp
+++ b/llvm/lib/SandboxIR/SandboxIR.cpp
@@ -1031,7 +1031,10 @@ void CallBrInst::setDefaultDest(BasicBlock *BB) {
   cast<llvm::CallBrInst>(Val)->setDefaultDest(cast<llvm::BasicBlock>(BB->Val));
 }
 void CallBrInst::setIndirectDest(unsigned Idx, BasicBlock *BB) {
-  Ctx.getTracker().emplaceIfTracking<CallBrInstSetIndirectDest>(this, Idx);
+  Ctx.getTracker()
+      .emplaceIfTracking<GenericSetterWithIdx<&CallBrInst::getIndirectDest,
+                                              &CallBrInst::setIndirectDest>>(
+          this, Idx);
   cast<llvm::CallBrInst>(Val)->setIndirectDest(Idx,
                                                cast<llvm::BasicBlock>(BB->Val));
 }
@@ -1102,10 +1105,10 @@ Value *PHINode::getIncomingValue(unsigned Idx) const {
   return Ctx.getValue(cast<llvm::PHINode>(Val)->getIncomingValue(Idx));
 }
 void PHINode::setIncomingValue(unsigned Idx, Value *V) {
-  auto &Tracker = Ctx.getTracker();
-  Tracker.emplaceIfTracking<PHISetIncoming>(this, Idx,
-                                            PHISetIncoming::What::Value);
-
+  Ctx.getTracker()
+      .emplaceIfTracking<GenericSetterWithIdx<&PHINode::getIncomingValue,
+                                              &PHINode::setIncomingValue>>(this,
+                                                                           Idx);
   cast<llvm::PHINode>(Val)->setIncomingValue(Idx, V->Val);
 }
 BasicBlock *PHINode::getIncomingBlock(unsigned Idx) const {
@@ -1118,9 +1121,13 @@ BasicBlock *PHINode::getIncomingBlock(const Use &U) const {
   return cast<BasicBlock>(Ctx.getValue(BB));
 }
 void PHINode::setIncomingBlock(unsigned Idx, BasicBlock *BB) {
-  auto &Tracker = Ctx.getTracker();
-  Tracker.emplaceIfTracking<PHISetIncoming>(this, Idx,
-                                            PHISetIncoming::What::Block);
+  // Helper to disambiguate PHINode::getIncomingBlock(unsigned).
+  constexpr BasicBlock *(PHINode::*GetIncomingBlockFn)(unsigned) const =
+      &PHINode::getIncomingBlock;
+  Ctx.getTracker()
+      .emplaceIfTracking<
+          GenericSetterWithIdx<GetIncomingBlockFn, &PHINode::setIncomingBlock>>(
+          this, Idx);
   cast<llvm::PHINode>(Val)->setIncomingBlock(Idx,
                                              cast<llvm::BasicBlock>(BB->Val));
 }

diff  --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp
index 23f4125dba026b..baf218aa41c1cd 100644
--- a/llvm/lib/SandboxIR/Tracker.cpp
+++ b/llvm/lib/SandboxIR/Tracker.cpp
@@ -27,32 +27,6 @@ void UseSwap::dump() const {
 }
 #endif // NDEBUG
 
-PHISetIncoming::PHISetIncoming(PHINode *PHI, unsigned Idx, What What)
-    : PHI(PHI), Idx(Idx) {
-  switch (What) {
-  case What::Value:
-    OrigValueOrBB = PHI->getIncomingValue(Idx);
-    break;
-  case What::Block:
-    OrigValueOrBB = PHI->getIncomingBlock(Idx);
-    break;
-  }
-}
-
-void PHISetIncoming::revert(Tracker &Tracker) {
-  if (auto *V = OrigValueOrBB.dyn_cast<Value *>())
-    PHI->setIncomingValue(Idx, V);
-  else
-    PHI->setIncomingBlock(Idx, OrigValueOrBB.get<BasicBlock *>());
-}
-
-#ifndef NDEBUG
-void PHISetIncoming::dump() const {
-  dump(dbgs());
-  dbgs() << "\n";
-}
-#endif // NDEBUG
-
 PHIRemoveIncoming::PHIRemoveIncoming(PHINode *PHI, unsigned RemovedIdx)
     : PHI(PHI), RemovedIdx(RemovedIdx) {
   RemovedV = PHI->getIncomingValue(RemovedIdx);
@@ -186,21 +160,6 @@ void RemoveFromParent::dump() const {
 }
 #endif
 
-CallBrInstSetIndirectDest::CallBrInstSetIndirectDest(CallBrInst *CallBr,
-                                                     unsigned Idx)
-    : CallBr(CallBr), Idx(Idx) {
-  OrigIndirectDest = CallBr->getIndirectDest(Idx);
-}
-void CallBrInstSetIndirectDest::revert(Tracker &Tracker) {
-  CallBr->setIndirectDest(Idx, OrigIndirectDest);
-}
-#ifndef NDEBUG
-void CallBrInstSetIndirectDest::dump() const {
-  dump(dbgs());
-  dbgs() << "\n";
-}
-#endif
-
 MoveInstr::MoveInstr(Instruction *MovedI) : MovedI(MovedI) {
   if (auto *NextI = MovedI->getNextNode())
     NextInstrOrBB = NextI;


        


More information about the llvm-commits mailing list