[llvm-commits] [llvm] r112452 - in /llvm/trunk: include/llvm/Analysis/AliasSetTracker.h lib/Analysis/AliasSetTracker.cpp lib/Transforms/Scalar/LICM.cpp

[Chris Lattner]

Author: lattner
Date: Sun Aug 29 13:42:23 2010
New Revision: 112452

URL: http://llvm.org/viewvc/llvm-project?rev=112452&view=rev
Log:
two changes: 1) make AliasSet hold the list of call sites with an
assertingvh so we get a violent explosion if the pointer dangles.

2) Fix AliasSetTracker::deleteValue to remove call sites with
   by-pointer comparisons instead of by-alias queries.  Using
   findAliasSetForCallSite can cause alias sets to get merged
   when they shouldn't, and can also miss alias sets when the
   call is readonly.

#2 fixes PR6889, which only repros with a .c file :(

Modified:
    llvm/trunk/include/llvm/Analysis/AliasSetTracker.h
    llvm/trunk/lib/Analysis/AliasSetTracker.cpp
    llvm/trunk/lib/Transforms/Scalar/LICM.cpp

Modified: llvm/trunk/include/llvm/Analysis/AliasSetTracker.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/AliasSetTracker.h?rev=112452&r1=112451&r2=112452&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/AliasSetTracker.h (original)
+++ llvm/trunk/include/llvm/Analysis/AliasSetTracker.h Sun Aug 29 13:42:23 2010
@@ -92,7 +92,8 @@
   AliasSet *Forward;             // Forwarding pointer.
   AliasSet *Next, *Prev;         // Doubly linked list of AliasSets.
 
-  std::vector<CallSite> CallSites; // All calls & invokes in this alias set.
+  // All calls & invokes in this alias set.
+  std::vector<AssertingVH<Instruction> > CallSites;
 
   // RefCount - Number of nodes pointing to this AliasSet plus the number of
   // AliasSets forwarding to it.
@@ -127,6 +128,11 @@
       removeFromTracker(AST);
   }
 
+  CallSite getCallSite(unsigned i) const {
+    assert(i < CallSites.size());
+    return CallSite(CallSites[i]);
+  }
+  
 public:
   /// Accessors...
   bool isRef() const { return AccessTy & Refs; }
@@ -229,7 +235,7 @@
   void addCallSite(CallSite CS, AliasAnalysis &AA);
   void removeCallSite(CallSite CS) {
     for (size_t i = 0, e = CallSites.size(); i != e; ++i)
-      if (CallSites[i].getInstruction() == CS.getInstruction()) {
+      if (CallSites[i] == CS.getInstruction()) {
         CallSites[i] = CallSites.back();
         CallSites.pop_back();
       }

Modified: llvm/trunk/lib/Analysis/AliasSetTracker.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/AliasSetTracker.cpp?rev=112452&r1=112451&r2=112452&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/AliasSetTracker.cpp (original)
+++ llvm/trunk/lib/Analysis/AliasSetTracker.cpp Sun Aug 29 13:42:23 2010
@@ -116,7 +116,7 @@
 }
 
 void AliasSet::addCallSite(CallSite CS, AliasAnalysis &AA) {
-  CallSites.push_back(CS);
+  CallSites.push_back(CS.getInstruction());
 
   AliasAnalysis::ModRefBehavior Behavior = AA.getModRefBehavior(CS);
   if (Behavior == AliasAnalysis::DoesNotAccessMemory)
@@ -167,10 +167,11 @@
   if (AA.doesNotAccessMemory(CS))
     return false;
 
-  for (unsigned i = 0, e = CallSites.size(); i != e; ++i)
-    if (AA.getModRefInfo(CallSites[i], CS) != AliasAnalysis::NoModRef ||
-        AA.getModRefInfo(CS, CallSites[i]) != AliasAnalysis::NoModRef)
+  for (unsigned i = 0, e = CallSites.size(); i != e; ++i) {
+    if (AA.getModRefInfo(getCallSite(i), CS) != AliasAnalysis::NoModRef ||
+        AA.getModRefInfo(CS, getCallSite(i)) != AliasAnalysis::NoModRef)
       return true;
+  }
 
   for (iterator I = begin(), E = end(); I != E; ++I)
     if (AA.getModRefInfo(CS, I.getPointer(), I.getSize()) !=
@@ -231,11 +232,10 @@
     if (I->Forward || !I->aliasesCallSite(CS, AA))
       continue;
     
-    if (FoundSet == 0) {  // If this is the first alias set ptr can go into.
-      FoundSet = I;       // Remember it.
-    } else if (!I->Forward) {     // Otherwise, we must merge the sets.
+    if (FoundSet == 0)        // If this is the first alias set ptr can go into.
+      FoundSet = I;           // Remember it.
+    else if (!I->Forward)     // Otherwise, we must merge the sets.
       FoundSet->mergeSetIn(*I, *this);     // Merge in contents.
-    }
   }
   return FoundSet;
 }
@@ -458,11 +458,17 @@
   AA.deleteValue(PtrVal);
 
   // If this is a call instruction, remove the callsite from the appropriate
-  // AliasSet.
-  if (CallSite CS = PtrVal)
-    if (!AA.doesNotAccessMemory(CS))
-      if (AliasSet *AS = findAliasSetForCallSite(CS))
-        AS->removeCallSite(CS);
+  // AliasSet (if present).
+  if (CallSite CS = PtrVal) {
+    if (!AA.doesNotAccessMemory(CS)) {
+      // Scan all the alias sets to see if this call site is contained.
+      for (iterator I = begin(), E = end(); I != E; ++I) {
+        if (I->Forward) continue;
+        
+        I->removeCallSite(CS);
+      }
+    }
+  }
 
   // First, look up the PointerRec for this pointer.
   PointerMapType::iterator I = PointerMap.find(PtrVal);
@@ -538,7 +544,7 @@
     OS << "\n    " << CallSites.size() << " Call Sites: ";
     for (unsigned i = 0, e = CallSites.size(); i != e; ++i) {
       if (i) OS << ", ";
-      WriteAsOperand(OS, CallSites[i].getCalledValue());
+      WriteAsOperand(OS, CallSites[i]);
     }
   }
   OS << "\n";

Modified: llvm/trunk/lib/Transforms/Scalar/LICM.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LICM.cpp?rev=112452&r1=112451&r2=112452&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LICM.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LICM.cpp Sun Aug 29 13:42:23 2010
@@ -315,6 +315,7 @@
     // If the instruction is dead, we would try to sink it because it isn't used
     // in the loop, instead, just delete it.
     if (isInstructionTriviallyDead(&I)) {
+      DEBUG(dbgs() << "LICM deleting dead inst: " << I << '\n');
       ++II;
       CurAST->deleteValue(&I);
       I.eraseFromParent();