<div dir="ltr">Hi David,<div><div class="gmail_extra"><br></div><div class="gmail_extra">Thanks a lot for your review!</div><div class="gmail_extra"><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>1) guarded under a master flag (so that others can easily experiment with it)<br></div></div></div></div></blockquote><div><br></div><div>Agree. I already defined a master flag called AgressiveInline. Given that this was not caught by you, I added comment to get it highlighted in file InlineSimple.cpp</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></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></div></div></blockquote><div><br></div><div>I think, at present, we don't have incremental loop info update interface, so it's no way to assume it unless we define it at the moment. If I'm wrong about this, please let me know.</div><div><br></div><div>The heuristics for vector bonus is for callee only, and I don't think it should be mixed with the call site context.</div><div><br></div><div>However, I agree to centralize all implementation of LoopAnalyzer, so it would be easy to get it enhanced with incremental loop info update.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>3) isolate your mock implementation of the loop info access in a centralized place</div></div></div></div></blockquote><div><br></div><div>Agree. Actually my code already got all heuristics isolated in function TuneThreshold in InlineSimple.cpp, and this is the only place to change threshold dynamically according to control flow context.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></div></blockquote><div><br></div><div>Disagree. This change itself is meaningful regardless if we use new pass manager or not. This change will definitely reduce compile-time. And loop info calculation just magnified the slow-down. I don't think it's a hacking at all. On the other hand, I don't think it is acceptable to ask others to endure the compiler slowdown. It would be quite annoying from engineering point of view.</div><div><br></div><div>However, I agree to get it split out to be a separate one. So now I have two patches as attached. One is to fix compile time issue, and the other is to fix performance.</div><div><br></div><div>Thanks,<br></div><div>-Jiangning</div><div> </div></div></div></div></div>