[llvm-branch-commits] [llvm] [GVN] More restructuring of `GVN.h` to reduce its size (NFC) (PR #211541)

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Jul 23 05:57:24 PDT 2026


llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Momchil Velikov (momchil-velikov)

<details>
<summary>Changes</summary>

* `GVNPass` left as an interface for the pass manager. Actuall `GVNPass` moved under the name `GVNPassImpl` to `GVN.cpp`.
* Various helper types moved out of `GVNPassImpl` and into an anonymous namespace in `GVN.cpp`

---

Patch is 67.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/211541.diff


2 Files Affected:

- (modified) llvm/include/llvm/Transforms/Scalar/GVN.h (+9-181) 
- (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+647-429) 


``````````diff
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index f5816313bf214..08af9e2f8eeae 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -123,45 +123,18 @@ struct GVNOptions {
 ///
 /// FIXME: We should have a good summary of the GVN algorithm implemented by
 /// this particular pass here.
+class GVNPassImpl;
 class GVNPass : public OptionalPassInfoMixin<GVNPass> {
-public:
-  struct AvailableValue;
-  struct AvailableValueInBlock;
-  struct ReachingMemVal;
-  struct DependencyBlockInfo;
-
-  friend class GVNValueTable;
-  friend class GVNLegacyPass;
-
-private:
-  GVNOptions Options;
-  MemoryDependenceResults *MD = nullptr;
-  DominatorTree *DT = nullptr;
-  const TargetLibraryInfo *TLI = nullptr;
-  AssumptionCache *AC = nullptr;
-  SetVector<BasicBlock *> DeadBlocks;
-  OptimizationRemarkEmitter *ORE = nullptr;
-  ImplicitControlFlowTracking *ICF = nullptr;
-  LoopInfo *LI = nullptr;
-  AAResults *AA = nullptr;
-  MemorySSAUpdater *MSSAU = nullptr;
-  GVNValueTable VN;
-  GVNLeaderMap LeaderTable;
-
-  // Map the block to reversed postorder traversal number. It is used to
-  // find back edge easily.
-  DenseMap<AssertingVH<BasicBlock>, uint32_t> BlockRPONumber;
-
-  // This is set 'true' initially and also when new blocks have been added to
-  // the function being analyzed. This boolean is used to control the updating
-  // of BlockRPONumber prior to accessing the contents of BlockRPONumber.
-  bool InvalidBlockRPONumbers = true;
-
-  // List of critical edges to be split between iterations.
-  SmallVector<std::pair<Instruction *, unsigned>, 4> ToSplit;
+  std::unique_ptr<GVNPassImpl> Impl;
 
 public:
-  GVNPass(GVNOptions Options = {}) : Options(Options) {}
+  GVNPass(GVNOptions Options = {});
+  ~GVNPass();
+
+  GVNPass(const GVNPass &) = delete;
+  GVNPass(GVNPass &&);
+  GVNPass &operator=(const GVNPass &) = delete;
+  GVNPass &operator=(GVNPass &&);
 
   /// Run the pass over the function.
   LLVM_ABI PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
@@ -169,151 +142,6 @@ class GVNPass : public OptionalPassInfoMixin<GVNPass> {
   LLVM_ABI void
   printPipeline(raw_ostream &OS,
                 function_ref<StringRef(StringRef)> MapClassName2PassName);
-
-private:
-  DominatorTree &getDominatorTree() const { return *DT; }
-  AAResults *getAliasAnalysis() const { return VN.getAliasAnalysis(); }
-  MemoryDependenceResults &getMemDep() const { return *MD; }
-
-  bool isScalarPREEnabled() const;
-  bool isLoadPREEnabled() const;
-  bool isLoadInLoopPREEnabled() const;
-  bool isLoadPRESplitBackedgeEnabled() const;
-  bool isMemDepEnabled() const;
-  bool isMemorySSAEnabled() const;
-
-  using LoadDepVect = SmallVector<NonLocalDepResult, 64>;
-  using AvailValInBlkVect = SmallVector<AvailableValueInBlock, 64>;
-  using UnavailBlkVect = SmallVector<BasicBlock *, 64>;
-
-  using DependencyBlockSet = DenseMap<BasicBlock *, DependencyBlockInfo>;
-
-  /// Given a select-dependency for the load (the load address is a select of
-  /// \p TrueAddr and \p FalseAddr guarded by \p Cond), determine whether a
-  /// value is available by finding dominating values for both addresses.  If
-  /// so, the load can be rematerialized as a select of those two values.
-  std::optional<AvailableValue>
-  analyzeSelectAvailability(LoadInst *Load, Value *Cond, Value *TrueAddr,
-                            Value *FalseAddr, Instruction *From);
-
-  /// Given a local dependency (Def or Clobber) determine if a value is
-  /// available for the load.
-  std::optional<AvailableValue>
-  analyzeLoadAvailability(LoadInst *Load, const ReachingMemVal &Dep,
-                          Value *Address);
-
-  /// Given a list of non-local dependencies, determine if a value is
-  /// available for the load in each specified block.  If it is, add it to
-  /// ValuesPerBlock.  If not, add it to UnavailableBlocks.
-  void analyzeLoadAvailability(LoadInst *Load,
-                               SmallVectorImpl<ReachingMemVal> &Deps,
-                               AvailValInBlkVect &ValuesPerBlock,
-                               UnavailBlkVect &UnavailableBlocks);
-
-  /// Given a critical edge from Pred to LoadBB, find a load instruction
-  /// which is identical to Load from another successor of Pred.
-  LoadInst *findLoadToHoistIntoPred(BasicBlock *Pred, BasicBlock *LoadBB,
-                                    LoadInst *Load);
-
-  /// Eliminates partially redundant \p Load, replacing it with \p
-  /// AvailableLoads (connected by Phis if needed).
-  void eliminatePartiallyRedundantLoad(
-      LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
-      MapVector<BasicBlock *, Value *> &AvailableLoads,
-      MapVector<BasicBlock *, LoadInst *> *CriticalEdgePredAndLoad);
-
-  // Helper functions for d etermining load dependencies.
-  std::optional<GVNPass::ReachingMemVal> scanMemoryAccessesUsers(
-      const MemoryLocation &Loc, bool IsInvariantLoad, BasicBlock *BB,
-      const SmallVectorImpl<MemoryAccess *> &ClobbersList, MemorySSA &MSSA,
-      BatchAAResults &AA, LoadInst *L = nullptr);
-
-  std::optional<GVNPass::ReachingMemVal>
-  accessMayModifyLocation(MemoryAccess *ClobberMA, const MemoryLocation &Loc,
-                          bool IsInvariantLoad, BasicBlock *BB, MemorySSA &MSSA,
-                          BatchAAResults &AA);
-
-  bool collectPredecessors(BasicBlock *BB, const PHITransAddr &Addr,
-                           MemoryAccess *ClobberMA, DependencyBlockSet &Blocks,
-                           SmallVectorImpl<BasicBlock *> &Worklist);
-
-  void collectClobberList(SmallVectorImpl<MemoryAccess *> &Clobbers,
-                          BasicBlock *BB, const DependencyBlockInfo &StartInfo,
-                          const DependencyBlockSet &Blocks, MemorySSA &MSSA);
-
-  bool findReachingValuesForLoad(LoadInst *Inst,
-                                 SmallVectorImpl<ReachingMemVal> &Values,
-                                 MemorySSA &MSSA, AAResults &AA);
-
-  bool performLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
-                      UnavailBlkVect &UnavailableBlocks);
-
-  /// Try to replace a load which executes on each loop iteraiton with Phi
-  /// translation of load in preheader and load(s) in conditionally executed
-  /// paths.
-  bool performLoopLoadPRE(LoadInst *Load, AvailValInBlkVect &ValuesPerBlock,
-                          UnavailBlkVect &UnavailableBlocks);
-
-  // Try to eliminate redundent loades with non-local dependencies.
-  bool processNonLocalLoad(LoadInst *L);
-  bool processNonLocalLoad(LoadInst *L, SmallVectorImpl<ReachingMemVal> &Deps);
-
-  /// Add any blocks determined to be unreachable by a conditional branch with a
-  /// constant condition to the dead blocks.
-  bool processFoldableCondBr(CondBrInst *BI);
-
-  /// Propagate equalities derived from llvm.assume intrinsics.
-  bool processAssumeIntrinsic(AssumeInst *II);
-
-  /// Try to eliminate redundant loads.
-  bool processLoad(LoadInst *L);
-
-  /// Try to eliminate masked loads which have loaded from
-  /// masked stores with the same mask.
-  bool processMaskedLoad(IntrinsicInst *I);
-
-  /// Propagate value of a condition to blocks dominated by "then" and "else"
-  /// edges, as well as certains derived equalities.
-  bool
-  propagateEquality(Value *LHS, Value *RHS,
-                    const std::variant<BasicBlockEdge, Instruction *> &Root);
-
-  // Pass iteration helper functions.
-  bool processInstruction(Instruction *I);
-  bool processBlock(BasicBlock *BB);
-  bool iterateOnFunction(Function &F);
-
-  // Scalar PRE helper functions
-  bool performScalarPREInsertion(Instruction *Instr, BasicBlock *Pred,
-                                 BasicBlock *Curr, unsigned int ValNo);
-  bool performScalarPRE(Instruction *I);
-  bool performPRE(Function &F);
-
-  /// Main entry point for the GVN pass. Also used by the GVNLegacyPass.
-  bool runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
-               const TargetLibraryInfo &RunTLI, AAResults &RunAA,
-               MemoryDependenceResults *RunMD, LoopInfo &LI,
-               OptimizationRemarkEmitter *ORE, MemorySSA *MSSA = nullptr);
-
-  // Other helper routines.
-
-  Value *findLeader(const BasicBlock *BB, uint32_t Num);
-  void cleanupGlobalSets();
-
-  void removeInstruction(Instruction *I);
-
-  /// This removes the specified instruction from
-  /// our various maps and marks it for deletion.
-  void salvageAndRemoveInstruction(Instruction *I);
-
-  void verifyRemoved(const Instruction *I) const;
-
-  bool splitCriticalEdges();
-  BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ);
-
-  void addDeadBlock(BasicBlock *BB);
-  void assignValNumForDeadCode();
-  void assignBlockRPONumber(Function &F);
 };
 
 /// Create a legacy GVN pass.
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 35e21e0b63017..ce39ddabfd665 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -84,9 +84,6 @@ using namespace llvm;
 using namespace llvm::VNCoercion;
 using namespace PatternMatch;
 
