[PATCH] D38641: [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition.

Mon Oct 16 10:27:19 PDT 2017

On Mon, Oct 16, 2017 at 10:23 AM, Jun Lim <junbuml at codeaurora.org> wrote:

>
>
>
>
> *From:* Xinliang David Li [mailto:davidxl at google.com]
> *Sent:* Monday, October 16, 2017 12:39 PM
> *To:* Jun Lim <junbuml at codeaurora.org>
> *Cc:* reviews+D38641+public+6e61ff737b781fa2 at reviews.llvm.org; Chandler
> Carruth <chandlerc at gmail.com>; Easwaran Raman <eraman at google.com>; Chad
> Rosier <mcrosier at codeaurora.org>; aemerson at apple.com; llvm-commits <
> llvm-commits at lists.llvm.org>; kristof.beyls at arm.com
> *Subject:* Re: [PATCH] D38641: [Inline][WIP] Expose more inlining
> opportunities by further constraining call site arguments based on
> splitting an OR condition.
>
>
>
>
>
>
>
> On Mon, Oct 16, 2017 at 9:32 AM, Jun Lim <junbuml at codeaurora.org> wrote:
>
>
>
> > I don't think there is a  need to hook up inline cost model for this. I
> consider this an pre-IPA enabler pass.
>
>
>
> Do you mean that we can clone a call site whenever we detect
> opportunities?
>
>
>
> > After inlining, if the mergeable callsites remain, the later cfg
> simplification pass should kick in and merge them.
>
>
>
>   If the call sites we split before inliner are not inlined, later passes
> may have an opportunity to merge them back. However, if we split an
> inlinable call site, then we might increase the code size by inlining two
> call sites.
>
>
>
>
>
> That is not a bad thing -- each inline instance will be more aggressively
> optimized due better contexts exposed. Such opportunity are usually very
> harder to materalize if inlinling is already done (without splitting),
> especially for functions with non-trivial bodies. To avoid code size
> increase, profile feedback should be used so that cold sites are skipped.
>
>
>
> David
>
>
>
>
>
> In order to avoid code size increase, do you think we should enable this
> only in PGO to skip cold sites?
>

 I don't see problem enabling it for O3 (with or without profile). For O2,
We need more data to decide if it should be on by default.

David

>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> *From:* Xinliang David Li [mailto:davidxl at google.com]
> *Sent:* Friday, October 13, 2017 6:18 PM
> *To:* Jun Lim <junbuml at codeaurora.org>
> *Cc:* reviews+D38641+public+6e61ff737b781fa2 at reviews.llvm.org; Chandler
> Carruth <chandlerc at gmail.com>; Easwaran Raman <eraman at google.com>; Chad
> Rosier <mcrosier at codeaurora.org>; aemerson at apple.com; llvm-commits <
> llvm-commits at lists.llvm.org>; kristof.beyls at arm.com
> *Subject:* Re: [PATCH] D38641: [Inline][WIP] Expose more inlining
> opportunities by further constraining call site arguments based on
> splitting an OR condition.
>
>
>
> I don't think there is a  need to hook up inline cost model for this. I
> consider this an pre-IPA enabler pass.   After inlining, if the mergeable
> callsites remain, the later cfg simplification pass should kick in and
> merge them.
>
>
>
> David
>
>
>
> On Fri, Oct 13, 2017 at 3:14 PM, Jun Lim <junbuml at codeaurora.org> wrote:
>
>
> Thanks David for the detail. I can also see many other opportunities for
> splitting/cloning call site not just for OR condition. We may be able to
> handle those step by step in a separate pass before inliner.  Don't you
> think we need to rely on the inline cost model to estimate the
> profitability before making decision for cloning a call site?
>
>
>
> -----Original Message-----
> From: David Li via Phabricator [mailto:reviews at reviews.llvm.org]
> Sent: Friday, October 13, 2017 5:05 PM
> To: junbuml at codeaurora.org; chandlerc at gmail.com; eraman at google.com;
> mcrosier at codeaurora.org
> Cc: davidxl at google.com; aemerson at apple.com; llvm-commits at lists.llvm.org;
> kristof.beyls at arm.com
> Subject: [PATCH] D38641: [Inline][WIP] Expose more inlining opportunities
> by further constraining call site arguments based on splitting an OR
> condition.
>
> davidxl added a comment.
>
> I see a lot of potential to make this more general. As I mentioned, this
> is similar to constant propagation based function cloning -- exposing
> specialization opportunities seems not limited to inliner though inlining
> could be the biggest customer.
>
> Consider this:
>
> define void @foo(i32) local_unnamed_addr #0 {
>
>   %2 = icmp eq i32 %0, 10
>   %3 = select i1 %2, i32 1, i32 2
>   tail call void @bar(i32 %3) #2
>   ret void
>
> }
>
> Converting Select into control flow and expose the constant propagation
> opportunity should be done in the same pass.
>
> Consider another example:
>
> define void @foo(i32) local_unnamed_addr #0 {
>
>   %2 = icmp eq i32 %0, 10
>   br i1 %2, label %3, label %4
>
> ; <label>:3:                                      ; preds = %1
>
>   tail call void @bar(i32 1) #2
>   br label %4
>
> ; <label>:4:                                      ; preds = %1, %3
>
>   %5 = phi i32 [ 1, %3 ], [ 2, %1 ]
>   tail call void @bar(i32 %5) #2
>   ret void
>
> }
>
> Hoisting 'bar' call into incoming block of the phi can also expose
> opportunity.
>
> Note that simplifyCFG pass in LLVM currently aggressively sink common code
> into the merge point -- which may lead to missing opportunities here.
> Chandler has a patch to undo that to reduce the damage done by the sinking
> but that pass is pretty late in the pipeline and won't help for
> inlining/cloning purpose.
>
>
> https://reviews.llvm.org/D38641
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171016/f042a496/attachment.html>