[LLVMdev] Inline hint for methods defined in-class
Eric Christopher
echristo at gmail.com
Wed Jun 24 15:30:40 PDT 2015
On Wed, Jun 24, 2015 at 3:25 PM David Blaikie <dblaikie at gmail.com> wrote:
> On Wed, Jun 24, 2015 at 3:04 PM, Easwaran Raman <eraman at google.com> wrote:
>
>> On Wed, Jun 24, 2015 at 2:35 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>> >
>> >
>> > On Wed, Jun 24, 2015 at 2:20 PM, Easwaran Raman <eraman at google.com>
>> wrote:
>> >>
>> >> 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.
>> >>
>> >> The reason I started looking into this is that, for a suite of
>> >> benchmarks we use internally, treating the in-class definitions
>> >> equivalent to having an 'inline' keyword, when combined with a higher
>> >> inlinehint-threshold, is a measurable win in performance. I am not
>> >> making any claim that this is a universal truth, but intuitively, the
>> >> description of 'inline' in C++ standard seems to influence what
>> >> methods are defined in-class.
>> >
>> >
>> > I'm not sure that's the case - in my experience (for my own code & the
>> code
>> > I see from others) people put stuff in headers that's "short enough"
>> that
>> > it's not worth the hassle of an external definition. I don't really
>> think
>> > authors are making an actual judgment about how much of a win inlining
>> their
>> > function is most of the time when they put a definition inline in a
>> class.
>> > (maybe a litttle more likely when it's a standalone function where you
>> have
>> > to write "inline" explicitly, but maybe not even then)
>> Ok, that may very well be the case.
>>
>> > It would seem that improving the inliner to do a better job of judging
>> the
>> > inlining benefit would be ideal (for this case and for LTO, etc - where
>> > we'll pick up equivalently small non-inline function definitions that
>> the
>> > author had decided to define out of line (either because they used to be
>> > longer or the author didn't find out of line definitions to be as
>> > inconveniently verbose as someone else, etc)), if there's something more
>> > useful to go on than "the user sort of maybe implied that this would be
>> good
>> > to inline". It seems like a very weak signal.
>>
>> I don't disagree with your ideal scenario. In the current non-ideal
>> state, LLVM does use a larger threshold for using the 'inline'
>> keyword. The question is whether using this larger threshold for
>> in-class definitions is a backward step.
>>
>
> Probably worth having this conversation on cfe-commits (as it's a Clang
> change and Clang developers are likely to have a better feel for how C++
> developers use inline definitions).
>
> Might want to rope in Chrome developers too - they are very sensitive to
> size increases.
>
>
Absolutely. In fact, Webkit was the project I was thinking of with "usually
out of date" :)
-eric
> & prototyping with the change to filter out templates would be relevant,
> of course.
>
> I don't see large-scale numbers (eg: across Google's perf benchmarks
> overall?) - spec is a bit narrow (& tends towards C code, if I'm not
> mistaken, so isn't likely to show much about this change), and that it
> improves the benchmark you were trying to improve would need to be weighed
> against the changes to a broader sample, I would think?
>
>
> - David
>
>
>>
>> - Easwaran
>>
>> > - David
>> >
>> >>
>> >>
>> >> - Easwaran
>> >>
>> >> > --paulr
>> >> >
>> >> >>
>> >> >> 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
>> >> >> >
>> >> >> >
>> >> _______________________________________________
>> >> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150624/22c19d3a/attachment.html>
More information about the llvm-dev
mailing list