From a32748ff239d25f4f5b8dd7e31f7b0a94f2b883f Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Mon, 8 Dec 2014 08:48:09 +0000 Subject: [PATCH] Merging r222338: ------------------------------------------------------------------------ r222338 | majnemer | 2014-11-19 01:41:05 -0800 (Wed, 19 Nov 2014) | 16 lines AliasSetTracker: UnknownInsts should contribute to the refcount AliasSetTracker::addUnknown may create an AliasSet devoid of pointers just to contain an instruction if no suitable AliasSet already exists. It will then AliasSet::addUnknownInst and we will be done. However, it's possible for addUnknown to choose an existing AliasSet to addUnknownInst. If this were to occur, we are in a bit of a pickle: removing pointers from the AliasSet can cause the entire AliasSet to become destroyed, taking our unknown instructions out with them. Instead, keep track whether or not our AliasSet has any unknown instructions. This fixes PR21582. ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_35@223627 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/AliasSetTracker.h | 5 ++- lib/Analysis/AliasSetTracker.cpp | 41 ++++++++++++++++--------- test/Transforms/LICM/PR21582.ll | 40 ++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 test/Transforms/LICM/PR21582.ll diff --git a/include/llvm/Analysis/AliasSetTracker.h b/include/llvm/Analysis/AliasSetTracker.h index 6117d91ec65..e32b6d628b7 100644 --- a/include/llvm/Analysis/AliasSetTracker.h +++ b/include/llvm/Analysis/AliasSetTracker.h @@ -253,13 +253,16 @@ private: const MDNode *TBAAInfo, bool KnownMustAlias = false); void addUnknownInst(Instruction *I, AliasAnalysis &AA); - void removeUnknownInst(Instruction *I) { + void removeUnknownInst(AliasSetTracker &AST, Instruction *I) { + bool WasEmpty = UnknownInsts.empty(); for (size_t i = 0, e = UnknownInsts.size(); i != e; ++i) if (UnknownInsts[i] == I) { UnknownInsts[i] = UnknownInsts.back(); UnknownInsts.pop_back(); --i; --e; // Revisit the moved entry. } + if (!WasEmpty && UnknownInsts.empty()) + dropRef(AST); } void setVolatile() { Volatile = true; } diff --git a/lib/Analysis/AliasSetTracker.cpp b/lib/Analysis/AliasSetTracker.cpp index a45fe2389ee..cf3aecfd010 100644 --- a/lib/Analysis/AliasSetTracker.cpp +++ b/lib/Analysis/AliasSetTracker.cpp @@ -55,12 +55,16 @@ void AliasSet::mergeSetIn(AliasSet &AS, AliasSetTracker &AST) { AliasTy = MayAlias; } + bool ASHadUnknownInsts = false; if (UnknownInsts.empty()) { // Merge call sites... - if (!AS.UnknownInsts.empty()) + if (!AS.UnknownInsts.empty()) { std::swap(UnknownInsts, AS.UnknownInsts); + addRef(); + } } else if (!AS.UnknownInsts.empty()) { UnknownInsts.insert(UnknownInsts.end(), AS.UnknownInsts.begin(), AS.UnknownInsts.end()); AS.UnknownInsts.clear(); + ASHadUnknownInsts = true; } AS.Forward = this; // Forward across AS now... @@ -76,6 +80,8 @@ void AliasSet::mergeSetIn(AliasSet &AS, AliasSetTracker &AST) { AS.PtrListEnd = &AS.PtrList; assert(*AS.PtrListEnd == nullptr && "End of list is not null?"); } + if (ASHadUnknownInsts) + AS.dropRef(AST); } void AliasSetTracker::removeAliasSet(AliasSet *AS) { @@ -123,6 +129,8 @@ void AliasSet::addPointer(AliasSetTracker &AST, PointerRec &Entry, } void AliasSet::addUnknownInst(Instruction *I, AliasAnalysis &AA) { + if (UnknownInsts.empty()) + addRef(); UnknownInsts.push_back(I); if (!I->mayWriteToMemory()) { @@ -218,13 +226,14 @@ AliasSet *AliasSetTracker::findAliasSetForPointer(const Value *Ptr, uint64_t Size, const MDNode *TBAAInfo) { AliasSet *FoundSet = nullptr; - for (iterator I = begin(), E = end(); I != E; ++I) { - if (I->Forward || !I->aliasesPointer(Ptr, Size, TBAAInfo, AA)) continue; + for (iterator I = begin(), E = end(); I != E;) { + iterator Cur = I++; + if (Cur->Forward || !Cur->aliasesPointer(Ptr, Size, TBAAInfo, AA)) continue; if (!FoundSet) { // If this is the first alias set ptr can go into. - FoundSet = I; // Remember it. + FoundSet = Cur; // Remember it. } else { // Otherwise, we must merge the sets. - FoundSet->mergeSetIn(*I, *this); // Merge in contents. + FoundSet->mergeSetIn(*Cur, *this); // Merge in contents. } } @@ -246,14 +255,14 @@ bool AliasSetTracker::containsPointer(Value *Ptr, uint64_t Size, AliasSet *AliasSetTracker::findAliasSetForUnknownInst(Instruction *Inst) { AliasSet *FoundSet = nullptr; - for (iterator I = begin(), E = end(); I != E; ++I) { - if (I->Forward || !I->aliasesUnknownInst(Inst, AA)) + for (iterator I = begin(), E = end(); I != E;) { + iterator Cur = I++; + if (Cur->Forward || !Cur->aliasesUnknownInst(Inst, AA)) continue; - if (!FoundSet) // 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. + FoundSet = Cur; // Remember it. + else if (!Cur->Forward) // Otherwise, we must merge the sets. + FoundSet->mergeSetIn(*Cur, *this); // Merge in contents. } return FoundSet; } @@ -393,6 +402,8 @@ void AliasSetTracker::add(const AliasSetTracker &AST) { /// tracker. void AliasSetTracker::remove(AliasSet &AS) { // Drop all call sites. + if (!AS.UnknownInsts.empty()) + AS.dropRef(*this); AS.UnknownInsts.clear(); // Clear the alias set. @@ -489,10 +500,10 @@ void AliasSetTracker::deleteValue(Value *PtrVal) { if (Instruction *Inst = dyn_cast(PtrVal)) { if (Inst->mayReadOrWriteMemory()) { // 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->removeUnknownInst(Inst); + for (iterator I = begin(), E = end(); I != E;) { + iterator Cur = I++; + if (!Cur->Forward) + Cur->removeUnknownInst(*this, Inst); } } } diff --git a/test/Transforms/LICM/PR21582.ll b/test/Transforms/LICM/PR21582.ll new file mode 100644 index 00000000000..c068c2f8e32 --- /dev/null +++ b/test/Transforms/LICM/PR21582.ll @@ -0,0 +1,40 @@ +; RUN: opt < %s -basicaa -licm -S | FileCheck %s +@b = external global i32, align 4 +@fn3.i = external global i32, align 4 + +declare i32 @g() nounwind + +define i32 @f() { +entry: + br label %for.cond + +for.cond: ; preds = %for.end, %entry +; CHECK-LABEL: for.cond: +; CHECK: store i32 0, i32* @b + store i32 0, i32* @b, align 4 + br i1 true, label %for.body.preheader, label %for.end + +for.body.preheader: ; preds = %for.cond + br label %for.body + +for.body: ; preds = %for.body, %for.body.preheader + %g.15 = phi i32 [ undef, %for.body ], [ 0, %for.body.preheader ] + %arrayidx2 = getelementptr inbounds i32* @fn3.i, i64 0 + %0 = load i32* %arrayidx2, align 4 + %call = call i32 @g() + br i1 false, label %for.body, label %for.end.loopexit + +for.end.loopexit: ; preds = %for.body + br label %for.end + +for.end: ; preds = %for.end.loopexit, %for.cond + %whatever = phi i32 [ %call, %for.end.loopexit ], [ undef, %for.cond ] + br i1 false, label %for.cond, label %if.then + +if.then: ; preds = %for.end +; CHECK-LABEL: if.then: +; CHECK: phi i32 [ {{.*}}, %for.end ] +; CHECK-NOT: store i32 0, i32* @b +; CHECK: ret i32 + ret i32 %whatever +}