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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to