[PATCH] D13304: Avoid inlining in throw statement

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 8 09:54:45 PDT 2015


> 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;

We need to differentiate between these two. For the second case, we should
not add any attribute because itÂ’s not invoked in the EH region and it
could have any other purposes other than exception handling. I believe we
need to specifically handle only the case where we can guarantee that the
call is invoked only in exception handling context.


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

I agree. If we need to add check for callee size, doing this in frontend
may not be the good idea. But it should be done before inliner.


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

Basically, current Inliner doesn't have any impact with Attribute::Cold
because ColdThreshold and InlineLimit are the same by default. I
understand noinline looks somewhat strong decision. But, even with
noinline, I believe we could expect more pros than cons.

In performance perspective, by avoiding inlining in throw statement, we
could open up more inline opportunities for functions containing throw
statements. For example, below small function could not be inlined in its
many callsites if the constructor of IndexOutOfBoundsException is
non-trivial and inlined first in elementAt().

  int elementAt(int idx) {
    if (idx >= limit)
      throw IndexOutOfBoundsException(idx, limit);
    return Array[idx];
  }
Pretty much same thing could actually happen in many c++ programs, and I
found the actual cases in spec2006/xalancbmk.

In most case we also don't need to increase code size by inlining
constructors invoked in throw statement, especially when there is a
hierarchy of exception handling classes.

The only downsides I can think of is the case where the constructor is
very small so that inlining it is profitable for size. My suggestion for
this is to move this implementation back to PruneEH.cpp so that we can
check the callee size right before the inliner. I may add the noinline
only when the constructor is large enough.




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



More information about the cfe-commits mailing list