[ValueTracking] Improve ananlysis of PHI nodes.

[mgudim]

In general, phi node p = phi [%v0, %bb0], ..., [%v_n, %bb_n] is not equal to %x if each of %v0, ..., %v_n is not equal to %x. If x happens to be also a phi node in the same basic block, we check that that the incomming values for the same predecessors are not equal.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Mikhail Gudim (mgudim)

Changes

In general, phi node p = phi [%v0, %bb0], ..., [%v_n, %bb_n] is not equal to %x if each of %v0, ..., %v_n is not equal to %x. If x happens to be also a phi node in the same basic block, we check that that the incomming values for the same predecessors are not equal.

---

Full diff: https://github.com/llvm/llvm-project/pull/71224.diff

2 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+26-22)
• (added) llvm/test/Analysis/BasicAA/non-equal-phi.ll (+123)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index a53f2f68cfab3..894dff6ca7ee2 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -3020,32 +3020,41 @@ static bool isNonEqualShl(const Value *V1, const Value *V2, unsigned Depth,
   return false;
 }
 
-static bool isNonEqualPHIs(const PHINode *PN1, const PHINode *PN2,
-                           unsigned Depth, const SimplifyQuery &Q) {
-  // Check two PHIs are in same block.
-  if (PN1->getParent() != PN2->getParent())
-    return false;
-
+static bool isNonEqualPhisSameBB(const PHINode *PN1, const PHINode *PN2,
+                                 unsigned Depth, const SimplifyQuery &Q) {
   SmallPtrSet<const BasicBlock *, 8> VisitedBBs;
-  bool UsedFullRecursion = false;
   for (const BasicBlock *IncomBB : PN1->blocks()) {
     if (!VisitedBBs.insert(IncomBB).second)
       continue; // Don't reprocess blocks that we have dealt with already.
     const Value *IV1 = PN1->getIncomingValueForBlock(IncomBB);
     const Value *IV2 = PN2->getIncomingValueForBlock(IncomBB);
-    const APInt *C1, *C2;
-    if (match(IV1, m_APInt(C1)) && match(IV2, m_APInt(C2)) && *C1 != *C2)
-      continue;
 
-    // Only one pair of phi operands is allowed for full recursion.
-    if (UsedFullRecursion)
+    SimplifyQuery RecQ = Q;
+    RecQ.CxtI = IncomBB->getTerminator();
+    if (!isKnownNonEqual(IV1, IV2, Depth + 1, RecQ))
       return false;
+  }
+  return true;
+}
+
+static bool isNonEqualPhi(const Value *V1, const Value *V2, unsigned Depth,
+                          const SimplifyQuery &Q) {
+  const PHINode *PN1 = dyn_cast<PHINode>(V1);
+  if (!PN1)
+    return false;
 
+  if (const PHINode *PN2 = dyn_cast<PHINode>(V2)) {
+    if (PN1->getParent() == PN2->getParent())
+      return isNonEqualPhisSameBB(PN1, PN2, Depth, Q);
+  }
+  for (const BasicBlock *IncomBB : PN1->blocks()) {
+    Value *V = PN1->getIncomingValueForBlock(IncomBB);
+    if (V == PN1)
+      continue;
     SimplifyQuery RecQ = Q;
     RecQ.CxtI = IncomBB->getTerminator();
-    if (!isKnownNonEqual(IV1, IV2, Depth + 1, RecQ))
+    if (!isKnownNonEqual(V, V2, Depth + 1, RecQ))
       return false;
-    UsedFullRecursion = true;
   }
   return true;
 }
@@ -3089,14 +3098,6 @@ static bool isKnownNonEqual(const Value *V1, const Value *V2, unsigned Depth,
   if (O1 && O2 && O1->getOpcode() == O2->getOpcode()) {
     if (auto Values = getInvertibleOperands(O1, O2))
       return isKnownNonEqual(Values->first, Values->second, Depth + 1, Q);
-
-    if (const PHINode *PN1 = dyn_cast<PHINode>(V1)) {
-      const PHINode *PN2 = cast<PHINode>(V2);
-      // FIXME: This is missing a generalization to handle the case where one is
-      // a PHI and another one isn't.
-      if (isNonEqualPHIs(PN1, PN2, Depth, Q))
-        return true;
-    };
   }
 
   if (isAddOfNonZero(V1, V2, Depth, Q) || isAddOfNonZero(V2, V1, Depth, Q))
@@ -3122,6 +3123,9 @@ static bool isKnownNonEqual(const Value *V1, const Value *V2, unsigned Depth,
   if (isNonEqualSelect(V1, V2, Depth, Q) || isNonEqualSelect(V2, V1, Depth, Q))
     return true;
 
+  if (isNonEqualPhi(V1, V2, Depth, Q) || isNonEqualPhi(V2, V1, Depth, Q))
+    return true;
+
   return false;
 }
 
diff --git a/llvm/test/Analysis/BasicAA/non-equal-phi.ll b/llvm/test/Analysis/BasicAA/non-equal-phi.ll
new file mode 100644
index 0000000000000..29b13a6004188
--- /dev/null
+++ b/llvm/test/Analysis/BasicAA/non-equal-phi.ll
@@ -0,0 +1,123 @@
+; RUN: opt < %s -aa-pipeline=basic-aa -passes=aa-eval -print-all-alias-modref-info -disable-output 2>&1 | FileCheck %s
+
+@a = external local_unnamed_addr global ptr, align 8
+
+
+define void @no_alias_phi(i1 %c, i64 %x) {
+; CHECK-LABEL: no_alias_phi
+; CHECK:  NoAlias:  i32* %arrayidx0, i32* %arrayidx1
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  %add1_ = add nsw i64 %x, 1
+  br label %end
+
+bb2:
+  %add2_ = add nsw i64 %x, 2
+  br label %end
+
+end:
+  %cond = phi i64 [%add1_, %bb1 ], [ %add2_, %bb2 ]
+  %arrayidx0 = getelementptr inbounds i32, ptr @a, i64 %cond
+  %arrayidx1 = getelementptr inbounds i32, ptr @a, i64 %x
+  store i32 123, ptr %arrayidx0, align 4
+  store i32 456, ptr %arrayidx1, align 4
+  ret void
+}
+
+define void @may_alias_phi(i1 %c, i64 %x) {
+; CHECK-LABEL: may_alias_phi
+; CHECK:  MayAlias:  i32* %arrayidx0, i32* %arrayidx1
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  %add1_ = add nsw i64 %x, 1
+  br label %end
+
+bb2:
+  br label %end
+
+end:
+  %cond = phi i64 [%add1_, %bb1 ], [ %x, %bb2 ]
+  %arrayidx0 = getelementptr inbounds i32, ptr @a, i64 %cond
+  %arrayidx1 = getelementptr inbounds i32, ptr @a, i64 %x
+  store i32 123, ptr %arrayidx0, align 4
+  store i32 456, ptr %arrayidx1, align 4
+  ret void
+}
+
+define void @no_alias_phi_same_bb(i1 %c, i64 %x) {
+; CHECK-LABEL: no_alias_phi_same_bb
+; CHECK:  NoAlias:  i32* %arrayidx0, i32* %arrayidx1
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  %add1_ = add nsw i64 %x, 1
+  %add3_ = add nsw i64 %x, 3
+  br label %end
+
+bb2:
+  %add2_ = add nsw i64 %x, 2
+  %add4_ = add nsw i64 %x, 4
+  br label %end
+
+end:
+  %phi1_ = phi i64 [%add1_, %bb1 ], [ %add2_, %bb2 ]
+  %phi2_ = phi i64 [%add3_, %bb1 ], [ %add4_, %bb2 ]
+  %arrayidx0 = getelementptr inbounds i32, ptr @a, i64 %phi1_
+  %arrayidx1 = getelementptr inbounds i32, ptr @a, i64 %phi2_
+  store i32 123, ptr %arrayidx0, align 4
+  store i32 456, ptr %arrayidx1, align 4
+  ret void
+}
+
+define void @no_alias_phi_same_bb_2(i1 %c, i64 %x) {
+; CHECK-LABEL: no_alias_phi_same_bb_2
+; CHECK:  NoAlias:  i32* %arrayidx0, i32* %arrayidx1
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  %add1_ = add nsw i64 %x, 1
+  br label %end
+
+bb2:
+  %add2_ = add nsw i64 %x, 2
+  br label %end
+
+end:
+  %phi1_ = phi i64 [%x, %bb1 ], [ %add2_, %bb2 ]
+  %phi2_ = phi i64 [%add1_, %bb1 ], [ %x, %bb2 ]
+  %arrayidx0 = getelementptr inbounds i32, ptr @a, i64 %phi1_
+  %arrayidx1 = getelementptr inbounds i32, ptr @a, i64 %phi2_
+  store i32 123, ptr %arrayidx0, align 4
+  store i32 456, ptr %arrayidx1, align 4
+  ret void
+}
+
+define void @may_alias_phi_same_bb(i1 %c, i64 %x) {
+; CHECK-LABEL: may_alias_phi_same_bb
+; CHECK:  MayAlias:  i32* %arrayidx0, i32* %arrayidx1
+entry:
+  br i1 %c, label %bb1, label %bb2
+
+bb1:
+  br label %end
+
+bb2:
+  %add2_ = add nsw i64 %x, 2
+  %add4_ = add nsw i64 %x, 4
+  br label %end
+
+end:
+  %phi1_ = phi i64 [%x, %bb1 ], [ %add2_, %bb2 ]
+  %phi2_ = phi i64 [%x, %bb1 ], [ %add4_, %bb2 ]
+  %arrayidx0 = getelementptr inbounds i32, ptr @a, i64 %phi1_
+  %arrayidx1 = getelementptr inbounds i32, ptr @a, i64 %phi2_
+  store i32 123, ptr %arrayidx0, align 4
+  store i32 456, ptr %arrayidx1, align 4
+  ret void
+}

[nikic]

This is incorrect. V and V2 here can be values from different cycle iterations.