[LifetimeSafety] Optimize fact storage with IDs and vector-based lookup

[usx95]

Optimize the FactManager and DataflowAnalysis classes by using vector-based storage with ID-based lookups instead of maps.

• Added a FactID type using the utils::ID template to uniquely identify facts
• Modified Fact class to store and manage IDs
• Changed FactManager to use vector-based storage indexed by block ID instead of a map
• Updated DataflowAnalysis to use vector-based storage for states instead of maps
• Modified lookups to use ID-based indexing for better performance

Improves compile time hit on long-tail targets like tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/TargetBuiltins/RISCV.cpp.o​ from 21% to 3.2%

[usx95]

• [LifetimeSafety] Optimize fact storage with IDs and vector-based lookup #165963 👈 (View in Graphite)
• [LifetimeSafety] Optimize loan propagation by separating persistent and block-local origins #165789
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang-temporal-safety

Author: Utkarsh Saxena (usx95)

Changes

Optimize the FactManager and DataflowAnalysis classes by using vector-based storage with ID-based lookups instead of maps.

• Added a FactID type using the utils::ID template to uniquely identify facts
• Modified Fact class to store and manage IDs
• Changed FactManager to use vector-based storage indexed by block ID instead of a map
• Updated DataflowAnalysis to use vector-based storage for states instead of maps
• Modified lookups to use ID-based indexing for better performance

Improves compile time hit on long-tail targets like tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/TargetBuiltins/RISCV.cpp.o​ from 21% to 3.2%

---

Full diff: https://github.com/llvm/llvm-project/pull/165963.diff

4 Files Affected:

• (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h (+23-8)
• (modified) clang/lib/Analysis/LifetimeSafety/Dataflow.h (+9-5)
• (modified) clang/lib/Analysis/LifetimeSafety/Facts.cpp (+5-8)
• (modified) clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp (+1)

diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index 063cb5c2d42ab..b9cad5340c940 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -16,6 +16,7 @@
 
 #include "clang/Analysis/Analyses/LifetimeSafety/Loans.h"
 #include "clang/Analysis/Analyses/LifetimeSafety/Origins.h"
+#include "clang/Analysis/Analyses/LifetimeSafety/Utils.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
 #include "llvm/ADT/SmallVector.h"
@@ -23,6 +24,9 @@
 #include <cstdint>
 
 namespace clang::lifetimes::internal {
+
+using FactID = utils::ID<struct FactTag>;
+
 /// An abstract base class for a single, atomic lifetime-relevant event.
 class Fact {
 
@@ -48,6 +52,7 @@ class Fact {
 
 private:
   Kind K;
+  FactID ID;
 
 protected:
   Fact(Kind K) : K(K) {}
@@ -56,6 +61,9 @@ class Fact {
   virtual ~Fact() = default;
   Kind getKind() const { return K; }
 
+  void setID(FactID ID) { this->ID = ID; }
+  FactID getID() const { return ID; }
+
   template <typename T> const T *getAs() const {
     if (T::classof(this))
       return static_cast<const T *>(this);
@@ -183,22 +191,26 @@ class TestPointFact : public Fact {
 
 class FactManager {
 public:
+  void init(const CFG &Cfg) {
+    assert(BlockToFacts.empty() && "FactManager already initialized");
+    BlockToFacts.resize(Cfg.getNumBlockIDs());
+  }
+
   llvm::ArrayRef<const Fact *> getFacts(const CFGBlock *B) const {
-    auto It = BlockToFactsMap.find(B);
-    if (It != BlockToFactsMap.end())
-      return It->second;
-    return {};
+    return BlockToFacts[B->getBlockID()];
   }
 
   void addBlockFacts(const CFGBlock *B, llvm::ArrayRef<Fact *> NewFacts) {
     if (!NewFacts.empty())
-      BlockToFactsMap[B].assign(NewFacts.begin(), NewFacts.end());
+      BlockToFacts[B->getBlockID()].assign(NewFacts.begin(), NewFacts.end());
   }
 
   template <typename FactType, typename... Args>
   FactType *createFact(Args &&...args) {
     void *Mem = FactAllocator.Allocate<FactType>();
-    return new (Mem) FactType(std::forward<Args>(args)...);
+    FactType *Res = new (Mem) FactType(std::forward<Args>(args)...);
+    Res->setID(NextFactID++);
+    return Res;
   }
 
   void dump(const CFG &Cfg, AnalysisDeclContext &AC) const;
@@ -214,16 +226,19 @@ class FactManager {
   /// \note This is intended for testing only.
   llvm::StringMap<ProgramPoint> getTestPoints() const;
 
+  unsigned getNumFacts() const { return NextFactID.Value; }
+
   LoanManager &getLoanMgr() { return LoanMgr; }
   const LoanManager &getLoanMgr() const { return LoanMgr; }
   OriginManager &getOriginMgr() { return OriginMgr; }
   const OriginManager &getOriginMgr() const { return OriginMgr; }
 
 private:
+  FactID NextFactID{0};
   LoanManager LoanMgr;
   OriginManager OriginMgr;
-  llvm::DenseMap<const clang::CFGBlock *, llvm::SmallVector<const Fact *>>
-      BlockToFactsMap;
+  /// Facts for each CFG block, indexed by block ID.
+  llvm::SmallVector<llvm::SmallVector<const Fact *>> BlockToFacts;
   llvm::BumpPtrAllocator FactAllocator;
 };
 } // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h
index 2f7bcb6e5dc81..de821bb17eb6b 100644
--- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h
+++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h
@@ -67,10 +67,10 @@ class DataflowAnalysis {
   llvm::DenseMap<const CFGBlock *, Lattice> InStates;
   /// The dataflow state after a basic block is processed.
   llvm::DenseMap<const CFGBlock *, Lattice> OutStates;
-  /// The dataflow state at a Program Point.
+  /// Dataflow state at each program point, indexed by Fact ID.
   /// In a forward analysis, this is the state after the Fact at that point has
   /// been applied, while in a backward analysis, it is the state before.
-  llvm::DenseMap<ProgramPoint, Lattice> PerPointStates;
+  llvm::SmallVector<Lattice> PointToState;
 
   static constexpr bool isForward() { return Dir == Direction::Forward; }
 
@@ -86,6 +86,8 @@ class DataflowAnalysis {
     Derived &D = static_cast<Derived &>(*this);
     llvm::TimeTraceScope Time(D.getAnalysisName());
 
+    PointToState.resize(FactMgr.getNumFacts());
+
     using Worklist =
         std::conditional_t<Dir == Direction::Forward, ForwardDataflowWorklist,
                            BackwardDataflowWorklist>;
@@ -116,7 +118,9 @@ class DataflowAnalysis {
   }
 
 protected:
-  Lattice getState(ProgramPoint P) const { return PerPointStates.lookup(P); }
+  Lattice getState(ProgramPoint P) const {
+    return PointToState[P->getID().Value];
+  }
 
   std::optional<Lattice> getInState(const CFGBlock *B) const {
     auto It = InStates.find(B);
@@ -144,12 +148,12 @@ class DataflowAnalysis {
     if constexpr (isForward()) {
       for (const Fact *F : Facts) {
         State = transferFact(State, F);
-        PerPointStates[F] = State;
+        PointToState[F->getID().Value] = State;
       }
     } else {
       for (const Fact *F : llvm::reverse(Facts)) {
         // In backward analysis, capture the state before applying the fact.
-        PerPointStates[F] = State;
+        PointToState[F->getID().Value] = State;
         State = transferFact(State, F);
       }
     }
diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index ecde390cd6ca3..4a4172fe55bf3 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -64,8 +64,8 @@ void TestPointFact::dump(llvm::raw_ostream &OS, const LoanManager &,
 
 llvm::StringMap<ProgramPoint> FactManager::getTestPoints() const {
   llvm::StringMap<ProgramPoint> AnnotationToPointMap;
-  for (const CFGBlock *Block : BlockToFactsMap.keys()) {
-    for (const Fact *F : getFacts(Block)) {
+  for (const auto &BlockFacts : BlockToFacts) {
+    for (const Fact *F : BlockFacts) {
       if (const auto *TPF = F->getAs<TestPointFact>()) {
         StringRef PointName = TPF->getAnnotation();
         assert(AnnotationToPointMap.find(PointName) ==
@@ -88,12 +88,9 @@ void FactManager::dump(const CFG &Cfg, AnalysisDeclContext &AC) const {
   // Print blocks in the order as they appear in code for a stable ordering.
   for (const CFGBlock *B : *AC.getAnalysis<PostOrderCFGView>()) {
     llvm::dbgs() << "  Block B" << B->getBlockID() << ":\n";
-    auto It = BlockToFactsMap.find(B);
-    if (It != BlockToFactsMap.end()) {
-      for (const Fact *F : It->second) {
-        llvm::dbgs() << "    ";
-        F->dump(llvm::dbgs(), LoanMgr, OriginMgr);
-      }
+    for (const Fact *F : getFacts(B)) {
+      llvm::dbgs() << "    ";
+      F->dump(llvm::dbgs(), LoanMgr, OriginMgr);
     }
     llvm::dbgs() << "  End of Block\n";
   }
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
index 00c7ed90503e7..a51ba4280f284 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
@@ -41,6 +41,7 @@ void LifetimeSafetyAnalysis::run() {
   const CFG &Cfg = *AC.getCFG();
   DEBUG_WITH_TYPE("PrintCFG", Cfg.dump(AC.getASTContext().getLangOpts(),
                                        /*ShowColors=*/true));
+  FactMgr.init(Cfg);
 
   FactsGenerator FactGen(FactMgr, AC);
   FactGen.run();

[usx95]

Merge activity

• Nov 7, 3:41 AM UTC: Graphite rebased this pull request as part of a merge.
• Nov 7, 3:57 AM UTC: Graphite rebased this pull request as part of a merge.
• Nov 7, 4:00 AM UTC: Graphite rebased this pull request as part of a merge.
• Nov 7, 4:05 AM UTC: Graphite rebased this pull request as part of a merge.
• Nov 7, 4:27 AM UTC: @usx95 merged this pull request with Graphite.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-darwin running on doug-worker-3 while building clang at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/15335

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang :: Headers/stdckdint.c' FAILED ********************
Exit Code: 127

Command Output (stdout):
--
# RUN: at line 2
/Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/bin/clang -cc1 -internal-isystem /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/lib/clang/22/include -nostdsysteminc -triple=x86_64 -emit-llvm -verify -std=c99 /Volumes/RAMDisk/buildbot-root/x86_64-darwin/llvm-project/clang/test/Headers/stdckdint.c -o - | /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/bin/FileCheck /Volumes/RAMDisk/buildbot-root/x86_64-darwin/llvm-project/clang/test/Headers/stdckdint.c
# executed command: /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/bin/clang -cc1 -internal-isystem /Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/lib/clang/22/include -nostdsysteminc -triple=x86_64 -emit-llvm -verify -std=c99 /Volumes/RAMDisk/buildbot-root/x86_64-darwin/llvm-project/clang/test/Headers/stdckdint.c -o -
# .---command stderr------------
# | Could not create process (/Volumes/RAMDisk/buildbot-root/x86_64-darwin/build/bin/clang) due to [Errno 24] Too many open files
# `-----------------------------
# error: command failed with exit status: 127

--

********************