[llvm-commits] [llvm] r60271 - in /llvm/trunk: include/llvm/Analysis/MemoryDependenceAnalysis.h lib/Analysis/MemoryDependenceAnalysis.cpp

Chris Lattner sabre at nondot.org
Sat Nov 29 18:28:26 PST 2008


Author: lattner
Date: Sat Nov 29 20:28:25 2008
New Revision: 60271

URL: http://llvm.org/viewvc/llvm-project?rev=60271&view=rev
Log:
Change NonLocalDeps to be a densemap of pointers to densemap
instead of containing them by value.  This increases the density
(!) of NonLocalDeps as well as making the reallocation case 
faster.  This speeds up gvn on 403.gcc by 2% and makes room for
future improvements.

I'm not super thrilled with having to explicitly manage the new/delete
of the map, but it is necesary for the next change.

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

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

==============================================================================
--- llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h (original)
+++ llvm/trunk/include/llvm/Analysis/MemoryDependenceAnalysis.h Sat Nov 29 20:28:25 2008
@@ -111,9 +111,9 @@
       Normal,
 
       /// None - This dependence type indicates that the query does not depend
-      /// on any instructions, either because it scanned to the start of the
-      /// function or it scanned to the definition of the memory
-      /// (alloca/malloc).
+      /// on any instructions, either because it is not a memory instruction or
+      /// because it scanned to the definition of the memory (alloca/malloc)
+      /// being accessed.
       None,
       
       /// NonLocal - This marker indicates that the query has no dependency in
@@ -128,9 +128,9 @@
     LocalDepMapType LocalDeps;
 
     // A map from instructions to their non-local dependencies.
-    // FIXME: DENSEMAP of DENSEMAP not a great idea.
     typedef DenseMap<Instruction*,
-                     DenseMap<BasicBlock*, DepResultTy> > NonLocalDepMapType;
+                     // This is an owning pointer.
+                     DenseMap<BasicBlock*, DepResultTy>*> NonLocalDepMapType;
     NonLocalDepMapType NonLocalDeps;
     
     // A reverse mapping from dependencies to the dependees.  This is
@@ -153,6 +153,9 @@
     /// 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;
       NonLocalDeps.clear();
       ReverseLocalDeps.clear();
       ReverseNonLocalDeps.clear();

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

==============================================================================
--- llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp (original)
+++ llvm/trunk/lib/Analysis/MemoryDependenceAnalysis.cpp Sat Nov 29 20:28:25 2008
@@ -227,7 +227,10 @@
                                                       MemDepResult> > &Result) {
   assert(getDependency(QueryInst).isNonLocal() &&
      "getNonLocalDependency should only be used on insts with non-local deps!");
-  DenseMap<BasicBlock*, DepResultTy> &Cache = NonLocalDeps[QueryInst];
+  DenseMap<BasicBlock*, DepResultTy>* &CacheP = NonLocalDeps[QueryInst];
+  if (CacheP == 0) CacheP = new DenseMap<BasicBlock*, DepResultTy>();
+  
+  DenseMap<BasicBlock*, DepResultTy> &Cache = *CacheP;
 
   /// 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
@@ -271,8 +274,14 @@
     // 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.getPointer())
+    if (Instruction *Inst = DirtyBBEntry.getPointer()) {
       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);
+    }
     
     // Find out if this block has a local dependency for QueryInst.
     DirtyBBEntry = getDependencyFromInternal(QueryInst, ScanPos, DirtyBB);
@@ -305,11 +314,16 @@
 void MemoryDependenceAnalysis::removeInstruction(Instruction *RemInst) {
   // Walk through the Non-local dependencies, removing this one as the value
   // for any cached queries.
-  for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
-       NonLocalDeps[RemInst].begin(), DE = NonLocalDeps[RemInst].end();
-       DI != DE; ++DI)
-    if (Instruction *Inst = DI->second.getPointer())
-      ReverseNonLocalDeps[Inst].erase(RemInst);
+  NonLocalDepMapType::iterator NLDI = NonLocalDeps.find(RemInst);
+  if (NLDI != NonLocalDeps.end()) {
+    DenseMap<BasicBlock*, DepResultTy> &BlockMap = *NLDI->second;
+    for (DenseMap<BasicBlock*, DepResultTy>::iterator DI =
+         BlockMap.begin(), DE = BlockMap.end(); DI != DE; ++DI)
+      if (Instruction *Inst = DI->second.getPointer())
+        ReverseNonLocalDeps[Inst].erase(RemInst);
+    delete &BlockMap;
+    NonLocalDeps.erase(NLDI);
+  }
 
   // If we have a cached local dependence query for this instruction, remove it.
   //
