[llvm-commits] [llvm] r60313 - in /llvm/trunk: include/llvm/Analysis/MemoryDependenceAnalysis.h lib/Analysis/MemoryDependenceAnalysis.cpp lib/Transforms/Scalar/GVN.cpp

Chris Lattner sabre at nondot.org
Sun Nov 30 17:15:43 PST 2008


Author: lattner
Date: Sun Nov 30 19:15:42 2008
New Revision: 60313

URL: http://llvm.org/viewvc/llvm-project?rev=60313&view=rev
Log:
Reimplement the non-local dependency data structure in terms of a sorted
vector instead of a densemap.  This shrinks the memory usage of this thing
substantially (the high water mark) as well as making operations like
scanning it faster.  This speeds up memdep slightly, gvn goes from
3.9376 to 3.9118s on 403.gcc

This also splits out the statistics for the cached non-local case to
differentiate between the dirty and clean cached case.  Here's the stats
for 403.gcc:

  6153 memdep - Number of dirty cached non-local responses
169336 memdep - Number of fully cached non-local responses
162428 memdep - Number of uncached non-local responses

yay for caching :)


Modified:
    llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
    llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp
    llvm/trunk/lib/Transforms/Scalar/GVN.cpp

Modified: llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h?rev=60313&r1=60312&r2=60313&view=diff

==============================================================================
--- llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h (original)
+++ llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h Sun Nov 30 19:15:42 2008
@@ -36,7 +36,7 @@
       /// Invalid - Clients of MemDep never see this.
       Invalid = 0,
       /// Normal - This is a normal instruction dependence.  The pointer member
-      /// of the DepResultTy pair holds the instruction.
+      /// of the MemDepResult pair holds the instruction.
       Normal,
 
       /// NonLocal - This marker indicates that the query has no dependency in
@@ -85,8 +85,10 @@
     /// is depended on.  Otherwise, return null.
     Instruction *getInst() const { return Value.getPointer(); }
     
-    bool operator==(const MemDepResult &M) { return M.Value == Value; }
-    bool operator!=(const MemDepResult &M) { return M.Value != Value; }
+    bool operator==(const MemDepResult &M) const { return M.Value == Value; }
+    bool operator!=(const MemDepResult &M) const { return M.Value != Value; }
+    bool operator<(const MemDepResult &M) const { return M.Value < Value; }
+    bool operator>(const MemDepResult &M) const { return M.Value > Value; }
   private:
     friend class MemoryDependenceAnalysis;
     /// Dirty - Entries with this marker occur in a LocalDeps map or
@@ -95,14 +97,14 @@
     /// instruction pointer.  If so, the pointer is an instruction in the
     /// block where scanning can start from, saving some work.
     ///
-    /// In a default-constructed DepResultTy object, the type will be Dirty
+    /// In a default-constructed MemDepResult object, the type will be Dirty
     /// and the instruction pointer will be null.
     ///
-    
+         
     /// isDirty - Return true if this is a MemDepResult in its dirty/invalid.
     /// state.
     bool isDirty() const { return Value.getInt() == Invalid; }
-
+    
     static MemDepResult getDirty(Instruction *Inst) {
       return MemDepResult(PairTy(Inst, Invalid));
     }
@@ -128,12 +130,15 @@
     typedef DenseMap<Instruction*, MemDepResult> LocalDepMapType;
     LocalDepMapType LocalDeps;
 
-    typedef DenseMap<BasicBlock*, MemDepResult> NonLocalDepInfo;
+  public:
+    typedef std::pair<BasicBlock*, MemDepResult> NonLocalDepEntry;
+    typedef std::vector<NonLocalDepEntry> NonLocalDepInfo;
+  private:
     
     /// PerInstNLInfo - This is the instruction we keep for each cached access
     /// that we have for an instruction.  The pointer is an owning pointer and
     /// the bool indicates whether we have any dirty bits in the set.
-    typedef PointerIntPair<NonLocalDepInfo*, 1, bool> PerInstNLInfo;
+    typedef std::pair<NonLocalDepInfo, bool> PerInstNLInfo;
     
     // A map from instructions to their non-local dependencies.
     typedef DenseMap<Instruction*, PerInstNLInfo> NonLocalDepMapType;
