From fa103836649c8c37cf4e49523d3448c1ae1a160f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Martin=20Storsj=C3=B6?= Date: Fri, 27 Nov 2020 21:38:39 +0200 Subject: [PATCH] Revert "[BasicAA] Fix BatchAA results for phi-phi assumptions" This reverts commit 8166ed1a7a26ee8ea8db9005cc8ee5d156adad9b, as it caused some compilations to hang/loop indefinitely, see https://reviews.llvm.org/D91936 for details. --- llvm/include/llvm/Analysis/BasicAliasAnalysis.h | 10 ----- llvm/lib/Analysis/BasicAliasAnalysis.cpp | 50 ++++++++----------------- llvm/unittests/Analysis/AliasAnalysisTest.cpp | 3 +- 3 files changed, 18 insertions(+), 45 deletions(-) diff --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h index 6f5a31d..3717fc9 100644 --- a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h +++ b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h @@ -162,10 +162,6 @@ private: /// Tracks instructions visited by pointsToConstantMemory. SmallPtrSet Visited; - /// Whether to disable persistent caching in AAQI. This is used to prevent - /// caching of results based on temporary assumptions. - bool DisableCache = false; - static const Value * GetLinearExpression(const Value *V, APInt &Scale, APInt &Offset, unsigned &ZExtBits, unsigned &SExtBits, @@ -220,12 +216,6 @@ private: LocationSize V2Size, const AAMDNodes &V2AATag, AAQueryInfo &AAQI, const Value *O1 = nullptr, const Value *O2 = nullptr); - - AliasResult aliasCheckRecursive(const Value *V1, LocationSize V1Size, - const AAMDNodes &V1AATag, const Value *V2, - LocationSize V2Size, const AAMDNodes &V2AATag, - AAQueryInfo &AAQI, const Value *O1, - const Value *O2); }; /// Analysis pass providing a never-invalidated alias analysis result. diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp index 0f13f98..5e6afd90 100644 --- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp +++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp @@ -1489,10 +1489,8 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize, // operand from outside the PHIs' cycle that is MayAlias/MustAlias or // there must be an operation on the PHIs within the PHIs' value cycle // that causes a MayAlias. - // Disable persistent caching, so intermediate results based on a - // possibly incorrect assumption do not get cached. - bool OrigDisableCache = DisableCache; - DisableCache = true; + // Pretend the phis do not alias. + AliasResult Alias = NoAlias; AliasResult OrigAliasResult; { // Limited lifetime iterator invalidated by the aliasCheck call below. @@ -1503,7 +1501,6 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize, CacheIt->second = NoAlias; } - AliasResult Alias = NoAlias; for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) { AliasResult ThisAlias = aliasCheck(PN->getIncomingValue(i), PNSize, PNAAInfo, @@ -1517,7 +1514,6 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize, // Reset if speculation failed. if (Alias != NoAlias) AAQI.updateResult(Locs, OrigAliasResult); - DisableCache = OrigDisableCache; return Alias; } @@ -1757,73 +1753,59 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size, if (!Pair.second) return Pair.first->second; - AliasResult Result = aliasCheckRecursive(V1, V1Size, V1AAInfo, V2, V2Size, - V2AAInfo, AAQI, O1, O2); - - // If caching is disabled, remove the entry once the recursive checks are - // done. We only needed it to prevent infinite recursion. - if (DisableCache) - AAQI.AliasCache.erase(AAQI.AliasCache.find(Locs)); - else if (Result != MayAlias) - AAQI.updateResult(Locs, Result); - return Result; -} - -AliasResult BasicAAResult::aliasCheckRecursive( - const Value *V1, LocationSize V1Size, const AAMDNodes &V1AAInfo, - const Value *V2, LocationSize V2Size, const AAMDNodes &V2AAInfo, - AAQueryInfo &AAQI, const Value *O1, const Value *O2) { if (const GEPOperator *GV1 = dyn_cast(V1)) { AliasResult Result = aliasGEP(GV1, V1Size, V1AAInfo, V2, V2Size, V2AAInfo, O1, O2, AAQI); if (Result != MayAlias) - return Result; + return AAQI.updateResult(Locs, Result); } else if (const GEPOperator *GV2 = dyn_cast(V2)) { AliasResult Result = aliasGEP(GV2, V2Size, V2AAInfo, V1, V1Size, V1AAInfo, O2, O1, AAQI); if (Result != MayAlias) - return Result; + return AAQI.updateResult(Locs, Result); } if (const PHINode *PN = dyn_cast(V1)) { AliasResult Result = aliasPHI(PN, V1Size, V1AAInfo, V2, V2Size, V2AAInfo, O2, AAQI); if (Result != MayAlias) - return Result; + return AAQI.updateResult(Locs, Result); } else if (const PHINode *PN = dyn_cast(V2)) { AliasResult Result = aliasPHI(PN, V2Size, V2AAInfo, V1, V1Size, V1AAInfo, O1, AAQI); if (Result != MayAlias) - return Result; + return AAQI.updateResult(Locs, Result); } if (const SelectInst *S1 = dyn_cast(V1)) { AliasResult Result = aliasSelect(S1, V1Size, V1AAInfo, V2, V2Size, V2AAInfo, O2, AAQI); if (Result != MayAlias) - return Result; + return AAQI.updateResult(Locs, Result); } else if (const SelectInst *S2 = dyn_cast(V2)) { AliasResult Result = aliasSelect(S2, V2Size, V2AAInfo, V1, V1Size, V1AAInfo, O1, AAQI); if (Result != MayAlias) - return Result; + return AAQI.updateResult(Locs, Result); } // If both pointers are pointing into the same object and one of them // accesses the entire object, then the accesses must overlap in some way. - if (O1 == O2) { - bool NullIsValidLocation = NullPointerIsDefined(&F); + if (O1 == O2) if (V1Size.isPrecise() && V2Size.isPrecise() && (isObjectSize(O1, V1Size.getValue(), DL, TLI, NullIsValidLocation) || isObjectSize(O2, V2Size.getValue(), DL, TLI, NullIsValidLocation))) - return PartialAlias; - } + return AAQI.updateResult(Locs, PartialAlias); // Recurse back into the best AA results we have, potentially with refined // memory locations. We have already ensured that BasicAA has a MayAlias // cache result for these, so any recursion back into BasicAA won't loop. - return getBestAAResults().alias(MemoryLocation(V1, V1Size, V1AAInfo), - MemoryLocation(V2, V2Size, V2AAInfo), AAQI); + AliasResult Result = getBestAAResults().alias(Locs.first, Locs.second, AAQI); + if (Result != MayAlias) + return AAQI.updateResult(Locs, Result); + + // MayAlias is already in the cache. + return MayAlias; } /// Check whether two Values can be considered equivalent. diff --git a/llvm/unittests/Analysis/AliasAnalysisTest.cpp b/llvm/unittests/Analysis/AliasAnalysisTest.cpp index 472ac9f..cfdada6 100644 --- a/llvm/unittests/Analysis/AliasAnalysisTest.cpp +++ b/llvm/unittests/Analysis/AliasAnalysisTest.cpp @@ -295,7 +295,8 @@ TEST_F(AliasAnalysisTest, BatchAAPhiAssumption) { BatchAAResults BatchAA(AA); EXPECT_EQ(MayAlias, BatchAA.alias(ALoc, BLoc)); - EXPECT_EQ(MayAlias, BatchAA.alias(ANextLoc, BNextLoc)); + // TODO: This is incorrect. + EXPECT_EQ(NoAlias, BatchAA.alias(ANextLoc, BNextLoc)); } class AAPassInfraTest : public testing::Test { -- 2.7.4