diff --git a/lib/Analysis/BasicAliasAnalysis.cpp b/lib/Analysis/BasicAliasAnalysis.cpp index b2c20110e90..14268ec3910 100644 --- a/lib/Analysis/BasicAliasAnalysis.cpp +++ b/lib/Analysis/BasicAliasAnalysis.cpp @@ -18,6 +18,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/AliasAnalysis.h" #include "llvm/Analysis/CaptureTracking.h" +#include "llvm/Analysis/Dominators.h" #include "llvm/Analysis/InstructionSimplify.h" #include "llvm/Analysis/MemoryBuiltins.h" #include "llvm/Analysis/ValueTracking.h" @@ -38,6 +39,12 @@ #include using namespace llvm; +/// Cutoff after which to stop analysing a set of phi nodes potentially involved +/// in a cycle. Because we are analysing 'through' phi nodes we need to be +/// careful with value equivalence. We use dominance to make sure a value cannot +/// be involved in a cycle. +const unsigned MaxNumPhiBBsValueDominanceCheck = 20; + //===----------------------------------------------------------------------===// // Useful predicates //===----------------------------------------------------------------------===// @@ -403,42 +410,6 @@ DecomposeGEPExpression(const Value *V, int64_t &BaseOffs, return V; } -/// GetIndexDifference - Dest and Src are the variable indices from two -/// decomposed GetElementPtr instructions GEP1 and GEP2 which have common base -/// pointers. Subtract the GEP2 indices from GEP1 to find the symbolic -/// difference between the two pointers. -static void GetIndexDifference(SmallVectorImpl &Dest, - const SmallVectorImpl &Src) { - if (Src.empty()) return; - - for (unsigned i = 0, e = Src.size(); i != e; ++i) { - const Value *V = Src[i].V; - ExtensionKind Extension = Src[i].Extension; - int64_t Scale = Src[i].Scale; - - // Find V in Dest. This is N^2, but pointer indices almost never have more - // than a few variable indexes. - for (unsigned j = 0, e = Dest.size(); j != e; ++j) { - if (Dest[j].V != V || Dest[j].Extension != Extension) continue; - - // If we found it, subtract off Scale V's from the entry in Dest. If it - // goes to zero, remove the entry. - if (Dest[j].Scale != Scale) - Dest[j].Scale -= Scale; - else - Dest.erase(Dest.begin()+j); - Scale = 0; - break; - } - - // If we didn't consume this entry, add it to the end of the Dest list. - if (Scale) { - VariableGEPIndex Entry = { V, Extension, -Scale }; - Dest.push_back(Entry); - } - } -} - //===----------------------------------------------------------------------===// // BasicAliasAnalysis Pass //===----------------------------------------------------------------------===// @@ -482,6 +453,7 @@ namespace { virtual AliasResult alias(const Location &LocA, const Location &LocB) { + DT = 0; assert(AliasCache.empty() && "AliasCache must be cleared after use!"); assert(notDifferentParent(LocA.Ptr, LocB.Ptr) && "BasicAliasAnalysis doesn't support interprocedural queries."); @@ -492,6 +464,7 @@ namespace { // SmallDenseMap if it ever grows larger. // FIXME: This should really be shrink_to_inline_capacity_and_clear(). AliasCache.shrink_and_clear(); + VisitedPhiBBs.clear(); return Alias; } @@ -532,9 +505,41 @@ namespace { typedef SmallDenseMap AliasCacheTy; AliasCacheTy AliasCache; + /// \brief Track phi nodes we have visited. When interpret "Value" pointer + /// equality as value equality we need to make sure that the "Value" is not + /// part of a cycle. Otherwise, two uses could come from different + /// "iterations" of a cycle and see different values for the same "Value" + /// pointer. + /// The following example shows the problem: + /// %p = phi(%alloca1, %addr2) + /// %l = load %ptr + /// %addr1 = gep, %alloca2, 0, %l + /// %addr2 = gep %alloca2, 0, (%l + 1) + /// alias(%p, %addr1) -> MayAlias ! + /// store %l, ... + SmallPtrSet VisitedPhiBBs; + // Visited - Track instructions visited by pointsToConstantMemory. SmallPtrSet Visited; + // We use the dominator tree to check values can't be part of a cycle. + DominatorTree *DT; + + /// \brief Check whether two Values can be considered equivalent. + /// + /// In addition to pointer equivalence of \p V1 and \p V2 this checks + /// whether they can not be part of a cycle in the value graph by looking at + /// all visited phi nodes an making sure that the value dominates all of + /// them. + bool isValueEqual(const Value *V1, const Value *V2); + + /// \brief Dest and Src are the variable indices from two decomposed + /// GetElementPtr instructions GEP1 and GEP2 which have common base + /// pointers. Subtract the GEP2 indices from GEP1 to find the symbolic + /// difference between the two pointers. + void GetIndexDifference(SmallVectorImpl &Dest, + const SmallVectorImpl &Src); + // aliasGEP - Provide a bunch of ad-hoc rules to disambiguate a GEP // instruction against another. AliasResult aliasGEP(const GEPOperator *V1, uint64_t V1Size, @@ -1094,6 +1099,10 @@ BasicAliasAnalysis::aliasPHI(const PHINode *PN, uint64_t PNSize, const MDNode *PNTBAAInfo, const Value *V2, uint64_t V2Size, const MDNode *V2TBAAInfo) { + // Track phi nodes we have visited. We use this information when we determine + // value equivalence. + VisitedPhiBBs.insert(PN->getParent()); + // If the values are PHIs in the same block, we can do a more precise // as well as efficient check: just check for aliases between the values // on corresponding edges. @@ -1187,7 +1196,7 @@ BasicAliasAnalysis::aliasCheck(const Value *V1, uint64_t V1Size, V2 = V2->stripPointerCasts(); // Are we checking for alias of the same value? - if (V1 == V2) return MustAlias; + if (isValueEqual(V1, V2)) return MustAlias; if (!V1->getType()->isPointerTy() || !V2->getType()->isPointerTy()) return NoAlias; // Scalars cannot alias each other @@ -1307,3 +1316,69 @@ BasicAliasAnalysis::aliasCheck(const Value *V1, uint64_t V1Size, Location(V2, V2Size, V2TBAAInfo)); return AliasCache[Locs] = Result; } + +bool BasicAliasAnalysis::isValueEqual(const Value *V, const Value *V2) { + if (V != V2) + return false; + + const Instruction *Inst = dyn_cast(V); + if (!Inst) + return true; + + // Use the dominance if available. + DT = getAnalysisIfAvailable(); + if (DT) { + if (VisitedPhiBBs.size() > MaxNumPhiBBsValueDominanceCheck) + return false; + + // Make sure that the visited phis are dominated by the Value. + for (SmallPtrSet::iterator + PI = VisitedPhiBBs.begin(), + PE = VisitedPhiBBs.end(); + PI != PE; ++PI) + if (!DT->dominates(Inst, *PI)) + return false; + return true; + } + + return false; +} + +/// GetIndexDifference - Dest and Src are the variable indices from two +/// decomposed GetElementPtr instructions GEP1 and GEP2 which have common base +/// pointers. Subtract the GEP2 indices from GEP1 to find the symbolic +/// difference between the two pointers. +void BasicAliasAnalysis::GetIndexDifference( + SmallVectorImpl &Dest, + const SmallVectorImpl &Src) { + if (Src.empty()) + return; + + for (unsigned i = 0, e = Src.size(); i != e; ++i) { + const Value *V = Src[i].V; + ExtensionKind Extension = Src[i].Extension; + int64_t Scale = Src[i].Scale; + + // Find V in Dest. This is N^2, but pointer indices almost never have more + // than a few variable indexes. + for (unsigned j = 0, e = Dest.size(); j != e; ++j) { + if (!isValueEqual(Dest[j].V, V) || Dest[j].Extension != Extension) + continue; + + // If we found it, subtract off Scale V's from the entry in Dest. If it + // goes to zero, remove the entry. + if (Dest[j].Scale != Scale) + Dest[j].Scale -= Scale; + else + Dest.erase(Dest.begin() + j); + Scale = 0; + break; + } + + // If we didn't consume this entry, add it to the end of the Dest list. + if (Scale) { + VariableGEPIndex Entry = { V, Extension, -Scale }; + Dest.push_back(Entry); + } + } +} diff --git a/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll b/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll index 06a804c392f..c6a9cd9a106 100644 --- a/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll +++ b/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -basicaa -aa-eval -disable-output 2>&1 | FileCheck %s +; RUN: opt < %s -domtree -basicaa -aa-eval -disable-output 2>&1 | FileCheck %s ; TEST that A[1][0] may alias A[0][i]. target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128" diff --git a/test/Analysis/BasicAA/phi-aa.ll b/test/Analysis/BasicAA/phi-aa.ll index 6aa26c185e0..ed2d23e3adc 100644 --- a/test/Analysis/BasicAA/phi-aa.ll +++ b/test/Analysis/BasicAA/phi-aa.ll @@ -1,12 +1,20 @@ ; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s +; RUN: opt < %s -domtree -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s --check-prefix=DOM +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + ; rdar://7282591 @X = common global i32 0 @Y = common global i32 0 @Z = common global i32 0 +; CHECK-LABEL: foo ; CHECK: NoAlias: i32* %P, i32* @Z +; DOM-LABEL: foo +; DOM: NoAlias: i32* %P, i32* @Z + define void @foo(i32 %cond) nounwind { entry: %"alloca point" = bitcast i32 0 to i32 @@ -29,3 +37,49 @@ bb2: return: ret void } + +; Pointers can vary in between iterations of loops. +; PR18068 + +; CHECK-LABEL: pr18068 +; CHECK: MayAlias: i32* %0, i32* %arrayidx5 + +; DOM-LABEL: pr18068 +; DOM: MayAlias: i32* %0, i32* %arrayidx5 + +define i32 @pr18068(i32* %jj7, i32* %j) { +entry: + %oa5 = alloca [100 x i32], align 16 + br label %codeRepl + +codeRepl: + %0 = phi i32* [ %arrayidx13, %for.body ], [ %j, %entry ] + %targetBlock = call i1 @cond(i32* %jj7) + br i1 %targetBlock, label %for.body, label %bye + +for.body: + %1 = load i32* %jj7, align 4 + %idxprom4 = zext i32 %1 to i64 + %arrayidx5 = getelementptr inbounds [100 x i32]* %oa5, i64 0, i64 %idxprom4 + %2 = load i32* %arrayidx5, align 4 + %sub6 = sub i32 %2, 6 + store i32 %sub6, i32* %arrayidx5, align 4 + ; %0 and %arrayidx5 can alias! It is not safe to DSE the above store. + %3 = load i32* %0, align 4 + store i32 %3, i32* %arrayidx5, align 4 + %sub11 = add i32 %1, -1 + %idxprom12 = zext i32 %sub11 to i64 + %arrayidx13 = getelementptr inbounds [100 x i32]* %oa5, i64 0, i64 %idxprom12 + call void @inc(i32* %jj7) + br label %codeRepl + +bye: + %.reload = load i32* %jj7, align 4 + ret i32 %.reload +} + +declare i1 @cond(i32*) + +declare void @inc(i32*) + + diff --git a/test/Analysis/BasicAA/phi-spec-order.ll b/test/Analysis/BasicAA/phi-spec-order.ll index 4172d0963f3..3c9fa92363c 100644 --- a/test/Analysis/BasicAA/phi-spec-order.ll +++ b/test/Analysis/BasicAA/phi-spec-order.ll @@ -1,6 +1,6 @@ target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-f128:128:128-v128:128:128-n32:64" target triple = "powerpc64-bgq-linux" -; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s +; RUN: opt < %s -domtree -basicaa -aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s @X = external global [16000 x double], align 32 @Y = external global [16000 x double], align 32 diff --git a/test/Analysis/GlobalsModRef/aliastest.ll b/test/Analysis/GlobalsModRef/aliastest.ll index 4cfed71bfb7..864c516ec95 100644 --- a/test/Analysis/GlobalsModRef/aliastest.ll +++ b/test/Analysis/GlobalsModRef/aliastest.ll @@ -1,4 +1,4 @@ -; RUN: opt < %s -basicaa -globalsmodref-aa -gvn -S | FileCheck %s +; RUN: opt < %s -domtree -basicaa -globalsmodref-aa -gvn -S | FileCheck %s @X = internal global i32 4 ; [#uses=1] diff --git a/test/Transforms/ObjCARC/weak-copies.ll b/test/Transforms/ObjCARC/weak-copies.ll index 5dab4e049e2..599689bc360 100644 --- a/test/Transforms/ObjCARC/weak-copies.ll +++ b/test/Transforms/ObjCARC/weak-copies.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -basicaa -objc-arc < %s | FileCheck %s +; RUN: opt -S -domtree -basicaa -objc-arc < %s | FileCheck %s target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64" target triple = "x86_64-apple-darwin11.0.0" diff --git a/test/Transforms/ObjCARC/weak-dce.ll b/test/Transforms/ObjCARC/weak-dce.ll index f09467182b6..787fb905fd1 100644 --- a/test/Transforms/ObjCARC/weak-dce.ll +++ b/test/Transforms/ObjCARC/weak-dce.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -basicaa -objc-arc < %s | FileCheck %s +; RUN: opt -S -domtree -basicaa -objc-arc < %s | FileCheck %s ; rdar://11434915 ; Delete the weak calls and replace them with just the net retain.