-using AvailableValue = GVNPass::AvailableValue;
-using AvailableValueInBlock = GVNPass::AvailableValueInBlock;
-
 #define DEBUG_TYPE "gvn"
 
 STATISTIC(NumGVNInstr, "Number of instructions deleted");
@@ -191,202 +188,6 @@ template <> struct llvm::DenseMapInfo<GVNValueTable::Expression> {
   }
 };
 
-/// Represents a particular available value that we know how to materialize.
-/// Materialization of an AvailableValue never fails.  An AvailableValue is
-/// implicitly associated with a rematerialization point which is the
-/// location of the instruction from which it was formed.
-struct llvm::GVNPass::AvailableValue {
-  enum class ValType {
-    SimpleVal, // A simple offsetted value that is accessed.
-    LoadVal,   // A value produced by a load.
-    MemIntrin, // A memory intrinsic which is loaded from.
-    UndefVal,  // A UndefValue representing a value from dead block (which
-               // is not yet physically removed from the CFG).
-    SelectVal, // A pointer select which is loaded from and for which the load
-               // can be replace by a value select.
-  };
-
-  /// Val - The value that is live out of the block.
-  Value *Val;
-  /// Kind of the live-out value.
-  ValType Kind;
-
-  /// Offset - The byte offset in Val that is interesting for the load query.
-  unsigned Offset = 0;
-  /// V1, V2 - The dominating non-clobbered values of SelectVal.
-  Value *V1 = nullptr, *V2 = nullptr;
-
-  static AvailableValue get(Value *V, unsigned Offset = 0) {
-    AvailableValue Res;
-    Res.Val = V;
-    Res.Kind = ValType::SimpleVal;
-    Res.Offset = Offset;
-    return Res;
-  }
-
-  static AvailableValue getMI(MemIntrinsic *MI, unsigned Offset = 0) {
-    AvailableValue Res;
-    Res.Val = MI;
-    Res.Kind = ValType::MemIntrin;
-    Res.Offset = Offset;
-    return Res;
-  }
-
-  static AvailableValue getLoad(LoadInst *Load, unsigned Offset = 0) {
-    AvailableValue Res;
-    Res.Val = Load;
-    Res.Kind = ValType::LoadVal;
-    Res.Offset = Offset;
-    return Res;
-  }
-
-  static AvailableValue getUndef() {
-    AvailableValue Res;
-    Res.Val = nullptr;
-    Res.Kind = ValType::UndefVal;
-    Res.Offset = 0;
-    return Res;
-  }
-
-  static AvailableValue getSelect(Value *Cond, Value *V1, Value *V2) {
-    AvailableValue Res;
-    Res.Val = Cond;
-    Res.Kind = ValType::SelectVal;
-    Res.Offset = 0;
-    Res.V1 = V1;
-    Res.V2 = V2;
-    return Res;
-  }
-
-  bool isSimpleValue() const { return Kind == ValType::SimpleVal; }
-  bool isCoercedLoadValue() const { return Kind == ValType::LoadVal; }
-  bool isMemIntrinValue() const { return Kind == ValType::MemIntrin; }
-  bool isUndefValue() const { return Kind == ValType::UndefVal; }
-  bool isSelectValue() const { return Kind == ValType::SelectVal; }
-
-  Value *getSimpleValue() const {
-    assert(isSimpleValue() && "Wrong accessor");
-    return Val;
-  }
-
-  LoadInst *getCoercedLoadValue() const {
-    assert(isCoercedLoadValue() && "Wrong accessor");
-    return cast<LoadInst>(Val);
-  }
-
-  MemIntrinsic *getMemIntrinValue() const {
-    assert(isMemIntrinValue() && "Wrong accessor");
-    return cast<MemIntrinsic>(Val);
-  }
-
-  Value *getSelectCondition() const {
-    assert(isSelectValue() && "Wrong accessor");
-    return Val;
-  }
-
-  /// Emit code at the specified insertion point to adjust the value defined
-  /// here to the specified type. This handles various coercion cases.
-  Value *MaterializeAdjustedValue(LoadInst *Load, Instruction *InsertPt) const;
-};
-
-Value *AvailableValue::MaterializeAdjustedValue(LoadInst *Load,
-                                                Instruction *InsertPt) const {
-  Value *Res;
-  Type *LoadTy = Load->getType();
-  const DataLayout &DL = Load->getDataLayout();
-  if (isSimpleValue()) {
-    Res = getSimpleValue();
-    if (Res->getType() != LoadTy) {
-      Res = getValueForLoad(Res, Offset, LoadTy, InsertPt, Load->getFunction());
-
-      LLVM_DEBUG(dbgs() << "GVN COERCED NONLOCAL VAL:\nOffset: " << Offset
-                        << "  " << *getSimpleValue() << '\n'
-                        << *Res << '\n'
-                        << "\n\n\n");
-    }
-  } else if (isCoercedLoadValue()) {
-    LoadInst *CoercedLoad = getCoercedLoadValue();
-    if (CoercedLoad->getType() == LoadTy && Offset == 0) {
-      Res = CoercedLoad;
-      combineMetadataForCSE(CoercedLoad, Load, false);
-    } else {
-      Res = getValueForLoad(CoercedLoad, Offset, LoadTy, InsertPt,
-                            Load->getFunction());
-      // We are adding a new user for this load, for which the original
-      // metadata may not hold. Additionally, the new load may have a different
-      // size and type, so their metadata cannot be combined in any
-      // straightforward way.
-      // Drop all metadata that is not known to cause immediate UB on violation,
-      // unless the load has !noundef, in which case all metadata violations
-      // will be promoted to UB.
-      // !noalias and !alias.scope are kept: the load is not moved and still
-      // accesses the same memory, and these are independent of the load type
-      // and offset, so they remain valid for the coerced result.
-      if (!CoercedLoad->hasMetadata(LLVMContext::MD_noundef))
-        CoercedLoad->dropUnknownNonDebugMetadata(
-            {LLVMContext::MD_dereferenceable,
-             LLVMContext::MD_dereferenceable_or_null,
-             LLVMContext::MD_invariant_load, LLVMContext::MD_invariant_group,
-             LLVMContext::MD_alias_scope, LLVMContext::MD_noalias});
-      LLVM_DEBUG(dbgs() << "GVN COERCED NONLOCAL LOAD:\nOffset: " << Offset
-                        << "  " << *getCoercedLoadValue() << '\n'
-                        << *Res << '\n'
-                        << "\n\n\n");
-    }
-  } else if (isMemIntrinValue()) {
-    Res = getMemInstValueForLoad(getMemIntrinValue(), Offset, LoadTy, InsertPt,
-                                 DL);
-    LLVM_DEBUG(dbgs() << "GVN COERCED NONLOCAL MEM INTRIN:\nOffset: " << Offset
-                      << "  " << *getMemIntrinValue() << '\n'
-                      << *Res << '\n'
-                      << "\n\n\n");
-  } else if (isSelectValue()) {
-    // Introduce a new value select for a load from an eligible pointer select.
-    Value *Cond = getSelectCondition();
-    assert(V1 && V2 && "both value operands of the select must be present");
-    Res = SelectInst::Create(Cond, V1, V2, "", InsertPt->getIterator());
-    // We use the DebugLoc from the original load here, as this instruction
-    // materializes the value that would previously have been loaded.
-    cast<SelectInst>(Res)->setDebugLoc(Load->getDebugLoc());
-  } else {
-    llvm_unreachable("Should not materialize value from dead block");
-  }
-  assert(Res && "failed to materialize?");
-  return Res;
-}
-
-/// Represents an AvailableValue which can be rematerialized at the end of
-/// the associated BasicBlock.
-struct llvm::GVNPass::AvailableValueInBlock {
-  /// BB - The basic block in question.
-  BasicBlock *BB = nullptr;
-
-  /// AV - The actual available value.
-  AvailableValue AV;
-
-  static AvailableValueInBlock get(BasicBlock *BB, AvailableValue &&AV) {
-    AvailableValueInBlock Res;
-    Res.BB = BB;
-    Res.AV = std::move(AV);
-    return Res;
-  }
-
-  static AvailableValueInBlock get(BasicBlock *BB, Value *V,
-                                   unsigned Offset = 0) {
-    return get(BB, AvailableValue::get(V, Offset));
-  }
-
-  static AvailableValueInBlock getUndef(BasicBlock *BB) {
-    return get(BB, AvailableValue::getUndef());
-  }
-
-  /// Emit code at the end of this block to adjust the value defined here to
-  /// the specified type. This handles various coercion cases.
-  Value *MaterializeAdjustedValue(LoadInst *Load) const {
-    return AV.MaterializeAdjustedValue(Load, BB->getTerminator());
-  }
-};
-
 //===----------------------------------------------------------------------===//
 //                     ValueTable Internal Functions
 //===----------------------------------------------------------------------===//
