[PATCH] D26037: Add LoopSink pass for PGO.

[Hal Finkel via llvm-commits]

----- Original Message -----

> From: "Xinliang David Li" <davidxl at google.com>
> To: reviews+D26037+public+89d2f1349c56fe36 at reviews.llvm.org
> Cc: "陈德颢" <danielcdh at gmail.com>, "Hal Finkel" <hfinkel at anl.gov>,
> "Junbum Lim" <junbuml at codeaurora.org>, "llvm-commits"
> <llvm-commits at lists.llvm.org>, anna at azul.com
> Sent: Friday, October 28, 2016 12:11:04 PM
> Subject: Re: [PATCH] D26037: Add LoopSink pass for PGO.

> On Fri, Oct 28, 2016 at 9:40 AM, Dehao Chen < danielcdh at gmail.com >
> wrote:

> > danielcdh added a comment.
> 

> > We need this pass to remove LCSSA nodes because this will be the
> > last
> > loop pass.
> 

> > Another question is: shall we pin this pass to PGO only? If not, we
> > can simply add it to lib/Transforms/IPO/PassManagerBuilder.cpp
> 

> Static branch prediction is conservative in predicting loop
> iterations (loop scale). This means it is likely some blocks (if
> wrapped under __builtin_expect) inside the loop will be wrongly
> marked as colder than preheader which can trigger loop sinking
> wrongly.
Can you please elaborate? If we statically predict a modest number of iterations, which seems like a reasonable trade off, and we have a block which is expected to not execute (because of __builtin_expect), then is it obviously wrong that it should be colder than the preheader? If the opposite of __builtin_expect is "almost never", which is true for error paths and the like, then that seems not unreasonable. Why would loop sinking hurt there? 

Thanks again, 
Hal 

> Making this pass depending on PGO is a safer thing to do.

> David

> > https://reviews.llvm.org/D26037
> 

-- 

Hal Finkel 
Lead, Compiler Technology and Programming Languages 
Leadership Computing Facility 
Argonne National Laboratory 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161028/ed3e10b7/attachment.html>