[PATCH] D39453: [SCEV] Strengthen variance condition in calculateLoopDisposition

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.

Given loops `L1` and `L2` with AddRecs `AR1` and `AR2` varying in them respectively.
When identifying loop disposition of `AR2` w.r.t. `L1`, we only say that it is varying if
`L1` contains `L2`. But there is also a possible situation where `L1` and `L2` are
consecutive sibling loops within the parent loop. In this case, `AR2` is also varying
w.r.t. `L1`, but we don't correctly identify it.

It can lead, for exaple, to attempt of incorrect folding. Consider:

  AR1 = {a,+,b}<L1>
  AR2 = {c,+,d}<L2>
  EXAR2 = sext(AR1)
  MUL = mul AR1, EXAR2

If we incorrectly assume that `EXAR2` is invariant w.r.t. `L1`, we can end up trying to
construct something like: `{a * {c,+,d}<L2>,+,b * {c,+,d}<L2>}<L1>`, which is incorrect
because `AR2` is not available on entrance of `L1`.

Both situations "`L1` contains `L2`" and "`L1` preceeds sibling loop `L2`" can be handled
with one check: "header of `L1` dominates header of `L2`". This patch replaces the old
insufficient check with this one.


https://reviews.llvm.org/D39453

Files:
  lib/Analysis/ScalarEvolution.cpp
  test/Analysis/ScalarEvolution/different-loops-recs.ll


Index: test/Analysis/ScalarEvolution/different-loops-recs.ll
===================================================================
--- test/Analysis/ScalarEvolution/different-loops-recs.ll
+++ test/Analysis/ScalarEvolution/different-loops-recs.ll
@@ -510,3 +510,56 @@
   %tmp10 = add i32 %iv.1.2, 3
   ret void
 }
+
+define i64 @test_09(i32 %param) {
+
+; CHECK-LABEL: Classifying expressions for: @test_09
+; CHECK:       %iv1 = phi i64 [ %iv1.next, %guarded ], [ 0, %outer.loop ]
+; CHECK-NEXT:    -->  {0,+,1}<nuw><nsw><%loop1>
+; CHECK:       %iv1.trunc = trunc i64 %iv1 to i32
+; CHECK-NEXT:    -->  {0,+,1}<%loop1>
+; CHECK:       %iv1.next = add nuw nsw i64 %iv1, 1
+; CHECK-NEXT:    -->  {1,+,1}<nuw><nsw><%loop1>
+; CHECK:       %iv2 = phi i32 [ %iv2.next, %loop2 ], [ %param, %loop2.preheader ]
+; CHECK-NEXT:    -->  {%param,+,1}<%loop2>
+; CHECK:       %iv2.next = add i32 %iv2, 1
+; CHECK-NEXT:    -->  {(1 + %param),+,1}<%loop2>
+; CHECK:       %iv2.ext = sext i32 %iv2.next to i64
+; CHECK-NEXT:    -->  (sext i32 {(1 + %param),+,1}<%loop2> to i64)
+; CHECK:       %ret = mul i64 %iv1, %iv2.ext
+; CHECK-NEXT:    -->  ((sext i32 {(1 + %param),+,1}<%loop2> to i64) * {0,+,1}<nuw><nsw><%loop1>)
+
+entry:
+  br label %outer.loop
+
+outer.loop:                                 ; preds = %loop2.exit, %entry
+  br label %loop1
+
+loop1:                                           ; preds = %guarded, %outer.loop
+  %iv1 = phi i64 [ %iv1.next, %guarded ], [ 0, %outer.loop ]
+  %iv1.trunc = trunc i64 %iv1 to i32
+  %cond1 = icmp ult i64 %iv1, 100
+  br i1 %cond1, label %guarded, label %deopt
+
+guarded:                                          ; preds = %loop1
+  %iv1.next = add nuw nsw i64 %iv1, 1
+  %tmp16 = icmp slt i32 %iv1.trunc, 2
+  br i1 %tmp16, label %loop1, label %loop2.preheader
+
+deopt:                                            ; preds = %loop1
+  unreachable
+
+loop2.preheader:                                 ; preds = %guarded
+  br label %loop2
+
+loop2:                                           ; preds = %loop2, %loop2.preheader
+  %iv2 = phi i32 [ %iv2.next, %loop2 ], [ %param, %loop2.preheader ]
+  %iv2.next = add i32 %iv2, 1
+  %cond2 = icmp slt i32 %iv2, %iv1.trunc
+  br i1 %cond2, label %loop2, label %exit
+
+exit:                                          ; preds = %loop2.exit
+  %iv2.ext = sext i32 %iv2.next to i64
+  %ret = mul i64 %iv1, %iv2.ext
+  ret i64 %ret
+}
Index: lib/Analysis/ScalarEvolution.cpp
===================================================================
--- lib/Analysis/ScalarEvolution.cpp
+++ lib/Analysis/ScalarEvolution.cpp
@@ -10847,9 +10847,13 @@
     if (!L)
       return LoopVariant;
 
-    // This recurrence is variant w.r.t. L if L contains AR's loop.
-    if (L->contains(AR->getLoop()))
+    // If AR's loop is dominated by L, it's variant. This may happen if:
+    // a) L contains AR's loop;
+    // b) L and AR's loop are consecutive.
+    if (DT.dominates(L->getHeader(), AR->getLoop()->getHeader()))
       return LoopVariant;
+    assert(!L->contains(AR->getLoop()) && "Containing loop's header does not"
+           " dominate the contained loop's header?");
 
     // This recurrence is invariant w.r.t. L if AR's loop contains L.
     if (AR->getLoop()->contains(L))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39453.120959.patch
Type: text/x-patch
Size: 3259 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171031/7f07d972/attachment.bin>