[PATCH] D33136: [ValueTracking] Don't let isAddOfNonZero look at adds with a PHI node as input

[Craig Topper via Phabricator via llvm-commits]

craig.topper updated this revision to Diff 99173.
craig.topper added a comment.

I couldn't come up with a bool variable name that I liked so I just taught BasicAA not to call isKnownNonEqual if one of the inputs is a PHI. Instead we'll just compute the known bits separately. I think we should add a helper method to KnownBits to check if two KnownBits structs are known non equal which would simplify the intersection check.


https://reviews.llvm.org/D33136

Files:
  lib/Analysis/BasicAliasAnalysis.cpp
  test/Transforms/GVN/pr32314.ll


Index: test/Transforms/GVN/pr32314.ll
===================================================================
--- /dev/null
+++ test/Transforms/GVN/pr32314.ll
@@ -0,0 +1,53 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -gvn < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; The load in the loop can not bypass the data from the previous loop. The store above it in the loop aliases.
+define void @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca [3 x i32], align 4
+; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
+; CHECK:       for.cond.cleanup:
+; CHECK-NEXT:    ret void
+; CHECK:       for.body:
+; CHECK-NEXT:    [[INDVARS_IV:%.*]] = phi i64 [ 1, [[ENTRY:%.*]] ], [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[P_017:%.*]] = phi i32* [ undef, [[ENTRY]] ], [ [[ARRAYIDX3:%.*]], [[FOR_BODY]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = add nsw i64 [[INDVARS_IV]], -1
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds [3 x i32], [3 x i32]* [[A]], i64 0, i64 [[TMP0]]
+; CHECK-NEXT:    store i32 50, i32* [[ARRAYIDX]], align 4
+; CHECK-NEXT:    [[TMP1:%.*]] = shl i64 [[INDVARS_IV]], 1
+; CHECK-NEXT:    [[TMP2:%.*]] = load i32, i32* [[P_017]], align 4
+; CHECK-NEXT:    [[TMP3:%.*]] = trunc i64 [[TMP1]] to i32
+; CHECK-NEXT:    [[ADD1:%.*]] = add nsw i32 [[TMP2]], [[TMP3]]
+; CHECK-NEXT:    [[ARRAYIDX3]] = getelementptr inbounds [3 x i32], [3 x i32]* [[A]], i64 0, i64 [[INDVARS_IV]]
+; CHECK-NEXT:    store i32 60, i32* [[ARRAYIDX3]], align 4
+; CHECK-NEXT:    [[INDVARS_IV_NEXT]] = add nuw nsw i64 [[INDVARS_IV]], 1
+; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp ne i64 [[INDVARS_IV_NEXT]], 3
+; CHECK-NEXT:    br i1 [[EXITCOND]], label [[FOR_BODY]], label [[FOR_COND_CLEANUP:%.*]]
+;
+entry:
+  %a = alloca [3 x i32], align 4
+  br label %for.body
+
+for.cond.cleanup:                                 ; preds = %for.body
+  ret void
+
+for.body:                                         ; preds = %for.body, %entry
+  %indvars.iv = phi i64 [ 1, %entry ], [ %indvars.iv.next, %for.body ]
+  %p.017 = phi i32* [ undef, %entry ], [ %arrayidx3, %for.body ]
+  %0 = add nsw i64 %indvars.iv, -1
+  %arrayidx = getelementptr inbounds [3 x i32], [3 x i32]* %a, i64 0, i64 %0
+  store i32 50, i32* %arrayidx, align 4
+  %1 = shl i64 %indvars.iv, 1
+  %2 = load i32, i32* %p.017, align 4
+  %3 = trunc i64 %1 to i32
+  %add1 = add nsw i32 %2, %3
+  %arrayidx3 = getelementptr inbounds [3 x i32], [3 x i32]* %a, i64 0, i64 %indvars.iv
+  store i32 60, i32* %arrayidx3, align 4
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp ne i64 %indvars.iv.next, 3
+  br i1 %exitcond, label %for.body, label %for.cond.cleanup
+}
Index: lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- lib/Analysis/BasicAliasAnalysis.cpp
+++ lib/Analysis/BasicAliasAnalysis.cpp
@@ -1008,10 +1008,22 @@
     // equal each other so we can exit early.
     if (C1 && C2)
       return NoAlias;
-    if (isKnownNonEqual(GEP1->getOperand(GEP1->getNumOperands() - 1),
-                        GEP2->getOperand(GEP2->getNumOperands() - 1),
-                        DL))
-      return NoAlias;
+    {
+      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.
+        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))
+        return NoAlias;
+    }
     return MayAlias;
   } else if (!LastIndexedStruct || !C1 || !C2) {
     return MayAlias;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33136.99173.patch
Type: text/x-patch
Size: 4274 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170516/cd66a636/attachment.bin>