[PATCH] D26429: [LSR] Allow formula containing Reg for SCEVAddRecExpr with loop other than current loop

[Wei Mi via llvm-commits]

wmi created this revision.
wmi added reviewers: sanjoy, atrick.
wmi added subscribers: llvm-commits, davidxl.
wmi set the repository for this revision to rL LLVM.
Herald added a subscriber: mzolotukhin.

 In RateRegister of existing LSR, if a formula contains a Reg which is a SCEVAddRecExpr, and this SCEVAddRecExpr's loop is not current loop, the formula will be marked as Loser and dropped.

Suppose we have an IR that %for.body is outerloop and %for.body2 is innerloop. LSR only handle inner loop now so only %for.body2 will be handled.

Using the logic above, formula like reg(%array) + reg({1,+, %size}<%for.body>) + 1*reg({0,+,1}<%for.body2>) will be dropped no matter what because reg({1,+, %size}<%for.body>) is a SCEVAddRecExpr type reg related with outerloop.
Only formula like reg(%array) + 1*reg({{1,+, %size}<%for.body>,+,1}<nuw><nsw><%for.body2>) will be kept because the SCEVAddRecExpr related with outerloop is folded into the initial value of the SCEVAddRecExpr related with current loop.

But in some cases, we do need to share the basic induction variable reg{0 ,+, 1}<%for.body2> among LSR Uses to reduce the final total number of induction variables used by LSR, so we don't want to drop the formula like reg(%array) + reg({1,+, %size}<%for.body>) + 1*reg({0,+,1}<%for.body2>) unconditionally.

>From the existing comment, it tries to avoid considering multiple level loops at the same time. However, existing LSR only handles innermost loop, so for any SCEVAddRecExpr with a loop other than current loop, it is an invariant and will be simple to handle. That is why the formula doesn't have to be dropped.




Repository:
  rL LLVM

https://reviews.llvm.org/D26429

Files:
  lib/Transforms/Scalar/LoopStrengthReduce.cpp
  test/Transforms/LoopStrengthReduce/nested-loop.ll


Index: test/Transforms/LoopStrengthReduce/nested-loop.ll
===================================================================
--- test/Transforms/LoopStrengthReduce/nested-loop.ll
+++ test/Transforms/LoopStrengthReduce/nested-loop.ll
@@ -0,0 +1,55 @@
+; RUN: opt -loop-reduce -disable-output -debug-only=loop-reduce < %s 2>&1 | FileCheck %s
+; REQUIRES: asserts
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @foo(i32 %size, i32 %nsteps, i32 %hsize, i32* %lined, i8* %maxarray) {
+entry:
+  %cmp215 = icmp sgt i32 %size, 1
+  %t0 = zext i32 %size to i64
+  %t1 = sext i32 %nsteps to i64
+  %sub2 = sub i64 %t0, 2
+  br label %for.body
+
+for.body:                                         ; preds = %for.inc, %entry
+  %indvars.iv2 = phi i64 [ %indvars.iv.next3, %for.inc ], [ 0, %entry ]
+  %t2 = mul nsw i64 %indvars.iv2, %t0
+  br i1 %cmp215, label %for.body2.preheader, label %for.inc
+
+for.body2.preheader:                              ; preds = %for.body
+  br label %for.body2
+
+; CHECK: After filtering out undesirable candidates:
+; CHECK: LSR is examining the following uses:
+; CHECK: LSR Use: Kind=Address
+; For ivuse with induction variables from both inner loop and outer loop, expect that the formula candidates for LSR solving contains the formula with a separate reg used for basic induction variable of inner loop.
+; CHECK: reg({1,+,(zext i32 %size to i64)}<%for.body>) + 1*reg({0,+,1}<%for.body2>)
+
+for.body2:                                        ; preds = %for.body2.preheader, %for.body2
+  %indvars.iv = phi i64 [ 1, %for.body2.preheader ], [ %indvars.iv.next, %for.body2 ]
+  %arrayidx1 = getelementptr inbounds i8, i8* %maxarray, i64 %indvars.iv
+  %v1 = load i8, i8* %arrayidx1, align 1
+  %idx2 = add nsw i64 %indvars.iv, %sub2
+  %arrayidx2 = getelementptr inbounds i8, i8* %maxarray, i64 %idx2
+  %v2 = load i8, i8* %arrayidx2, align 1
+  %tmpv = xor i8 %v1, %v2
+  %t4 = add nsw i64 %t2, %indvars.iv
+  %add.ptr = getelementptr inbounds i8, i8* %maxarray, i64 %t4
+  store i8 %tmpv, i8* %add.ptr, align 1
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %wide.trip.count = zext i32 %size to i64
+  %exitcond = icmp ne i64 %indvars.iv.next, %wide.trip.count
+  br i1 %exitcond, label %for.body2, label %for.inc.loopexit
+
+for.inc.loopexit:                                 ; preds = %for.body2
+  br label %for.inc
+
+for.inc:                                          ; preds = %for.inc.loopexit, %for.body
+  %indvars.iv.next3 = add nuw nsw i64 %indvars.iv2, 1
+  %cmp = icmp slt i64 %indvars.iv.next3, %t1
+  br i1 %cmp, label %for.body, label %for.end.loopexit
+
+for.end.loopexit:                                 ; preds = %for.inc
+  ret void
+}
Index: lib/Transforms/Scalar/LoopStrengthReduce.cpp
===================================================================
--- lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -1048,17 +1048,16 @@
                         const Loop *L,
                         ScalarEvolution &SE, DominatorTree &DT) {
   if (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(Reg)) {
-    // If this is an addrec for another loop, don't second-guess its addrec phi
-    // nodes. LSR isn't currently smart enough to reason about more than one
-    // loop at a time. LSR has already run on inner loops, will not run on outer
-    // loops, and cannot be expected to change sibling loops.
+    // If this is an addrec for another loop, it should be an invariant
+    // with respect to L since L is the innermost loop (at least
+    // for now LSR only handles innermost loops).
     if (AR->getLoop() != L) {
       // If the AddRec exists, consider it's register free and leave it alone.
       if (isExistingPhi(AR, SE))
         return;
 
-      // Otherwise, do not consider this formula at all.
-      Lose();
+      // Otherwise, it will be an invariant with respect to Loop L.
+      ++NumRegs;
       return;
     }
     AddRecCost += 1; /// TODO: This should be a function of the stride.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26429.77278.patch
Type: text/x-patch
Size: 4096 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161108/957c7286/attachment.bin>