<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 20, 2017 at 1:48 PM, Easwaran Raman via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">eraman added a comment.<br>
<span class=""><br>
<br>
<br>
> Yes, i was thinking adding a limit on the chain length as an option. Probably do this a a follow up -- which also needs to look at profile data.  (Note that Partialinlininer is currently not enabled in any optimization pipeline yet).<br>
<br>
</span>I think instruction count will be better limit than chain length.<br></blockquote><div><br></div><div>yep.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<br>
================<br>
Comment at: lib/Transforms/IPO/<wbr>PartialInlining.cpp:193<br>
+    }<br>
+  } while (true);<br>
+<br>
----------------<br>
This will look cleaner if this is made to while (CommonSucc) and peel the else part outside of the while loop (since you always break from there). Perhaps you can even peel the first iteration out of the loop: first find the candidate non-return block, then walk the chain in a loop as long as one successor equals non-return block and the other succesor has a single predecessor. Some more comments will also increase readability.<br>
<br></blockquote><div><br></div><div>This part will be rewritten to handle more general cases.</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">
<br>
<a href="https://reviews.llvm.org/D32249" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32249</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>