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

[Hal Finkel]

----- Original Message -----
> From: "Andrew Trick" <atrick at apple.com>
> To: "Chandler Carruth" <chandlerc at gmail.com>
> Cc: "LLVM Developers Mailing List" <llvmdev at cs.uiuc.edu>
> Sent: Wednesday, January 22, 2014 2:09:06 PM
> Subject: Re: [LLVMdev] Why should we have the LoopPass and LoopPassManager? Can we get rid of this complexity?
> 
> 
> 
> 
> 
> 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.

I had mentioned to Chandler previously that in the new pass manager I wanted the ability to register cleanup passes, "run this pass only if the previous pass actually changed something"... maybe this speaks for a slightly more general functionality, "run this pass only if the previous pass changed something, and here's a hint of a list of blocks or loops at which you should look." If we had something like this, we might be able to to handle cases like this without burdening the overall configuration with extra (unneeded) pass invocations.

 -Hal

> 
> 
> -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.
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory