<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 16, 2017 at 10:23 AM, Jun Lim <span dir="ltr"><<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-US"><div class="gmail-m_4665564861155403388WordSection1"><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><span class="gmail-"><b>From:</b> Xinliang David Li [mailto:<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>] <br></span><b>Sent:</b> Monday, October 16, 2017 12:39 PM<span class="gmail-"><br><b>To:</b> Jun Lim <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>><br><b>Cc:</b> <a href="mailto:reviews%2BD38641%2Bpublic%2B6e61ff737b781fa2@reviews.llvm.org" target="_blank">reviews+D38641+public+<wbr>6e61ff737b781fa2@reviews.llvm.<wbr>org</a>; Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>>; Easwaran Raman <<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>>; Chad Rosier <<a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a>>; <a href="mailto:aemerson@apple.com" target="_blank">aemerson@apple.com</a>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>; <a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a><br><b>Subject:</b> Re: [PATCH] D38641: [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition.<u></u><u></u></span></p><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal"><u></u> <u></u></p><div><p class="MsoNormal">On Mon, Oct 16, 2017 at 9:32 AM, Jun Lim <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>> wrote:<u></u><u></u></p><span class="gmail-"><blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in"><div><div><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">> I don't think there is a  need to hook up inline cost model for this. I consider this an pre-IPA enabler pass. <u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">Do you mean that we can clone a call site whenever we detect opportunities? <u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">> After inlining, if the mergeable callsites remain, the later cfg simplification pass should kick in and merge them.<u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal">  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.<u></u><u></u></p></div></div></blockquote><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">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.<u></u><u></u></p></div><div><p class="MsoNormal"><u></u> <u></u></p></div><div><p class="MsoNormal">David<u></u><u></u></p></div></span><div><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">In order to avoid code size increase, do you think we should enable this only in PGO to skip cold sites?</p></div></div></div></div></div></div></blockquote><div><br></div><div><br></div><div> 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.</div><div><br></div><div>David</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div lang="EN-US"><div class="gmail-m_4665564861155403388WordSection1"><div><div><div><div><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><u></u> <u></u></p></div><div><div class="gmail-h5"><blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in"><div><div><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"> <u></u><u></u></p><p class="MsoNormal"><b>From:</b> Xinliang David Li [mailto:<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>] <br><b>Sent:</b> Friday, October 13, 2017 6:18 PM<br><b>To:</b> Jun Lim <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>><br><b>Cc:</b> <a href="mailto:reviews%2BD38641%2Bpublic%2B6e61ff737b781fa2@reviews.llvm.org" target="_blank">reviews+D38641+public+<wbr>6e61ff737b781fa2@reviews.llvm.<wbr>org</a>; Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>>; Easwaran Raman <<a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>>; Chad Rosier <<a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a>>; <a href="mailto:aemerson@apple.com" target="_blank">aemerson@apple.com</a>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>>; <a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a><br><b>Subject:</b> Re: [PATCH] D38641: [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition.<u></u><u></u></p><div><div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">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.<u></u><u></u></p><div><p class="MsoNormal"> <u></u><u></u></p></div><div><p class="MsoNormal">David<u></u><u></u></p></div></div><div><p class="MsoNormal"> <u></u><u></u></p><div><p class="MsoNormal">On Fri, Oct 13, 2017 at 3:14 PM, Jun Lim <<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>> wrote:<u></u><u></u></p><blockquote style="border-top:none;border-right:none;border-bottom:none;border-left:1pt solid rgb(204,204,204);padding:0in 0in 0in 6pt;margin:5pt 0in 5pt 4.8pt"><p class="MsoNormal"><br>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?<u></u><u></u></p><div><div><p class="MsoNormal" style="margin-bottom:12pt"><br><br>-----Original Message-----<br>From: David Li via Phabricator [mailto:<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.<wbr>org</a>]<br>Sent: Friday, October 13, 2017 5:05 PM<br>To: <a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>; <a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>; <a href="mailto:eraman@google.com" target="_blank">eraman@google.com</a>; <a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a><br>Cc: <a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>; <a href="mailto:aemerson@apple.com" target="_blank">aemerson@apple.com</a>; <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>; <a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a><br>Subject: [PATCH] D38641: [Inline][WIP] Expose more inlining opportunities by further constraining call site arguments based on splitting an OR condition.<br><br>davidxl added a comment.<br><br>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.<br><br>Consider this:<br><br>define void @foo(i32) local_unnamed_addr #0 {<br><br>  %2 = icmp eq i32 %0, 10<br>  %3 = select i1 %2, i32 1, i32 2<br>  tail call void @bar(i32 %3) #2<br>  ret void<br><br>}<br><br>Converting Select into control flow and expose the constant propagation opportunity should be done in the same pass.<br><br>Consider another example:<br><br>define void @foo(i32) local_unnamed_addr #0 {<br><br>  %2 = icmp eq i32 %0, 10<br>  br i1 %2, label %3, label %4<br><br>; <label>:3:                                      ; preds = %1<br><br>  tail call void @bar(i32 1) #2<br>  br label %4<br><br>; <label>:4:                                      ; preds = %1, %3<br><br>  %5 = phi i32 [ 1, %3 ], [ 2, %1 ]<br>  tail call void @bar(i32 %5) #2<br>  ret void<br><br>}<br><br>Hoisting 'bar' call into incoming block of the phi can also expose opportunity.<br><br>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.<br><br><br><a href="https://reviews.llvm.org/D38641" target="_blank">https://reviews.llvm.org/<wbr>D38641</a><br><br><br><u></u><u></u></p></div></div></blockquote></div><p class="MsoNormal"> <u></u><u></u></p></div></div></div></div></div></blockquote></div></div></div><p class="MsoNormal"><u></u> <u></u></p></div></div></div></div></blockquote></div><br></div></div>