[llvm] r206429 - Inline a function when the always_inline attribute

[Gerolf Hoflehner]

I think this is the summary of all the good discussions about always_inline: 
- stick with a semantics like inline only when the compiler can determine that it is safe to do so (rather than have the user be responsible for it)
- issue a warning/error message when the compiler does not inline
- improve the scope of the analysis to allow more inlining

So for now I reverted the patch.


-Gerolf
On Apr 16, 2014, at 11:23 PM, Chandler Carruth <chandlerc at google.com> wrote:

> 
> On Wed, Apr 16, 2014 at 10:54 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > False. Do not submit.
> >
> > If you want to inline functions that contain indirect branches, you need to teach the inliner how to do it correctly.
> 
> I agree with Eric and Nick.  It seems like a dangerous semantic change
> for the attribute, making previously correct code incorrect.
> 
> GCC docs for always_inline [1] (clang mentions it [2] twice [3] but
> doesn't seem to define it) and LLVM LangRef.rst for alwaysinline [4]
> indicate that the inlining thresholds should be ignored, but don't say
> anything about the attribute trumping correctness.
> 
> [1]: http://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/Function-Attributes.html
> [2]: http://clang.llvm.org/docs/LanguageExtensions.html
> [3]: http://clang.llvm.org/docs/AttributeReference.html
> [4]: http://llvm.org/docs/LangRef.html#function-attributes
> 
> As the last person to deal with this, I can confidently say that it does *not* trump correctness. I hit many, many cases where it was absolutely critical that it apply in all cases except those which it cannot apply under.
> 
> At most, we should error in the frontend whenever we can trivially show that always_inline is incompatible with the function, but that is totally different from inlining blindly.
>  
> 
> 
> > This has the added bonus that we'll then be able to inline regular functions with indirect branches too.
> 
> I'm not convinced this is a good idea anyway, unless our optimizations
> are tuned for handling irreducible control flow.  It seems likely that
> keeping that logic sequestered is profitable.
> 
> I think this is a good idea for one simple reason: inlining is one of the only things that has a decent chance at constant folding away the indirectbr or DCE-ing it.
> 
> I don't think we need to worry much about getting the thresholds right here. The way the current inliner works it will never do this except for tiny indirectbr usages, or cases where some or all of the indirectbr reached blocks are DCE-ed.
> 
> -Chandler

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140417/8d49bdfc/attachment.html>