<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 7, 2015 at 3:10 PM, Jun Bum Lim via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">junbuml added a comment.<br>
<br>
Thanks Richard for the comment.<br>
<br>
Initially, I intended to implement this in inliner by checking if a callsite is in exception handling regions. However, I decided not to implement this in inliner because this kind of check should be performed for all callsites if we implement it in inliner.<br>
<br>
Instead of directly adding complexity in inliner, I implemented this in PruneEH.cpp (<a href="http://reviews.llvm.org/D12979" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12979</a>) because this is very specific to exception handling regions. In this patch, I tried to mark all callsites invoked from throw statements as cold (noinline) by looking up users of __cxa_throw() and __cxa_allocate_exception(). We had many discussions and finally ended up to implement the same thing in clang to be more general and simpler as Hal suggested in <a href="http://reviews.llvm.org/D12979" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12979</a>.<br></blockquote><div><br></div><div>I think this actually makes it less general. You would presumably perform different inlining for:</div><div><br></div><div>  throw f(x, y);</div><div><br></div><div>versus</div><div><br></div><div>  auto k = f(x, y);</div><div>  throw k;</div><div><br></div><div>which doesn't really seem defensible.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
As you point out, it should be done by influencing inline cost heurisic, so I believe Attribute::Cold is the right attribute to be added here. However, as I FIXMEed, the current ColdThreshold is not tuned yet (r200898). So for now, I add both cold and noinline.<br>
<br>
Regarding code size, I believe not inlining contractor calls in throw statements could be potentially more helpful for code size in many cases. If inlining very small callsites in throw statements could be issue, then we may be able to  check if callee is smaller than some threshold to avoid adding the attributes (cold and noinline).</blockquote><div><br></div><div>That's not something we can really do from the frontend, because small constructors often don't look small until they themselves have been optimized.</div><div><br></div><div>Would it be sufficient to mark just the invocation of __cxa_throw (or its equivalent) as cold? If the optimizer can't infer from that that the code leading up to the call is also cold, we should fix that in the LLVM-side analysis rather than working around it in the frontend.</div></div></div></div>