[LLVMdev] Inline hint for methods defined in-class

[Xinliang David Li]

On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul
<Paul_Robinson at playstation.sony.com> wrote:
>> -----Original Message-----
>> From: Easwaran Raman [mailto:eraman at google.com]
>> Sent: Wednesday, June 24, 2015 1:27 PM
>> To: Xinliang David Li
>> Cc: Robinson, Paul; Xinliang David Li; <llvmdev at cs.uiuc.edu> List
>> Subject: Re: [LLVMdev] Inline hint for methods defined in-class
>>
>> The method to identify functions with in-class definitions is one part
>> of my question. Even if there is a way to do that without passing the
>> hint, I'm interested in getting feedback on treating it at-par with
>> functions having the inline hint in inline cost analysis.
>
> Well, personally I think having the 'inline' keyword mean "try harder"
> is worth something, but that's intuition backed by no data whatsoever.
> Your patch would turn 'inline' into noise, when applied to a function
> with an in-class definition.  Granted that the way the C++ standard
> describes 'inline' it is effectively noise in that situation.
> --paulr

You are assuming most of the functions are defined in-class, which I
think is not true.

David

>
>>
>> Thanks,
>> Easwaran
>>
>>
>> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li
>> <xinliangli at gmail.com> wrote:
>> > The problem is that the other way around is not true: a function
>> > linkonce_odr linkage may be neither inline declared nor have in-class
>> > definition.
>> >
>> > David
>> >
>> >
>> > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul
>> > <Paul_Robinson at playstation.sony.com> wrote:
>> >>
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-
>> bounces at cs.uiuc.edu]
>> >> > On
>> >> > Behalf Of Easwaran Raman
>> >> > Sent: Wednesday, June 24, 2015 9:54 AM
>> >> > To: Xinliang David Li
>> >> > Cc: <llvmdev at cs.uiuc.edu> List
>> >> > Subject: Re: [LLVMdev] Inline hint for methods defined in-class
>> >> >
>> >> > Ping.
>> >> >
>> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li
>> <davidxl at google.com>
>> >> > wrote:
>> >> > > that looks like a different fix. The case mentioned by Easwaran is
>> >> > >
>> >> > > class A{
>> >> > >    int foo () { return 1; }
>> >> > >   ...
>> >> > > };
>> >> > >
>> >> > > where 'foo' is not explicitly declared with 'inline' keyword.
>> >> > >
>> >> > > David
>> >> > >
>> >> > > On Wed, Jun 17, 2015 at 4:07 PM, Balaram Makam
>> <bmakam at codeaurora.org>
>> >> > wrote:
>> >> > >> AFAIK, this was fixed in r233817.
>> >>
>> >> That was later reverted.
>> >>
>> >> > >>
>> >> > >> -----Original Message-----
>> >> > >> From: llvmdev-bounces at cs.uiuc.edu
>> >> > >> [mailto:llvmdev-bounces at cs.uiuc.edu]
>> >> > On
>> >> > >> Behalf Of Easwaran Raman
>> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM
>> >> > >> To: llvmdev at cs.uiuc.edu
>> >> > >> Cc: David Li
>> >> > >> Subject: [LLVMdev] Inline hint for methods defined in-class
>> >> > >>
>> >> > >> Clang adds the InlineHint attribute to functions that are
>> explicitly
>> >> > marked
>> >> > >> inline, but not if they are defined in the class body. I tried the
>> >> > following
>> >> > >> patch, which I believe handles the in-class definition
>> >> > >> case:
>> >> > >>
>> >> > >> --- a/lib/CodeGen/CodeGenFunction.cpp
>> >> > >> +++ b/lib/CodeGen/CodeGenFunction.cpp
>> >> > >> @@ -630,7 +630,7 @@ void CodeGenFunction::StartFunction(GlobalDecl
>> >> > >> GD,
>> >> > >>    if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
>> {
>> >> > >>      if (!CGM.getCodeGenOpts().NoInline) {
>> >> > >>        for (auto RI : FD->redecls())
>> >> > >> -        if (RI->isInlineSpecified()) {
>> >> > >> +        if (RI->isInlined()) {
>> >> > >>            Fn->addFnAttr(llvm::Attribute::InlineHint);
>> >> > >>            break;
>> >> > >>          }
>> >> > >>
>> >> > >> I tried this on C++ benchmarks in SPEC 2006. There is no
>> noticeable
>> >> > >> performance difference and the maximum text size increase is <
>> 0.25%.
>> >> > >> I then built clang with and without this change. This increases
>> the
>> >> > text
>> >> > >> size by 4.1%.  For measuring performance, I compiled a large (4.8
>> >> > million
>> >> > >> lines) preprocessed file. This change improves runtime performance
>> by
>> >> > 0.9%
>> >> > >> (average of 10 runs) in O0 and O2.
>> >> > >>
>> >> > >> I think knowing whether a function is defined inside a class body
>> is
>> >> > >> a
>> >> > >> useful hint to the inliner. FWIW, GCC's inliner doesn't
>> differentiate
>> >> > these
>> >> > >> from explicit inline functions. If the above results doesn't
>> justify
>> >> > this
>> >> > >> change, are there other benchmarks that I should evaluate? Another
>> >> > >> possibility is to add a separate hint for this instead of using
>> the
>> >> > existing
>> >> > >> inlinehint to allow for better tuning in the inliner.
>> >>
>> >> A function with an in-class definition will have linkonce_odr linkage,
>> >> so it should be possible to identify such functions in the inliner
>> >> without introducing the inlinehint attribute.
>> >> --paulr
>> >>
>> >> > >>
>> >> > >> Thanks,
>> >> > >> Easwaran
>> >> > >> _______________________________________________
>> >> > >> LLVM Developers mailing list
>> >> > >> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>> >> > >>
>> >> > _______________________________________________
>> >> > LLVM Developers mailing list
>> >> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>> >>
>> >> _______________________________________________
>> >> LLVM Developers mailing list
>> >> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>> >
>> >