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

[Wei Mi via Phabricator via llvm-commits]

wmi updated this revision to Diff 87563.
wmi added subscribers: rengolin, t.p.northover, jmolloy.
wmi added a comment.

First, sorry to revisit the patch after long time.

The patch was reverted after being commited because of the testcase failure: test/CodeGen/AArch64/eliminate-trunc.ll. 
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20161212/412459.html.

Here is some explaination about how the patch broke the testcase and how it was fixed:

How the patch affect the testcase:

  By allowing fomula containing SCEVAddRecExpr Reg from outer loop, LSR has more options to choose than before and it chooses to use fewer induction variables. Without the patch, two induction vars will be used. With the patch, only one induction var will be used.

Without the patch:

  Before LSR:
    There is only one induction variable being used. The induction var will both be used in array suffix expr and also used in the compare expr in loop latch (other than in the array suffix expr, the induction var is used as a 32bit var), so it should be an 64 bit var.
  
  After LSR:
    Two induction vars will be generated. One is used in array suffix expr. The other is used as loop iteration counter. They are separated from each other so the loop iteration counter can be a 32 bit var. That is what CHECK in the testcase is looking for.
     Note in asm, addrec instruction for the induction var used in array suffix expr is absorbed by post-increment load/store.

With the patch:

  After LSR:
    Only one induction var will be used. Since it is both used in array suffix expr and used as loop iteration counter, it should be a 64 bit var. That is how the patch broke the testcase.

To make the testcase remain valid, I change the testcase and stop using outerloop induction var in array suffix expr, so we won't have fomula containing SCEVAddRecExpr Reg from outer loop, i.e., the patch will stop kicking in for the testcase.

Is it ok to fix the testcase like this and recommit the patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D26429

Files:
  lib/Transforms/Scalar/LoopStrengthReduce.cpp
  test/CodeGen/AArch64/eliminate-trunc.ll
  test/CodeGen/X86/licm-nested.ll
  test/Transforms/LoopStrengthReduce/X86/nested-loop.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26429.87563.patch
Type: text/x-patch
Size: 6484 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170208/5efd89d1/attachment.bin>