[PATCH] D97401: [basicaa] Recurse through a single phi input

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: nikic, fhahn.
Herald added subscribers: dantrushin, hiraditya, mcrosier.
Herald added a reviewer: bollu.
reames requested review of this revision.
Herald added a project: LLVM.

BasicAA knows how to analyze phis, but to control compile time, we're fairly limited in doing so.  This patch looses that restriction when there is exactly one phi input (after discounting induction variable increments).  The result of this is that we can handle more cases around nested and sibling loops with pointer induction variables.

A few points to note.

1. I could be even more restrictive here and restrict this to exactly one incoming value.  (Right now, I allow one phi, and an arbitrary number of other values.)  I don't have a strong preference here, reviewer comments welcome.

2. As seen in the test file, we're still missing cases which aren't *directly* based on phis (e.g. using the indvar increment).  I believe this to be a separate problem and am going to explore this in another patch once this one lands.

3. As seen in the test file, this results in the unfortunate fact that using phivalues sometimes results in worse quality results.  I believe this comes down to an oversight in how recursive phi detection was implemented for phivalues.  I'm happy to tackle this in a follow up change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97401

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
@@ -296,9 +296,9 @@
 ; CHECK: NoAlias:	i8* %a, i8* %p.base
 ; CHECK: NoAlias:	i8* %a, i8* %p.outer
 ; CHECK: NoAlias:	i8* %a, i8* %p.outer.next
-; CHECK: MayAlias:	i8* %a, i8* %p.inner
+; NO-PHI-VALUES: NoAlias:	i8* %a, i8* %p.inner
+; PHI-VALUES: MayAlias:	i8* %a, i8* %p.inner
 ; CHECK: NoAlias:	i8* %a, i8* %p.inner.next
-; TODO: %p.inner does not alias %a
 define void @nested_loop3(i1 %c, i1 %c2, i8* noalias %p.base) {
 entry:
   %a = alloca i8
@@ -351,9 +351,9 @@
 ; CHECK: NoAlias:	i8* %a, i8* %p.base
 ; CHECK: NoAlias:	i8* %a, i8* %p1
 ; CHECK: NoAlias:	i8* %a, i8* %p1.next
-; CHECK: MayAlias:	i8* %a, i8* %p2
+; NO-PHI-VALUES: NoAlias:	i8* %a, i8* %p2
+; PHI-VALUES: MayAlias:	i8* %a, i8* %p2
 ; CHECK: NoAlias:	i8* %a, i8* %p2.next
-; TODO: %p2 does not alias %a
 define void @sibling_loop2(i1 %c, i1 %c2, i8* noalias %p.base) {
 entry:
   %a = alloca i8
Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1385,13 +1385,19 @@
     // If we don't have PhiInfo then just look at the operands of the phi itself
     // FIXME: Remove this once we can guarantee that we have PhiInfo always
     SmallPtrSet<Value *, 4> UniqueSrc;
+    Value *OnePhi = nullptr;
     for (Value *PV1 : PN->incoming_values()) {
-      if (isa<PHINode>(PV1))
-        // If any of the source itself is a PHI, return MayAlias conservatively
-        // to avoid compile time explosion. The worst possible case is if both
-        // sides are PHI nodes. In which case, this is O(m x n) time where 'm'
-        // and 'n' are the number of PHI sources.
-        return MayAlias;
+      if (isa<PHINode>(PV1)) {
+        if (OnePhi && OnePhi != PV1) {
+          // To control potential compile time explosion, we choose to be
+          // conserviate when we have more than one Phi input.  It is important
+          // that we handle the single phi case as that lets us handle LCSSA
+          // phi nodes and (combined with the recursive phi handling) simple
+          // pointer induction variable patterns.
+          return MayAlias;
+        }
+        OnePhi = PV1;
+      }
 
       if (CheckForRecPhi(PV1))
         continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97401.326138.patch
Type: text/x-patch
Size: 2476 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210224/9851f4e1/attachment.bin>