<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 19, 2015 at 8:55 PM, Jiangning Liu <span dir="ltr"><<a href="mailto:liujiangning1@gmail.com" target="_blank">liujiangning1@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi David,<div class="gmail_extra"><br><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Regarding your patch, I think the simple loop heuristic may be a reasonable incremental improvement, but you really need to wait for Chandler's pass manager change instead of doing all the ordering tricks.</div></div></div></div></blockquote><div><br></div></span><div>Loop analysis is critical for my patch and it is expensive. The ordering tricks (3.b) is only for compile-time reduction. As I mentioned in replying Yin's comments, (3.b) itself can actually reduce compile-time a lot (~5% or more for llvm bootstrap) without changing performance generally. If I don't add (3.b), we would see ~10% compile-time regression. I don't think it is acceptable. So now the only difference is how we update loop info. i.e. use my current naive solution, or use new pass manager to help it?</div><div><br></div><div>So do you know when Chandler's pass manager change can be ready for inlining use?</div></div></div></div></blockquote><div><br></div><div>It will be done in a very near future, but Chandler can give you more precise info. </div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> Why we can't just use this native method first, and I can add //FIXME to highlight it, and then when Chandler's great new pass manager is ready for inlining use, we just switch to it? Eventually we would be able to see big compile-time reduction.</div></div></div></div></blockquote><div><br></div><div><br></div><div>You need to discuss with Chandler or other inliner maintainers.  It is probably possible but not in the current form of patch. IMO, the patch need to be in the current form (because of the hack to workaround limitations).</div><div><br></div><div>1) guarded under a master flag (so that others can easily experiment with it)</div><div>2) assumes the infrastructure is ready and just make very simple changes to boost inlining with bonuses (similar to existing heuristics such as vector bonus)</div><div>3) isolate your mock implementation of the loop info access in a centralized place</div><div>4) get rid of the ordering change hack -- basically tolerate the compile time increase assuming it will go away (and because it is under a flag).</div><div><br></div><div>This is just my personal opinion. Better discuss this with Chandler.</div><div><br></div><div>thanks,</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>On the other hand, I don't see the 'same callee' heuristics is generally valid.  How much size contribution does it have? Is it just skewed by one benchmark?<br></div></div></div></div></blockquote><div><br></div></span><div>OK. I agree with this. I can get it removed, and actually the code size impact for SPEC overall is about ~2% for this rule. I just personally think it's unreasonable to inline a callee being called a lot of times naively. Actually this is just real case for one of SPEC2000 benchmarks, but generally I think it is meaningful. For abnormal request like building Android for one of the huge functions, if you want inline everything, then that's fine, we can just using command line option to enable it. But generally for common programming, I still think it's wired to inline this case.</div><div><br></div><div>Thanks,</div><div>-Jiangning</div></div></div></div>
</blockquote></div><br></div></div>