@@ -162,9 +167,7 @@
     /// Clean up memory in between runs
     void releaseMemory() {
       LocalDeps.clear();
-      for (NonLocalDepMapType::iterator I = NonLocalDeps.begin(),
-           E = NonLocalDeps.end(); I != E; ++I)
-        delete I->second.getPointer();
+      NonLocalDeps.clear();
       NonLocalDeps.clear();
       ReverseLocalDeps.clear();
       ReverseNonLocalDeps.clear();
@@ -194,11 +197,14 @@
     /// potentially live across.  The returned set of results will include a
     /// "NonLocal" result for all blocks where the value is live across.
     ///
-    /// This method assumes the instruction returns a "nonlocal" dependency
+    /// This method assumes the instruction returns a "NonLocal" dependency
     /// within its own block.
-    void getNonLocalDependency(Instruction *QueryInst,
-                               SmallVectorImpl<std::pair<BasicBlock*, 
-                                                       MemDepResult> > &Result);
+    ///
+    /// This returns a reference to an internal data structure that may be
+    /// invalidated on the next non-local query or when an instruction is
+    /// removed.  Clients must copy this data if they want it around longer than
+    /// that.
+    const NonLocalDepInfo &getNonLocalDependency(Instruction *QueryInst);
     
     /// removeInstruction - Remove an instruction from the dependence analysis,
     /// updating the dependence of instructions that previously depended on it.

Modified: llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp?rev=60313&r1=60312&r2=60313&view=diff

==============================================================================
--- llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp Sun Nov 30 19:15:42 2008
@@ -28,9 +28,9 @@
 #include "llvm/Target/TargetData.h"
 using namespace llvm;
 
-STATISTIC(NumCacheNonLocal, "Number of cached non-local responses");
+STATISTIC(NumCacheNonLocal, "Number of fully cached non-local responses");
+STATISTIC(NumCacheDirtyNonLocal, "Number of dirty cached non-local responses");
 STATISTIC(NumUncacheNonLocal, "Number of uncached non-local responses");
-
 char MemoryDependenceAnalysis::ID = 0;
   
 // Register this pass...
@@ -51,10 +51,12 @@
   return false;
 }
 
