diff options
author | Tom Stellard <thomas.stellard@amd.com> | 2014-04-11 19:35:37 +0000 |
---|---|---|
committer | Tom Stellard <thomas.stellard@amd.com> | 2014-04-11 19:35:37 +0000 |
commit | ffb9f967d83cf680eb1bfea1fdd995ccef34223e (patch) | |
tree | 4492da57442c30e9c641df2dd83a2f1a969a7995 /lib/Analysis | |
parent | ad17ccef24b2078275201d62c1feaa1aad6be8e3 (diff) | |
download | llvm-ffb9f967d83cf680eb1bfea1fdd995ccef34223e.tar.gz llvm-ffb9f967d83cf680eb1bfea1fdd995ccef34223e.tar.bz2 llvm-ffb9f967d83cf680eb1bfea1fdd995ccef34223e.tar.xz |
Merging r198290:
------------------------------------------------------------------------
r198290 | aschwaighofer | 2014-01-01 22:31:36 -0500 (Wed, 01 Jan 2014) | 23 lines
BasicAA: Fix value equality and phi cycles
When there are cycles in the value graph we have to be careful interpreting
"Value*" identity as "value" equivalence. We interpret the value of a phi node
as the value of its operands.
When we check for value equivalence now we make sure that the "Value*" dominates
all cycles (phis).
%0 = phi [%noaliasval, %addr2]
%l = load %ptr
%addr1 = gep @a, 0, %l
%addr2 = gep @a, 0, (%l + 1)
store %ptr ...
Before this patch we would return NoAlias for (%0, %addr1) which is wrong
because the value of the load is from different iterations of the loop.
Tested on x86_64 -mavx at O3 and O3 -flto with no performance or compile time
regressions.
PR18068
radar://15653794
------------------------------------------------------------------------
git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_34@206051 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib/Analysis')
-rw-r--r-- | lib/Analysis/BasicAliasAnalysis.cpp | 149 |
1 files changed, 112 insertions, 37 deletions
diff --git a/lib/Analysis/BasicAliasAnalysis.cpp b/lib/Analysis/BasicAliasAnalysis.cpp index b2c20110e9..14268ec391 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 <algorithm> 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<VariableGEPIndex> &Dest, - const SmallVectorImpl<VariableGEPIndex> &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<LocPair, AliasResult, 8> 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<const BasicBlock*, 8> VisitedPhiBBs; + // Visited - Track instructions visited by pointsToConstantMemory. SmallPtrSet<const Value*, 16> 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<VariableGEPIndex> &Dest, + const SmallVectorImpl<VariableGEPIndex> &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<Instruction>(V); + if (!Inst) + return true; + + // Use the dominance if available. + DT = getAnalysisIfAvailable<DominatorTree>(); + if (DT) { + if (VisitedPhiBBs.size() > MaxNumPhiBBsValueDominanceCheck) + return false; + + // Make sure that the visited phis are dominated by the Value. + for (SmallPtrSet<const BasicBlock *, 8>::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<VariableGEPIndex> &Dest, + const SmallVectorImpl<VariableGEPIndex> &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); + } + } +} |