[PATCH] D13304: Avoid inlining in throw statement

[Richard Smith via cfe-commits]

rsmith added a comment.

I am not convinced that it's reasonable to put inlining heuristics into clang's IR generation. This will cause maintenance problems for the inliner in the future (anyone tuning the inlining heuristics and thresholds will need to be aware of this, and clang will behave differently from other frontends in this regard).

I don't really see a need for clang's IR to change in order to support the inliner in this regard -- the inliner can already determine that a call to a `noreturn` function can only exit via unwinding, and can already determine which code is only reachable via landingpads. That seems like enough information for all the cases you're addressing here. (If I remember correctly, we already have a heuristic that treats calls followed by `unreachable` as being cold; perhaps we could extend that to also look for invokes that branch to `unreachable`, if it doesn't already handle that case.) If we handle this at that level, we'll also handle cases such as a user-defined function whose purpose is to throw an exception, and we'll treat the code leading to the throw the same regardless of whether it's within the operand of the //throw-expression//.

`noinline` seems like too strong of a constraint for this case. There are calls that we really want to inline, even if they occur in cold regions (because doing so will reduce code size), and calls that we must inline for correctness (calls to `always_inline` functions). If the `cold` attribute isn't working properly, that's an inliner bug and should be fixed in the inliner, not worked around here.

If you want to change the inlining heuristics to make a different choice here, it seems to me that you should make that change in the inliner.


http://reviews.llvm.org/D13304