[PATCH] D31238: [ScalarEvolution] Re-enable Predicate implication from operations

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev updated this revision to Diff 92905.
mkazantsev marked 3 inline comments as done.
mkazantsev added a comment.
This revision is now accepted and ready to land.

Fixed overprotective assertions and added a regression test on the newly detected crash related to pointers.


https://reviews.llvm.org/D31238

Files:
  lib/Analysis/ScalarEvolution.cpp
  test/Analysis/ScalarEvolution/implied-via-addition.ll


Index: test/Analysis/ScalarEvolution/implied-via-addition.ll
===================================================================
--- test/Analysis/ScalarEvolution/implied-via-addition.ll
+++ test/Analysis/ScalarEvolution/implied-via-addition.ll
@@ -25,3 +25,26 @@
  exit:
   ret void
 }
+
+define void @test_02(i8 %t) {
+; CHECK-LABEL: test_02
+ entry:
+  %t.ptr = inttoptr i8 %t to i8*
+  %p.42 = inttoptr i8 42 to i8*
+  %cmp1 = icmp slt i8* %t.ptr, %p.42
+  call void(i1, ...) @llvm.experimental.guard(i1 %cmp1) [ "deopt"() ]
+  br label %loop
+
+ loop:
+; CHECK-LABEL: loop
+  %idx = phi i8* [ %t.ptr, %entry ], [ %snext, %loop ]
+  %snext = getelementptr inbounds i8, i8* %idx, i64 1
+  %c = icmp slt i8* %idx, %p.42
+; CHECK: call void @use(i1 true)
+  call void @use(i1 %c)
+  %be = icmp slt i8* %snext, %p.42
+  br i1 %be, label %loop, label %exit
+
+ exit:
+  ret void
+}
Index: lib/Analysis/ScalarEvolution.cpp
===================================================================
--- lib/Analysis/ScalarEvolution.cpp
+++ lib/Analysis/ScalarEvolution.cpp
@@ -8573,6 +8573,12 @@
                                              const SCEV *FoundLHS,
                                              const SCEV *FoundRHS,
                                              unsigned Depth) {
+  assert(getTypeSizeInBits(LHS->getType()) ==
+             getTypeSizeInBits(RHS->getType()) &&
+         "LHS and RHS have different sizes?");
+  assert(getTypeSizeInBits(FoundLHS->getType()) ==
+             getTypeSizeInBits(FoundRHS->getType()) &&
+         "FoundLHS and FoundRHS have different sizes?");
   // We want to avoid hurting the compile time with analysis of too big trees.
   if (Depth > MaxSCEVOperationsImplicationDepth)
     return false;
@@ -8601,7 +8607,6 @@
 
   // Is the SGT predicate can be proved trivially or using the found context.
   auto IsSGTViaContext = [&](const SCEV *S1, const SCEV *S2) {
-    assert(S1->getType() == S2->getType() && "Proving for wrong types?");
     return isKnownViaSimpleReasoning(ICmpInst::ICMP_SGT, S1, S2) ||
            isImpliedViaOperations(ICmpInst::ICMP_SGT, S1, S2, OrigFoundLHS,
                                   FoundRHS, Depth + 1);
@@ -8664,17 +8669,24 @@
       if (!HasSameValue(Numerator, FoundLHS) || !isKnownPositive(Denominator))
         return false;
 
+      auto *DTy = Denominator->getType();
+      auto *FRHSTy = FoundRHS->getType();
+      if (DTy->isPointerTy() != FRHSTy->isPointerTy())
+        // One of types is a pointer and another one is not. We cannot extend
+        // them properly to a wider type, so let us just reject this case.
+        return false;
+
       // Given that:
       // FoundLHS > FoundRHS, LHS = FoundLHS / Denominator, Denominator > 0.
-      auto *Ty2 = getWiderType(Denominator->getType(), FoundRHS->getType());
-      auto *DenominatorExt = getNoopOrSignExtend(Denominator, Ty2);
-      auto *FoundRHSExt = getNoopOrSignExtend(FoundRHS, Ty2);
+      auto *WTy = getWiderType(DTy, FRHSTy);
+      auto *DenominatorExt = getNoopOrSignExtend(Denominator, WTy);
+      auto *FoundRHSExt = getNoopOrSignExtend(FoundRHS, WTy);
 
       // Try to prove the following rule:
       // (FoundRHS > Denominator - 2) && (RHS <= 0) => (LHS > RHS).
       // For example, given that FoundLHS > 2. It means that FoundLHS is at
       // least 3. If we divide it by Denominator < 4, we will have at least 1.
-      auto *DenomMinusTwo = getMinusSCEV(DenominatorExt, getConstant(Ty2, 2));
+      auto *DenomMinusTwo = getMinusSCEV(DenominatorExt, getConstant(WTy, 2));
       if (isKnownNonPositive(RHS) &&
           IsSGTViaContext(FoundRHSExt, DenomMinusTwo))
         return true;
@@ -8686,7 +8698,7 @@
       // 1. If FoundLHS is negative, then the result is 0.
       // 2. If FoundLHS is non-negative, then the result is non-negative.
       // Anyways, the result is non-negative.
-      auto *MinusOne = getNegativeSCEV(getOne(Ty2));
+      auto *MinusOne = getNegativeSCEV(getOne(WTy));
       auto *NegDenomMinusOne = getMinusSCEV(MinusOne, DenominatorExt);
       if (isKnownNegative(RHS) &&
           IsSGTViaContext(FoundRHSExt, NegDenomMinusOne))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D31238.92905.patch
Type: text/x-patch
Size: 4156 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170324/fdc6c18d/attachment.bin>