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.

diff --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
index 6f5a31d..3717fc9 100644 (file)
--- 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<const Value *, 16> 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..5e6afd9 100644 (file)
--- 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<GEPOperator>(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<GEPOperator>(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<PHINode>(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<PHINode>(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<SelectInst>(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<SelectInst>(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 (file)
--- 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 {