+
 /// getCallSiteDependency - Private helper for finding the local dependencies
 /// of a call site.
 MemDepResult MemoryDependenceAnalysis::
 getCallSiteDependency(CallSite C, BasicBlock::iterator ScanIt, BasicBlock *BB) {
+  
   // Walk backwards through the block, looking for dependencies
   while (ScanIt != BB->begin()) {
     Instruction *Inst = --ScanIt;
@@ -224,16 +226,13 @@
 /// This method assumes the instruction returns a "nonlocal" dependency
 /// within its own block.
 ///
-void MemoryDependenceAnalysis::
-getNonLocalDependency(Instruction *QueryInst,
-                      SmallVectorImpl<std::pair<BasicBlock*, 
-                                                      MemDepResult> > &Result) {
+const MemoryDependenceAnalysis::NonLocalDepInfo &
+MemoryDependenceAnalysis::getNonLocalDependency(Instruction *QueryInst) {
   assert(getDependency(QueryInst).isNonLocal() &&
      "getNonLocalDependency should only be used on insts with non-local deps!");
   PerInstNLInfo &CacheP = NonLocalDeps[QueryInst];
-  if (CacheP.getPointer() == 0) CacheP.setPointer(new NonLocalDepInfo());
   
-  NonLocalDepInfo &Cache = *CacheP.getPointer();
+  NonLocalDepInfo &Cache = CacheP.first;
 
   /// DirtyBlocks - This is the set of blocks that need to be recomputed.  In
   /// the cached case, this can happen due to instructions being deleted etc. In
@@ -242,17 +241,24 @@
   SmallVector<BasicBlock*, 32> DirtyBlocks;
   
   if (!Cache.empty()) {
+    // Okay, we have a cache entry.  If we know it is not dirty, just return it
+    // with no computation.
+    if (!CacheP.second) {
+      NumCacheNonLocal++;
+      return Cache;
+    }
+    
     // If we already have a partially computed set of results, scan them to
-    // determine what is dirty, seeding our initial DirtyBlocks worklist.  The
-    // Int bit of CacheP tells us if we have anything dirty.
-    if (CacheP.getInt())
-      for (NonLocalDepInfo::iterator I = Cache.begin(), E = Cache.end();
-         I != E; ++I)
-        if (I->second.isDirty())
-          DirtyBlocks.push_back(I->first);
+    // determine what is dirty, seeding our initial DirtyBlocks worklist.
+    for (NonLocalDepInfo::iterator I = Cache.begin(), E = Cache.end();
+       I != E; ++I)
+      if (I->second.isDirty())
+        DirtyBlocks.push_back(I->first);
     
-    NumCacheNonLocal++;
+    // Sort the cache so that we can do fast binary search lookups below.
+    std::sort(Cache.begin(), Cache.end());
     
+    ++NumCacheDirtyNonLocal;
     //cerr << "CACHED CASE: " << DirtyBlocks.size() << " dirty: "
     //     << Cache.size() << " cached: " << *QueryInst;
   } else {
@@ -262,52 +268,80 @@
     NumUncacheNonLocal++;
   }
   
+  // Visited checked first, vector in sorted order.
+  SmallPtrSet<BasicBlock*, 64> Visited;
+  
+  unsigned NumSortedEntries = Cache.size();
+  
   // Iterate while we still have blocks to update.
   while (!DirtyBlocks.empty()) {
     BasicBlock *DirtyBB = DirtyBlocks.back();
     DirtyBlocks.pop_back();
     
-    // Get the entry for this block.  Note that this relies on MemDepResult
-    // default initializing to Dirty.
-    MemDepResult &DirtyBBEntry = Cache[DirtyBB];
+    // Already processed this block?
+    if (!Visited.insert(DirtyBB))
+      continue;
+    
+    // Do a binary search to see if we already have an entry for this block in
+    // the cache set.  If so, find it.
+    NonLocalDepInfo::iterator Entry = 
+      std::upper_bound(Cache.begin(), Cache.begin()+NumSortedEntries,
+                       std::make_pair(DirtyBB, MemDepResult()));
+    if (Entry != Cache.begin() && (&*Entry)[-1].first == DirtyBB)
+      --Entry;
+    
+    MemDepResult *ExistingResult = 0;
+    if (Entry != Cache.begin()+NumSortedEntries && 
+        Entry->first == DirtyBB) {
+      // If we already have an entry, and if it isn't already dirty, the block
+      // is done.
+      if (!Entry->second.isDirty())
+        continue;
+      
+      // Otherwise, remember this slot so we can update the value.
+      ExistingResult = &Entry->second;
+    }
     
-    // If DirtyBBEntry isn't dirty, it ended up on the worklist multiple times.
-    if (!DirtyBBEntry.isDirty()) continue;
-
     // If the dirty entry has a pointer, start scanning from it so we don't have
     // to rescan the entire block.
     BasicBlock::iterator ScanPos = DirtyBB->end();
-    if (Instruction *Inst = DirtyBBEntry.getInst()) {
-      ScanPos = Inst;
+    if (ExistingResult) {
+      if (Instruction *Inst = ExistingResult->getInst()) {
+        ScanPos = Inst;
       
-      // We're removing QueryInst's dependence on Inst.
-      SmallPtrSet<Instruction*, 4> &InstMap = ReverseNonLocalDeps[Inst];
-      InstMap.erase(QueryInst);
-      if (InstMap.empty()) ReverseNonLocalDeps.erase(Inst);
+        // We're removing QueryInst's use of Inst.
+        SmallPtrSet<Instruction*, 4> &InstMap = ReverseNonLocalDeps[Inst];
+        InstMap.erase(QueryInst);
+        if (InstMap.empty()) ReverseNonLocalDeps.erase(Inst);
+      }
     }
     
     // Find out if this block has a local dependency for QueryInst.
-    DirtyBBEntry = getDependencyFrom(QueryInst, ScanPos, DirtyBB);
-           
+    MemDepResult Dep = getDependencyFrom(QueryInst, ScanPos, DirtyBB);
+    
+    // If we had a dirty entry for the block, update it.  Otherwise, just add
+    // a new entry.
+    if (ExistingResult)
+      *ExistingResult = Dep;
+    else
+      Cache.push_back(std::make_pair(DirtyBB, Dep));
+    
     // If the block has a dependency (i.e. it isn't completely transparent to
-    // the value), remember it!
-    if (!DirtyBBEntry.isNonLocal()) {
+    // the value), remember the association!
+    if (!Dep.isNonLocal()) {
       // Keep the ReverseNonLocalDeps map up to date so we can efficiently
       // update this when we remove instructions.
-      if (Instruction *Inst = DirtyBBEntry.getInst())
+      if (Instruction *Inst = Dep.getInst())
         ReverseNonLocalDeps[Inst].insert(QueryInst);
-      continue;
-    }
+    } else {
     
-    // If the block *is* completely transparent to the load, we need to check
-    // the predecessors of this block.  Add them to our worklist.
-    DirtyBlocks.append(pred_begin(DirtyBB), pred_end(DirtyBB));
+      // If the block *is* completely transparent to the load, we need to check
+      // the predecessors of this block.  Add them to our worklist.
+      DirtyBlocks.append(pred_begin(DirtyBB), pred_end(DirtyBB));
+    }
   }
   
-  
-  // Copy the result into the output set.
-  for (NonLocalDepInfo::iterator I = Cache.begin(), E = Cache.end(); I != E;++I)
-    Result.push_back(std::make_pair(I->first, I->second));
+  return Cache;
 }
 
 /// removeInstruction - Remove an instruction from the dependence analysis,
@@ -318,12 +352,11 @@
   // for any cached queries.
   NonLocalDepMapType::iterator NLDI = NonLocalDeps.find(RemInst);
   if (NLDI != NonLocalDeps.end()) {
-    NonLocalDepInfo &BlockMap = *NLDI->second.getPointer();
+    NonLocalDepInfo &BlockMap = NLDI->second.first;
     for (NonLocalDepInfo::iterator DI = BlockMap.begin(), DE = BlockMap.end();
          DI != DE; ++DI)
       if (Instruction *Inst = DI->second.getInst())
         ReverseNonLocalDeps[Inst].erase(RemInst);
-    delete &BlockMap;
     NonLocalDeps.erase(NLDI);
   }
 
@@ -392,12 +425,11 @@
       assert(*I != RemInst && "Already removed NonLocalDep info for RemInst");
       
       PerInstNLInfo &INLD = NonLocalDeps[*I];
-      assert(INLD.getPointer() != 0 && "Reverse mapping out of date?");
       // The information is now dirty!
-      INLD.setInt(true);
+      INLD.second = true;
       
-      for (NonLocalDepInfo::iterator DI = INLD.getPointer()->begin(), 
-           DE = INLD.getPointer()->end(); DI != DE; ++DI) {
+      for (NonLocalDepInfo::iterator DI = INLD.first.begin(), 
+           DE = INLD.first.end(); DI != DE; ++DI) {
         if (DI->second.getInst() != RemInst) continue;
         
         // Convert to a dirty entry for the subsequent instruction.
@@ -439,8 +471,8 @@
        E = NonLocalDeps.end(); I != E; ++I) {
     assert(I->first != D && "Inst occurs in data structures");
     const PerInstNLInfo &INLD = I->second;
-    for (NonLocalDepInfo::iterator II = INLD.getPointer()->begin(),
-         EE = INLD.getPointer()->end(); II  != EE; ++II)
+    for (NonLocalDepInfo::const_iterator II = INLD.first.begin(),
+         EE = INLD.first.end(); II  != EE; ++II)
       assert(II->second.getInst() != D && "Inst occurs in data structures");
   }
   

Modified: llvm/trunk/lib/Transforms/Scalar/GVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVN.cpp?rev=60313&r1=60312&r2=60313&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVN.cpp Sun Nov 30 19:15:42 2008
@@ -497,15 +497,15 @@
         return v;
       }
       
-      
-      SmallVector<std::pair<BasicBlock*, MemDepResult>, 32> deps;
-      MD->getNonLocalDependency(C, deps);
+
+      const MemoryDependenceAnalysis::NonLocalDepInfo &deps = 
+        MD->getNonLocalDependency(C);
       CallInst* cdep = 0;
       
       // Check to see if we have a single dominating call instruction that is
       // identical to C.
-      for (SmallVector<std::pair<BasicBlock*, MemDepResult>, 32>
-             ::iterator I = deps.begin(), E = deps.end(); I != E; ++I) {
+      for (unsigned i = 0, e = deps.size(); i != e; ++i) {
+        const MemoryDependenceAnalysis::NonLocalDepEntry *I = &deps[i];
         // Ignore non-local dependencies.
         if (I->second.isNonLocal())
           continue;
@@ -868,11 +868,18 @@
 bool GVN::processNonLocalLoad(LoadInst* L,
                               SmallVectorImpl<Instruction*> &toErase) {
   // Find the non-local dependencies of the load
-  SmallVector<std::pair<BasicBlock*, MemDepResult>, 32> deps;
-  MD->getNonLocalDependency(L, deps);
-
+  const MemoryDependenceAnalysis::NonLocalDepInfo &deps = 
+    MD->getNonLocalDependency(L);
   DEBUG(cerr << "INVESTIGATING NONLOCAL LOAD: " << deps.size() << *L);
-
+#if 0
+  DEBUG(for (unsigned i = 0, e = deps.size(); i != e; ++i) {
+        cerr << "  " << deps[i].first->getName();
+          if (Instruction *I = deps[i].second.getInst())
+        cerr << *I;
+        else
+        cerr << "\n";
+        });
+#endif          
   
   // If we had to process more than one hundred blocks to find the
   // dependencies, this load isn't worth worrying about.  Optimizing
@@ -885,31 +892,36 @@
   DenseMap<BasicBlock*, Value*> repl;
   
   // Filter out useless results (non-locals, etc)
-  for (SmallVector<std::pair<BasicBlock*, MemDepResult>, 32>::iterator
-       I = deps.begin(), E = deps.end(); I != E; ++I) {
-    if (I->second.isNone()) {
-      repl[I->first] = UndefValue::get(L->getType());
-      continue;
-    }
-  
-    if (I->second.isNonLocal()) {
+  for (unsigned i = 0, e = deps.size(); i != e; ++i) {
+    BasicBlock *DepBB = deps[i].first;
+    MemDepResult DepInfo = deps[i].second;
+    
+    if (DepInfo.isNonLocal()) {
       // If this is a non-local dependency in the entry block, then we depend on
       // the value live-in at the start of the function.  We could insert a load
       // in the entry block to get this, but for now we'll just bail out.
+      //
       // FIXME: Consider emitting a load in the entry block to catch this case!
-      if (I->first == EntryBlock)
+      // Tricky part is to sink so that it doesn't execute in places where it
+      // isn't needed.
+      if (DepBB == EntryBlock)
         return false;
       continue;
     }
+    
+    if (DepInfo.isNone()) {
+      repl[DepBB] = UndefValue::get(L->getType());
+      continue;
+    }
   
-    if (StoreInst* S = dyn_cast<StoreInst>(I->second.getInst())) {
+    if (StoreInst* S = dyn_cast<StoreInst>(DepInfo.getInst())) {
       if (S->getPointerOperand() != L->getPointerOperand())
         return false;
-      repl[I->first] = S->getOperand(0);
-    } else if (LoadInst* LD = dyn_cast<LoadInst>(I->second.getInst())) {
+      repl[DepBB] = S->getOperand(0);
+    } else if (LoadInst* LD = dyn_cast<LoadInst>(DepInfo.getInst())) {
       if (LD->getPointerOperand() != L->getPointerOperand())
         return false;
-      repl[I->first] = LD;
+      repl[DepBB] = LD;
     } else {
       return false;
     }





More information about the llvm-commits mailing list