[PATCH] D15449: [PassManagerBuilder] Add a few more scalar optimization passes

[Mehdi Amini via llvm-commits]

> On Dec 11, 2015, at 8:19 AM, James Molloy <james at jamesmolloy.co.uk> wrote:
> 
> Hi,
> 
> > - I'd rather see this as two patches: one for the GlobalOpt and the other for the scalar optimizations
> 
> Sure, that's easily done. Would you prefer me to open another phab review or are happy with it being committed split apart?

It is more about the commit. So that the performance can be assessed separately and any issue would be better bisected.

> 
> > - Do you have benchmark results before/after?
> 
> Yes and no. The mem2reg changes do affect benchmarks I care about, but they're not in test-suite and I'm not allowed to quote numbers from them.
> I don't have an LTO setup of the test-suite to get numbers for the LTO portions either (although I do have LTO set up for third party test suites that I can't quote numbers from!). I haven't seen any regressions in any test, and some improve drastically. Sorry for the weasel words.

Can you at least give an overview (without naming), like “on some internal benchmarks it improves XX% on average, with XX test cases that regressed around ~XX%” ?


> 
> As a general principle, I think the LTO driver isn't currently doing enough scalar optimization. I've seen several cases where really poor code gets through to late passes like CGP purely because SimplifyCFG/InstCombine weren't run enough.


Clearly, the problem is the tradeoff with the compile time.

> 
> > - See also: http://reviews.llvm.org/D13443 <http://reviews.llvm.org/D13443> ; I paused my work on this till January because of the ThinLTO bringup, but I still plan to move forward with it.
> 
> This looks good. It looks like a real reegineering of the pipeline, which is a bit more work than I was hoping to chew off - I hope that my work might go some way towards improving the LTO codegen without requiring thousands of benchmarking hours to check it's OK!


Indeed I spent some hundred of hours of benchmarking in September. I’d be happy if you could test D13443 on your hardware/bench by the way :)


> 
> (Aside, in D13443 you don't run GlobalOpt/Mem2Reg early. I think functionattrs+globalopt+mem2reg needs to run as early as possible so that demoted globals become first class SSA values for the whole of the pass pipeline).

Note that global opt needs *also* to run after the inliner because it can do more work. But again compile time...

— 
Mehdi




> 
> James
> 
> On Fri, 11 Dec 2015 at 16:08 Mehdi AMINI via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> joker.eph added a comment.
> 
> Hi James,
> 
> A few points:
> 
> - I'd rather see this as two patches: one for the GlobalOpt and the other for the scalar optimizations
> - Do you have benchmark results before/after?
> - See also: http://reviews.llvm.org/D13443 <http://reviews.llvm.org/D13443> ; I paused my work on this till January because of the ThinLTO bringup, but I still plan to move forward with it.
> 
> Thanks!
> 
> 
> Repository:
>   rL LLVM
> 
> http://reviews.llvm.org/D15449 <http://reviews.llvm.org/D15449>
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151211/4ca3dcf7/attachment.html>