[PATCH] D13304: Avoid inlining in throw statement

[Richard Smith via cfe-commits]

On Wed, Oct 7, 2015 at 3:10 PM, Jun Bum Lim via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> junbuml added a comment.
>
> Thanks Richard for the comment.
>
> 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.
>
> Instead of directly adding complexity in inliner, I implemented this in
> PruneEH.cpp (http://reviews.llvm.org/D12979) 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
> http://reviews.llvm.org/D12979.
>

I think this actually makes it less general. You would presumably perform
different inlining for:

  throw f(x, y);

versus

  auto k = f(x, y);
  throw k;

which doesn't really seem defensible.


> 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.
>
> 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).


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.

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151007/06635b9c/attachment.html>