[PATCH] D91914: [BasicAA] Generalize recursive phi alias analysis

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: asbirlea, jdoerfert, dmgreen.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
nikic requested review of this revision.

For recursive phis, we skip the recursive operands and check that the remaining operands are NoAlias with an unknown size. Currently, this is limited to inbounds GEPs with positive offsets, to guarantee that the recursion only ever increases the pointer.

Make this more general by only requiring that the underlying object of the phi operand is the phi itself, i.e. it it based on itself in some way. To compensate, we need to use a `beforeOrAfterPointer()` location size, as we no longer have the guarantee that the pointer is strictly increasing.

This allows us to handle some additional cases like negative geps, geps with dynamic offsets or geps that aren't inbounds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91914

Files:
  llvm/lib/Analysis/BasicAliasAnalysis.cpp
  llvm/test/Analysis/BasicAA/recphi.ll


Index: llvm/test/Analysis/BasicAA/recphi.ll
===================================================================
--- llvm/test/Analysis/BasicAA/recphi.ll
+++ llvm/test/Analysis/BasicAA/recphi.ll
@@ -141,10 +141,10 @@
 ; CHECK:         NoAlias:      [3 x i16]* %int_arr.10, i16** %argv.6.par
 ; CHECK:         NoAlias:      i16* %_tmp1, i16** %argv.6.par
 ; CHECK:         PartialAlias: [3 x i16]* %int_arr.10, i16* %_tmp1
-; CHECK:         MayAlias:     i16* %ls1.9.0, i16** %argv.6.par
+; CHECK:         NoAlias:      i16* %ls1.9.0, i16** %argv.6.par
 ; CHECK:         MayAlias:     [3 x i16]* %int_arr.10, i16* %ls1.9.0
 ; CHECK:         MayAlias:     i16* %_tmp1, i16* %ls1.9.0
-; CHECK:         MayAlias:     i16* %_tmp7, i16** %argv.6.par
+; CHECK:         NoAlias:      i16* %_tmp7, i16** %argv.6.par
 ; CHECK:         MayAlias:     [3 x i16]* %int_arr.10, i16* %_tmp7
 ; CHECK:         MayAlias:     i16* %_tmp1, i16* %_tmp7
 ; CHECK:         NoAlias:      i16* %_tmp7, i16* %ls1.9.0
@@ -191,9 +191,9 @@
 ; CHECK-LABEL: Function: dynamic_offset
 ; CHECK: NoAlias:  i8* %a, i8* %p.base
 ; CHECK: MayAlias: i8* %p, i8* %p.base
-; CHECK: MayAlias: i8* %a, i8* %p
+; CHECK: NoAlias:  i8* %a, i8* %p
 ; CHECK: MayAlias: i8* %p.base, i8* %p.next
-; CHECK: MayAlias: i8* %a, i8* %p.next
+; CHECK: NoAlias:  i8* %a, i8* %p.next
 ; CHECK: MayAlias: i8* %p, i8* %p.next
 define void @dynamic_offset(i1 %c, i8* noalias %p.base) {
 entry:
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1525,27 +1525,16 @@
     }
 
   SmallVector<Value *, 4> V1Srcs;
-  // For a recursive phi, that recurses through a contant gep, we can perform
-  // aliasing calculations using the other phi operands with an unknown size to
-  // specify that an unknown number of elements after the initial value are
-  // potentially accessed.
+  // If a phi operand recurses back to the phi, we can still determine NoAlias
+  // if we don't alias the underlying objects of the other phi operands, as we
+  // know that the recursive phi needs to be based on them in some way.
   bool isRecursive = false;
   auto CheckForRecPhi = [&](Value *PV) {
     if (!EnableRecPhiAnalysis)
       return false;
-    if (GEPOperator *PVGEP = dyn_cast<GEPOperator>(PV)) {
-      // Check whether the incoming value is a GEP that advances the pointer
-      // result of this PHI node (e.g. in a loop). If this is the case, we
-      // would recurse and always get a MayAlias. Handle this case specially
-      // below. We need to ensure that the phi is inbounds and has a constant
-      // positive operand so that we can check for alias with the initial value
-      // and an unknown but positive size.
-      if (PVGEP->getPointerOperand() == PN && PVGEP->isInBounds() &&
-          PVGEP->getNumIndices() == 1 && isa<ConstantInt>(PVGEP->idx_begin()) &&
-          !cast<ConstantInt>(PVGEP->idx_begin())->isNegative()) {
-        isRecursive = true;
-        return true;
-      }
+    if (getUnderlyingObject(PV) == PN) {
+      isRecursive = true;
+      return true;
     }
     return false;
   };
@@ -1591,15 +1580,11 @@
   if (V1Srcs.empty())
     return MayAlias;
 
-  // If this PHI node is recursive, set the size of the accessed memory to
-  // unknown to represent all the possible values the GEP could advance the
-  // pointer to.
+  // If this PHI node is recursive, indicate that the pointer may be moved
+  // across iterations. We can only prove NoAlias if different underlying
+  // objects are involved.
   if (isRecursive)
-    // TODO: We are checking above that the addrec GEP has a positive offset
-    // and can thus assume that all accesses happen after the base pointer.
-    // It might be better to drop the offset requirement and use
-    // beforeOrAfterPointer().
-    PNSize = LocationSize::afterPointer();
+    PNSize = LocationSize::beforeOrAfterPointer();
 
   // In the recursive alias queries below, we may compare values from two
   // different loop iterations. Keep track of visited phi blocks, which will


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91914.306851.patch
Type: text/x-patch
Size: 4169 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201121/c477caac/attachment.bin>