[PATCH] D92694: [BasicAA] Fix a bug with relational reasoning across iterations

[Philip Reames via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbfda69416c6d: [BasicAA] Fix a bug with relational reasoning across iterations (authored by reames).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92694/new/

https://reviews.llvm.org/D92694

Files:
  llvm/include/llvm/Analysis/BasicAliasAnalysis.h
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/test/Analysis/BasicAA/phi-aa.ll


Index: llvm/test/Analysis/BasicAA/phi-aa.ll
===================================================================
--- llvm/test/Analysis/BasicAA/phi-aa.ll
+++ llvm/test/Analysis/BasicAA/phi-aa.ll
@@ -173,3 +173,27 @@
 }
 
 declare void @llvm.memset.p0i8.i32(i8*, i8, i32, i1)
+
+; CHECK-LABEL: unsound_inequality
+; CHECK: MayAlias:  i32* %arrayidx13, i32* %phi
+; CHECK: MayAlias:  i32* %arrayidx5, i32* %phi
+; CHECK: NoAlias:   i32* %arrayidx13, i32* %arrayidx5
+
+; When recursively reasoning about phis, we can't use predicates between
+; two values as we might be comparing the two from different iterations.
+define i32 @unsound_inequality(i32* %jj7, i32* %j) {
+entry:
+  %oa5 = alloca [100 x i32], align 16
+  br label %for.body
+
+for.body:                                         ; preds = %for.body, %entry
+  %phi = phi i32* [ %arrayidx13, %for.body ], [ %j, %entry ]
+  %idx = load i32, i32* %jj7, align 4
+  %arrayidx5 = getelementptr inbounds [100 x i32], [100 x i32]* %oa5, i64 0, i32 %idx
+  store i32 0, i32* %arrayidx5, align 4
+  store i32 0, i32* %phi, align 4
+  %notequal = add i32 %idx, 1
+  %arrayidx13 = getelementptr inbounds [100 x i32], [100 x i32]* %oa5, i64 0, i32 %notequal
+  store i32 0, i32* %arrayidx13, align 4
+  br label %for.body
+} 
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1032,11 +1032,11 @@
 
 /// Provide ad-hoc rules to disambiguate accesses through two GEP operators,
 /// both having the exact same pointer operand.
-static AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,
-                                            LocationSize MaybeV1Size,
-                                            const GEPOperator *GEP2,
-                                            LocationSize MaybeV2Size,
-                                            const DataLayout &DL) {
+AliasResult BasicAAResult::aliasSameBasePointerGEPs(const GEPOperator *GEP1,
+                                                    LocationSize MaybeV1Size,
+                                                    const GEPOperator *GEP2,
+                                                    LocationSize MaybeV2Size,
+                                                    const DataLayout &DL) {
   assert(GEP1->getPointerOperand()->stripPointerCastsAndInvariantGroups() ==
              GEP2->getPointerOperand()->stripPointerCastsAndInvariantGroups() &&
          GEP1->getPointerOperandType() == GEP2->getPointerOperandType() &&
@@ -1126,24 +1126,12 @@
     if (C1 && C2)
       return NoAlias;
     {
+      // If we're not potentially reasoning about values from different
+      // iterations, see if we can prove them inequal.
       Value *GEP1LastIdx = GEP1->getOperand(GEP1->getNumOperands() - 1);
       Value *GEP2LastIdx = GEP2->getOperand(GEP2->getNumOperands() - 1);
-      if (isa<PHINode>(GEP1LastIdx) || isa<PHINode>(GEP2LastIdx)) {
-        // If one of the indices is a PHI node, be safe and only use
-        // computeKnownBits so we don't make any assumptions about the
-        // relationships between the two indices. This is important if we're
-        // asking about values from different loop iterations. See PR32314.
-        // TODO: We may be able to change the check so we only do this when
-        // we definitely looked through a PHINode.
-        if (GEP1LastIdx != GEP2LastIdx &&
-            GEP1LastIdx->getType() == GEP2LastIdx->getType()) {
-          KnownBits Known1 = computeKnownBits(GEP1LastIdx, DL);
-          KnownBits Known2 = computeKnownBits(GEP2LastIdx, DL);
-          if (Known1.Zero.intersects(Known2.One) ||
-              Known1.One.intersects(Known2.Zero))
-            return NoAlias;
-        }
-      } else if (isKnownNonEqual(GEP1LastIdx, GEP2LastIdx, DL))
+      if (VisitedPhiBBs.empty() &&
+          isKnownNonEqual(GEP1LastIdx, GEP2LastIdx, DL))
         return NoAlias;
     }
   }
Index: llvm/include/llvm/Analysis/BasicAliasAnalysis.h
===================================================================
--- llvm/include/llvm/Analysis/BasicAliasAnalysis.h
+++ llvm/include/llvm/Analysis/BasicAliasAnalysis.h
@@ -202,6 +202,12 @@
       const DecomposedGEP &DecompGEP, const DecomposedGEP &DecompObject,
       LocationSize ObjectAccessSize);
 
+  AliasResult aliasSameBasePointerGEPs(const GEPOperator *GEP1,
+                                       LocationSize MaybeV1Size,
+                                       const GEPOperator *GEP2,
+                                       LocationSize MaybeV2Size,
+                                       const DataLayout &DL);
+
   /// A Heuristic for aliasGEP that searches for a constant offset
   /// between the variables.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92694.309748.patch
Type: text/x-patch
Size: 4828 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201205/6e94c207/attachment.bin>