[LLVMdev] Why should we have the LoopPass and LoopPassManager? Can we get rid of this complexity?

[Andrew Trick]

On Jan 22, 2014, at 4:01 AM, Chandler Carruth <chandlerc at gmail.com> wrote:

> 
> On Wed, Jan 22, 2014 at 3:39 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> I have a patch that does #1 already, but wanted to check that you're OK weakening the verification. Otherwise, I have to do 1, 2, 3, and 5 in a single commit, or teach the LoopVectorizer and LSR to preserve LoopSimplify... Yuck.
> 
> This patch appears to cause slight changes in three test cases:
> 
>     LLVM :: Transforms/IndVarSimplify/lftr-reuse.ll
>     LLVM :: Transforms/LoopSimplify/ashr-crash.ll
>     LLVM :: Transforms/LoopStrengthReduce/2011-12-19-PostincQuadratic.ll
> 
> Looking at lftr-reuse.ll, we successfully hoist the 'icmp slt' into the 'outer:' block as the comment says would be nice (because the outer loop is simplified now, the test is checking for unsimplified). The LSR failure is just that the loop basic blocks have different names (loopexit instead of preheader).
> 
> The ashr-crash.ll case is minutely interesting -- we fail to hoist the comparison that the test wants hoisted into the entry block. My suspicion is that getting this to hoist with the heavily reduced pipeline used is problematic, as the test seems more geared to tickle SCEV bugs than test important optimization invariants.

This looks like an example of the kind of order-of-loop-transform problems that I spent a staggering amount of time debugging. So the underlying problem should go away.

That said, it also looks like an example where we benefit from applying multiple passes to the inner loop before optimizing the outer loop. If you want to file a PR and disable it for now that’s cool.

-Andy

> All of the other regression tests pass, so this looks pretty good to me. Let me know! I can mail the patch tomorrow if it helps.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140122/ee6f3e79/attachment.html>