@@ -1013,221 +814,565 @@ void GVNValueTable::erase(Value *V) {
     NumberingBB.erase(Num);
 }
 
-/// verifyRemoved - Verify that the value is removed from all internal data
-/// structures.
-void GVNValueTable::verifyRemoved(const Value *V) const {
-  assert(!ValueNumbering.contains(V) &&
-         "Inst still occurs in value numbering map!");
-}
+/// verifyRemoved - Verify that the value is removed from all internal data
+/// structures.
+void GVNValueTable::verifyRemoved(const Value *V) const {
+  assert(!ValueNumbering.contains(V) &&
+         "Inst still occurs in value numbering map!");
+}
+
+//===----------------------------------------------------------------------===//
+//                     LeaderMap External Functions
+//===----------------------------------------------------------------------===//
+
+/// Push a new Value to the LeaderTable onto the list for its value number.
+void GVNLeaderMap::insert(uint32_t N, Value *V, const BasicBlock *BB) {
+  const auto &[It, Inserted] = NumToLeaders.try_emplace(N, V, BB, nullptr);
+  if (!Inserted) {
+    // Key already exists: insert new node after the head.
+    auto *NewSlot = TableAllocator.Allocate<LeaderListNode>();
+    new (NewSlot) LeaderListNode(V, BB, It->second.Next);
+    It->second.Next = NewSlot;
+  }
+}
+
+/// Scan the list of values corresponding to a given
+/// value number, and remove the given instruction if encountered.
+void GVNLeaderMap::erase(uint32_t N, Instruction *I, const BasicBlock *BB) {
+  auto It = NumToLeaders.find(N);
+  if (It == NumToLeaders.end())
+    return;
+
+  LeaderListNode *Prev = nullptr;
+  LeaderListNode *Curr = &It->second;
+
+  while (Curr && (Curr->Entry.Val != I || Curr->Entry.BB != BB)) {
+    Prev = Curr;
+    Curr = Curr->Next;
+  }
+
+  if (!Curr)
+    return;
+
+  if (Prev) {
+    // Non-head node: unlink and destroy.
+    Prev->Next = Curr->Next;
+    Curr->~LeaderListNode();
+    TableAllocator.Deallocate<LeaderListNode>(Curr);
+  } else {
+    // Head node (stored by value in DenseMap).
+    if (!Curr->Next) {
+      // Only node; erase from map (DenseMap calls the destructor).
+      NumToLeaders.erase(It);
+    } else {
+      // Move second node's data into head, then destroy second node.
+      LeaderListNode *Next = Curr->Next;
+      Curr->Entry.Val = std::move(Next->Entry.Val);
+      Curr->Entry.BB = Next->Entry.BB;
+      Curr->Next = Next->Next;
+      Next->~LeaderListNode();
+      TableAllocator.Deallocate<LeaderListNode>(Next);
+    }
+  }
+}
+
+namespace {
+//===----------------------------------------------------------------------===//
+//                     Helper Dependency Information Classes
+//===----------------------------------------------------------------------===//
+
+enum class DepKind {
+  Other = 0, // Unknown value.
+  Def,       // Exactly overlapping locations.
+  Clobber,   // Reaching value superset of needed bits.
+  Select,    // Reaching value is a select of two reaching addresses.
+};
+
+// Describe a memory location value, such that there exists a path to a point
+// in the program, along which that memory location is not modified.
+struct Reach...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/211541


More information about the llvm-branch-commits mailing list