@@ -347,10 +361,8 @@
     for (SmallPtrSet<Instruction*, 4>::iterator I = ReverseDeps.begin(),
          E = ReverseDeps.end(); I != E; ++I) {
       Instruction *InstDependingOnRemInst = *I;
-      
-      // If we thought the instruction depended on itself (possible for
-      // unconfirmed dependencies) ignore the update.
-      if (InstDependingOnRemInst == RemInst) continue;
+      assert(InstDependingOnRemInst != RemInst &&
+             "Already removed our local dep info");
                         
       LocalDeps[InstDependingOnRemInst] = DepResultTy(NewDepInst, Dirty);
       
@@ -374,22 +386,27 @@
   if (ReverseDepIt != ReverseNonLocalDeps.end()) {
     SmallPtrSet<Instruction*, 4>& set = ReverseDepIt->second;
     for (SmallPtrSet<Instruction*, 4>::iterator I = set.begin(), E = set.end();
-         I != E; ++I)
+         I != E; ++I) {
+      assert(*I != RemInst && "Already removed NonLocalDep info for RemInst");
+      
+      DenseMap<BasicBlock*, DepResultTy> &INLD = *NonLocalDeps[*I];
+      assert(&INLD != 0 && "Reverse mapping out of date?");
+      
       for (DenseMap<BasicBlock*, DepResultTy>::iterator
-           DI = NonLocalDeps[*I].begin(), DE = NonLocalDeps[*I].end();
-           DI != DE; ++DI)
-        if (DI->second.getPointer() == RemInst) {
-          // Convert to a dirty entry for the subsequent instruction.
-          DI->second.setInt(Dirty);
-          if (RemInst->isTerminator())
-            DI->second.setPointer(0);
-          else {
-            Instruction *NextI = next(BasicBlock::iterator(RemInst));
-            DI->second.setPointer(NextI);
-            assert(NextI != RemInst);
-            ReverseDepsToAdd.push_back(std::make_pair(NextI, *I));
-          }
+           DI = INLD.begin(), DE = INLD.end(); DI != DE; ++DI) {
+        if (DI->second.getPointer() != RemInst) continue;
+        
+        // Convert to a dirty entry for the subsequent instruction.
+        DI->second.setInt(Dirty);
+        if (RemInst->isTerminator())
+          DI->second.setPointer(0);
+        else {
+          Instruction *NextI = next(BasicBlock::iterator(RemInst));
+          DI->second.setPointer(NextI);
+          ReverseDepsToAdd.push_back(std::make_pair(NextI, *I));
         }
+      }
+    }
 
     ReverseNonLocalDeps.erase(ReverseDepIt);
 
@@ -401,7 +418,7 @@
     }
   }
   
-  NonLocalDeps.erase(RemInst);
+  assert(!NonLocalDeps.count(RemInst) && "RemInst got reinserted?");
   getAnalysis<AliasAnalysis>().deleteValue(RemInst);
   DEBUG(verifyRemoved(RemInst));
 }
@@ -419,21 +436,26 @@
   for (NonLocalDepMapType::const_iterator I = NonLocalDeps.begin(),
        E = NonLocalDeps.end(); I != E; ++I) {
     assert(I->first != D && "Inst occurs in data structures");
-    for (DenseMap<BasicBlock*, DepResultTy>::iterator II = I->second.begin(),
-         EE = I->second.end(); II  != EE; ++II)
+    DenseMap<BasicBlock*, DepResultTy> &INLD = *I->second;
+    for (DenseMap<BasicBlock*, DepResultTy>::iterator II = INLD.begin(),
+         EE = INLD.end(); II  != EE; ++II)
       assert(II->second.getPointer() != D && "Inst occurs in data structures");
   }
   
   for (ReverseDepMapType::const_iterator I = ReverseLocalDeps.begin(),
-       E = ReverseLocalDeps.end(); I != E; ++I)
+       E = ReverseLocalDeps.end(); I != E; ++I) {
+    assert(I->first != D && "Inst occurs in data structures");
     for (SmallPtrSet<Instruction*, 4>::const_iterator II = I->second.begin(),
          EE = I->second.end(); II != EE; ++II)
       assert(*II != D && "Inst occurs in data structures");
+  }
   
   for (ReverseDepMapType::const_iterator I = ReverseNonLocalDeps.begin(),
        E = ReverseNonLocalDeps.end();
-       I != E; ++I)
+       I != E; ++I) {
+    assert(I->first != D && "Inst occurs in data structures");
     for (SmallPtrSet<Instruction*, 4>::const_iterator II = I->second.begin(),
          EE = I->second.end(); II != EE; ++II)
       assert(*II != D && "Inst occurs in data structures");
+  }
 }





More information about the llvm-commits mailing list