From cb7f65342291caa3636cb50c0ee04b383cd79f8d Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 29 Aug 2010 18:42:23 +0000 Subject: [PATCH] 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 :( git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@112452 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/AliasSetTracker.h | 10 ++++++-- lib/Analysis/AliasSetTracker.cpp | 34 +++++++++++++++---------- lib/Transforms/Scalar/LICM.cpp | 1 + 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/include/llvm/Analysis/AliasSetTracker.h b/include/llvm/Analysis/AliasSetTracker.h index 09f12ad281a..8e2f7fd29a3 100644 --- a/include/llvm/Analysis/AliasSetTracker.h +++ b/include/llvm/Analysis/AliasSetTracker.h @@ -92,7 +92,8 @@ class AliasSet : public ilist_node { AliasSet *Forward; // Forwarding pointer. AliasSet *Next, *Prev; // Doubly linked list of AliasSets. - std::vector CallSites; // All calls & invokes in this alias set. + // All calls & invokes in this alias set. + std::vector > CallSites; // RefCount - Number of nodes pointing to this AliasSet plus the number of // AliasSets forwarding to it. @@ -127,6 +128,11 @@ class AliasSet : public ilist_node { 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 @@ private: 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(); } diff --git a/lib/Analysis/AliasSetTracker.cpp b/lib/Analysis/AliasSetTracker.cpp index d52085e5754..5a163a2ab0f 100644 --- a/lib/Analysis/AliasSetTracker.cpp +++ b/lib/Analysis/AliasSetTracker.cpp @@ -116,7 +116,7 @@ void AliasSet::addPointer(AliasSetTracker &AST, PointerRec &Entry, } 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 @@ bool AliasSet::aliasesCallSite(CallSite CS, AliasAnalysis &AA) const { 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 @@ AliasSet *AliasSetTracker::findAliasSetForCallSite(CallSite CS) { 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 @@ void AliasSetTracker::deleteValue(Value *PtrVal) { 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 @@ void AliasSet::print(raw_ostream &OS) const { 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"; diff --git a/lib/Transforms/Scalar/LICM.cpp b/lib/Transforms/Scalar/LICM.cpp index aa842e3183b..aed6a209fca 100644 --- a/lib/Transforms/Scalar/LICM.cpp +++ b/lib/Transforms/Scalar/LICM.cpp @@ -315,6 +315,7 @@ void LICM::SinkRegion(DomTreeNode *N) { // 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();