thr3ads.net - llvm dev - [LLVMdev] Inline hint for methods defined in-class [Jul 2015]

If this information is useful, please help other people find it:
Share via:

Easwaran Raman

2015-Jul-07 23:07 UTC

[LLVMdev] Inline hint for methods defined in-class

I'm reviving this thread after a while and CCing cfe-commits as
suggested by David Blaikie. I've also collected numbers building
chrome (from chromium, on Linux) with and without this patch as
suggested by David. I've re-posted the proposed patch and
performance/size numbers collected at the top to make it easily
readable for those reading it through cfe-commits.

The proposed patch will add InlineHint to methods defined inside a class:

--- 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;
         }

Here are the performance and size numbers I've collected:


- C++ subset of Spec: No performance effects, < 0.1% size increase
(all size numbers are text sizes returned by 'size')
- Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
file) , 4.1% size increase
- Chrome: no performance improvement, 0.24% size increase
- Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
performance improvement, no size regression

If there is any other important benchmark/application that needs to be
evaluated, I'll work on that.

The main skepticism in this thread is about whether a developer
intends/expects a method defined in-class to be inlined or purely uses
size of the method body to make this decision. I'll let CFE developers
chime in on this. But irrespective of the intention, I think the data
suggests this is a useful signal in some good cases and has a small
size penalty in some bad cases. Note that if the criterion for placing
it in-class is purely based on size, and assuming the inline-threshold
is chosen to inline "small" functions, this change should only affect
a small number of functions (in the inline-threshold to
inlinehint-threshold range) and the risk of serious size bloat is low.

- Easwaran

On Wed, Jun 24, 2015 at 3:15 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.
>
> & 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
>> >
>> >
>
>

Chandler Carruth

2015-Jul-08 01:06 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On Tue, Jul 7, 2015 at 4:11 PM Easwaran Raman <eraman at google.com>
wrote:
> I'm reviving this thread after a while and CCing cfe-commits as
> suggested by David Blaikie. I've also collected numbers building
> chrome (from chromium, on Linux) with and without this patch as
> suggested by David. I've re-posted the proposed patch and
> performance/size numbers collected at the top to make it easily
> readable for those reading it through cfe-commits.
>
First off, thanks for collecting the numbers and broadening the
distribution. Also, sorry it took me so long to get over to this thread.

I want to lay out my stance on this issue at a theoretical and practical
level first. I'll follow up with thoughts on the numbers as well after that.

I think that tying *any* optimizer behavior to the 'inline' keyword is
fundamentally the wrong direction. We have reasons why we have done this
historically, and we can't just do an immediate about face, but we should
be actively looking for ways to *reduce* the optimizer's reliance on this
keyword to convey any meaning whatsoever.

The reason I think that is the correct direction is because, for better or
worse, the 'inline' keyword in C++ is not about optimization, but about
linkage. It has a functional impact and can be both necessary or impossible
to use to meet those functional requirements. This in turn leaves
programmers in a lurch if the functional requirements are ever in tension
with the optimizer requirements.

We're also working really hard to get more widely deployed cross-module
optimization strategies, in part to free programmers from the requirement
that they put all their performance critical code in header files. That
makes compilation faster, and has lots of benefits to the factoring and
design of the code itself. We shouldn't then create an incentive to keep
things in header files so that they pick up a hint to the optimizer.

Ultimately, the world will be a better place if we can eventually move code
away from relying on the hint provided by the 'inline' keyword to the
optimizer.

That doesn't mean that the core concept of hinting to the optimizer that a
particular function is a particularly good candidate for inlining is
without value. While I think it is a bad practice that we shouldn't
encourage in code (especially portable code) I can see the desire to at
least have *some* attribute which is nothing more or less than a hint to
the optimizer to inline harder[1]. It would help people work around inliner
bugs in the short term, and even help debug inliner-rooted optimization
problems. Codebases with strong portability requirements could still (and
probably should) forbid or tightly control access to this kind of hint. I
would want really strong documentation about how this attribute *completely
voids* your performance warranty (if such a thing exists) as from version
to version of the compiler it may go from a helpful hint to a devastatingly
bad hint. But I think I could be persuaded to live with such a hint
existing. But I'm *really* uncomfortable with it being tied to something
that also impacts linkage or other semantics of the program.

[1]: Currently, the only other hint we have available is pretty terrible as
it *also* has semantic effects: the always_inline attribute.

> The proposed patch will add InlineHint to methods defined inside a class:
>
> --- 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;
>          }
>
> Here are the performance and size numbers I've collected:
>
>
> - C++ subset of Spec: No performance effects, < 0.1% size increase
> (all size numbers are text sizes returned by 'size')
> - Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
> file) , 4.1% size increase
>
FWIW, this size increase seems *really* bad. I think that kills this
approach even in the short term.

> - Chrome: no performance improvement, 0.24% size increase
> - Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
> performance improvement, no size regression
>
I'm also somewhat worried about the lack of any performance improvements
outside of the Google benchmarks. That somewhat strongly suggests that our
benchmarks are overly coupled to this hint already. The fact that neither
Chrome, Clang, nor SPEC improved is... not at all encouraging.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150708/240f2758/attachment.html>

Xinliang David Li

2015-Jul-08 05:25 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On Tue, Jul 7, 2015 at 6:06 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:> On Tue, Jul 7, 2015 at 4:11 PM Easwaran Raman <eraman at google.com>
wrote:
>>
>> I'm reviving this thread after a while and CCing cfe-commits as
>> suggested by David Blaikie. I've also collected numbers building
>> chrome (from chromium, on Linux) with and without this patch as
>> suggested by David. I've re-posted the proposed patch and
>> performance/size numbers collected at the top to make it easily
>> readable for those reading it through cfe-commits.
>
>
> First off, thanks for collecting the numbers and broadening the
> distribution. Also, sorry it took me so long to get over to this thread.
>
> I want to lay out my stance on this issue at a theoretical and practical
> level first. I'll follow up with thoughts on the numbers as well after
that.
>
> I think that tying *any* optimizer behavior to the 'inline' keyword
is
> fundamentally the wrong direction.
Chandler, thanks for sharing your thought -- however I don't think it
is wrong, let alone 'fundamentally wrong'.  Despite all the analysis
that can be done, the inliner is in the end heuristic based. In lack
of the profile data, when inlining two calls yield the same static
benefit and size cost, it is reasonable for the inliner to think the
call to the function with inline hint to yield more high
dynamic/runtime benefit -- thus it has a higher static size budget to
burn.
>We have reasons why we have done this
> historically, and we can't just do an immediate about face, but we
should be
> actively looking for ways to *reduce* the optimizer's reliance on this
> keyword to convey any meaning whatsoever.
yes those additional things will be done, but they are really orthogonal.
>
> The reason I think that is the correct direction is because, for better or
> worse, the 'inline' keyword in C++ is not about optimization, but
about
> linkage.
It is about both optimization and linkage. In fact the linkage simply
serves as an implementation detail. In C++ standard 7.1.2,  paragraph
2 says:

"A function declaration (8.3.5, 9.3, 11.3) with an inline specifier
declares an inline function. The inline specifier indicates to the
implementation that inline substitution of the function body at the
point of call is to be preferred to the usual function call mechanism.
An implementation is not required to perform this inline substitution
at the point of call; however, even if this inline substitution is
omitted, the other rules for inline functions defined by 7.1.2 shall
still be respected."

Developers see those and rely on those to give compiler the hints.

Most importantly, paragraph 3 says:

"A function defined within a class definition is an inline function.
The inline specifier shall not appear on a block scope function
declaration.93 If the inline specifier is used in a friend
declaration, that declaration shall be a definition or the function
shall have previously been declared inline."

Here we can see regardless of how optimizer will honor the hint and to
what extent, and based on what analysis,
it is basically incorrect to drop the attribute on the floor for
in-class function definitions. Eswaran's fix is justified with this
reason alone.  The side effect of changing inliner behavior is
irrelevant.
> It has a functional impact and can be both necessary or impossible
> to use to meet those functional requirements. This in turn leaves
> programmers in a lurch if the functional requirements are ever in tension
> with the optimizer requirements.
Not sure what you mean. Performance conscious programmers use it all the time.
>
> We're also working really hard to get more widely deployed cross-module
> optimization strategies, in part to free programmers from the requirement
> that they put all their performance critical code in header files. That
> makes compilation faster, and has lots of benefits to the factoring and
> design of the code itself. We shouldn't then create an incentive to
keep
> things in header files so that they pick up a hint to the optimizer.
>
> Ultimately, the world will be a better place if we can eventually move code
> away from relying on the hint provided by the 'inline' keyword to
the
> optimizer.
>
While I would like to see that happen some day, I do think  it is an
independent matter.
>
> That doesn't mean that the core concept of hinting to the optimizer
that a
> particular function is a particularly good candidate for inlining is
without
> value.
yes.
>While I think it is a bad practice that we shouldn't encourage in
> code (especially portable code)
yes -- there are indeed programmers who use this casually without
considering performance.
> I can see the desire to at least have *some*
> attribute which is nothing more or less than a hint to the optimizer to
> inline harder[1].
yes -- there are programmers who use the attribute consciously.
> It would help people work around inliner bugs in the short
> term, and even help debug inliner-rooted optimization problems.
I think it is a good hint to the compiler even in the longer term.
With PGO, we should minimize the reliance on the hint though.
>Codebases
> with strong portability requirements could still (and probably should)
> forbid or tightly control access to this kind of hint. I would want really
> strong documentation about how this attribute *completely voids* your
> performance warranty (if such a thing exists) as from version to version of
> the compiler it may go from a helpful hint to a devastatingly bad hint.
Why? If the compiler becomes smarter and smarter, the inline hint will
become more and more irrelevant and eventually has no effect -- why
would the performance warranty be voided? If the compiler is not yet
smart enough, why would the compiler refuse to take the hint and
forbid developer provide the hint?
> But
> I think I could be persuaded to live with such a hint existing. But I'm
> *really* uncomfortable with it being tied to something that also impacts
> linkage or other semantics of the program.
For consistent with standard, we should pass the attribute. Linkage is
not affected in anyway.
>
> [1]: Currently, the only other hint we have available is pretty terrible as
> it *also* has semantic effects: the always_inline attribute.
>
>
>>
>> The proposed patch will add InlineHint to methods defined inside a
class:
>>
>> --- 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;
>>          }
>>
>> Here are the performance and size numbers I've collected:
>>
>>
>> - C++ subset of Spec: No performance effects, < 0.1% size increase
>> (all size numbers are text sizes returned by 'size')
>> - Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
>> file) , 4.1% size increase
>
>
> FWIW, this size increase seems *really* bad. I think that kills this
> approach even in the short term.
Re. size and performance trade-off -- 0.9% performance improvement
should greatly win the size cost. Besides among all programs see, only
clang sees this size increase with all the others seeing negligible
size increase.

This is not a short term vs long term situation. It is basically a bug
fix that FE drops the attribute. If it exposes inliner heuristic bug,
that should be fixed/tuned separately.  With the hint correctly passed
in, Easwaran will do further tuning including time based analysis.
>
>>
>> - Chrome: no performance improvement, 0.24% size increase
>> - Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
>> performance improvement, no size regression
>
>
> I'm also somewhat worried about the lack of any performance
improvements
> outside of the Google benchmarks. That somewhat strongly suggests that our
> benchmarks are overly coupled to this hint already. The fact that neither
> Chrome, Clang, nor SPEC improved is... not at all encouraging.
Other than Google benchmarks, we do see Clang improve performance.
Besides, current inliner needs to be further tuned in order to get
more performance benefit. Passing the hint through is simply an
enabler.  Also remember that most of SPEC benchmarks are C programs.
C++ programs with heavy use of virtual functions may not benefit a lot
either.

David

>
> -Chandler

Krzysztof Parzyszek

2015-Jul-08 21:40 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On 7/7/2015 8:06 PM, Chandler Carruth wrote:>
> I think that tying *any* optimizer behavior to the 'inline' keyword
is
> fundamentally the wrong direction. We have reasons why we have done this
> historically, and we can't just do an immediate about face, but we
> should be actively looking for ways to *reduce* the optimizer's
reliance
> on this keyword to convey any meaning whatsoever.
When it comes to performance, it is often the case that tuning the 
inliner can produce measurable benefits.  The problem is though, that 
these tweaks will likely degrade other applications.  I don't believe 
that the compiler's analysis of the code will ever be sufficient to 
adequately meet everyone's needs.  For now, the "inline" keyword
is used
as a hint that the programmer can provide to the compiler.  If we move 
away from paying any attention to "inline", we should keep in place 
another mechanism to allow the programmer to influence the inliner's 
decisions.

> The reason I think that is the correct direction is because, for better
> or worse, the 'inline' keyword in C++ is not about optimization,
but
> about linkage. It has a functional impact and can be both necessary or
> impossible to use to meet those functional requirements. This in turn
> leaves programmers in a lurch if the functional requirements are ever in
> tension with the optimizer requirements.
Could you give an example of such a situation?  The "inline" keyword
in
C has some unintuitive consequences (i.e. not provinding an "external 
definition"), but since C++ treats all inline functions as
"static",
this problem went away.  This is the only issue related to linkage that 
I am aware of.

> We're also working really hard to get more widely deployed cross-module
> optimization strategies, in part to free programmers from the
> requirement that they put all their performance critical code in header
> files. That makes compilation faster, and has lots of benefits to the
> factoring and design of the code itself. We shouldn't then create an
> incentive to keep things in header files so that they pick up a hint to
> the optimizer.
The incentive is already there---this is a programming practice that has 
existed for a long time.  We can provide alternative means (such as LTO) 
that are arguably better, but making existing practice worse for the 
programmer will not win too many people over.


-Krzysztof


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation

Nico Weber

2015-Jul-09 19:49 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On Tue, Jul 7, 2015 at 4:07 PM, Easwaran Raman <eraman at google.com>
wrote:
> I'm reviving this thread after a while and CCing cfe-commits as
> suggested by David Blaikie. I've also collected numbers building
> chrome (from chromium, on Linux) with and without this patch as
> suggested by David. I've re-posted the proposed patch and
> performance/size numbers collected at the top to make it easily
> readable for those reading it through cfe-commits.
>
> The proposed patch will add InlineHint to methods defined inside a class:
>
> --- 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;
>          }
>
> Here are the performance and size numbers I've collected:
>
>
> - C++ subset of Spec: No performance effects, < 0.1% size increase
> (all size numbers are text sizes returned by 'size')
> - Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
> file) , 4.1% size increase
> - Chrome: no performance improvement, 0.24% size increase
>
This is probably relative to a nonstripped linux release build? So this
means adding, what, 300kB to binary size without any benefit?

> - Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
> performance improvement, no size regression
>
> If there is any other important benchmark/application that needs to be
> evaluated, I'll work on that.
>
> The main skepticism in this thread is about whether a developer
> intends/expects a method defined in-class to be inlined or purely uses
> size of the method body to make this decision. I'll let CFE developers
> chime in on this. But irrespective of the intention, I think the data
> suggests this is a useful signal in some good cases and has a small
> size penalty in some bad cases. Note that if the criterion for placing
> it in-class is purely based on size, and assuming the inline-threshold
> is chosen to inline "small" functions, this change should only
affect
> a small number of functions (in the inline-threshold to
> inlinehint-threshold range) and the risk of serious size bloat is low.
>
> - Easwaran
>
> On Wed, Jun 24, 2015 at 3:15 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.
> >
> > & 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
> >> >
> >> >
> >
> >
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150709/2e48f574/attachment.html>

Xinliang David Li

2015-Jul-09 20:04 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On Thu, Jul 9, 2015 at 12:49 PM, Nico Weber <thakis at chromium.org>
wrote:> On Tue, Jul 7, 2015 at 4:07 PM, Easwaran Raman <eraman at google.com>
wrote:
>>
>> I'm reviving this thread after a while and CCing cfe-commits as
>> suggested by David Blaikie. I've also collected numbers building
>> chrome (from chromium, on Linux) with and without this patch as
>> suggested by David. I've re-posted the proposed patch and
>> performance/size numbers collected at the top to make it easily
>> readable for those reading it through cfe-commits.
>>
>> The proposed patch will add InlineHint to methods defined inside a
class:
>>
>> --- 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;
>>          }
>>
>> Here are the performance and size numbers I've collected:
>>
>>
>> - C++ subset of Spec: No performance effects, < 0.1% size increase
>> (all size numbers are text sizes returned by 'size')
>> - Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
>> file) , 4.1% size increase
>> - Chrome: no performance improvement, 0.24% size increase
>
>
> This is probably relative to a nonstripped linux release build? So this
> means adding, what, 300kB to binary size without any benefit?
Passing the hint through is not the end goal. It is an information
(about the source construct and user intention) that backend deserves
to know to enable more analysis based tuning selectively.  In the end,
I hope inliner can be tuned to make everybody happy, but that is not
always possible. For those who care about both performance and size,
PGO is the way to go.

David


>
>>
>> - Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
>> performance improvement, no size regression
>>
>> If there is any other important benchmark/application that needs to be
>> evaluated, I'll work on that.
>>
>> The main skepticism in this thread is about whether a developer
>> intends/expects a method defined in-class to be inlined or purely uses
>> size of the method body to make this decision. I'll let CFE
developers
>> chime in on this. But irrespective of the intention, I think the data
>> suggests this is a useful signal in some good cases and has a small
>> size penalty in some bad cases. Note that if the criterion for placing
>> it in-class is purely based on size, and assuming the inline-threshold
>> is chosen to inline "small" functions, this change should
only affect
>> a small number of functions (in the inline-threshold to
>> inlinehint-threshold range) and the risk of serious size bloat is low.
>>
>> - Easwaran
>>
>> On Wed, Jun 24, 2015 at 3:15 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.
>> >
>> > & 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
>> >> >
>> >> >
>> >
>> >
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>

Easwaran Raman

2015-Jul-09 20:49 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On Thu, Jul 9, 2015 at 12:49 PM, Nico Weber <thakis at chromium.org>
wrote:> On Tue, Jul 7, 2015 at 4:07 PM, Easwaran Raman <eraman at google.com>
wrote:
>>
>> I'm reviving this thread after a while and CCing cfe-commits as
>> suggested by David Blaikie. I've also collected numbers building
>> chrome (from chromium, on Linux) with and without this patch as
>> suggested by David. I've re-posted the proposed patch and
>> performance/size numbers collected at the top to make it easily
>> readable for those reading it through cfe-commits.
>>
>> The proposed patch will add InlineHint to methods defined inside a
class:
>>
>> --- 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;
>>          }
>>
>> Here are the performance and size numbers I've collected:
>>
>>
>> - C++ subset of Spec: No performance effects, < 0.1% size increase
>> (all size numbers are text sizes returned by 'size')
>> - Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
>> file) , 4.1% size increase
>> - Chrome: no performance improvement, 0.24% size increase
>
>
> This is probably relative to a nonstripped linux release build? So this
> means adding, what, 300kB to binary size without any benefit?
The sizes I reported are text sizes. This increases the text size by
~100K. The actual unstripped binary size actually decreases - likely
due to smaller symbol table as outline copies get eliminated. If
binary size is what you care about, this is a positive change for
chrome :)

Any heuristic based optimization is bound to negatively affect some
application even if it improves a vast majority of cases. Also, to put
this in perspective, if we set the inlinehint-threshold to
inline-threshold (equivalent to removing the hint for functions with
inline keyword), there is no change in performance* and the text size
reduces by ~150K from the current baseline.

*I am running the page_cycler.typical_25 benchmark and using the
change in cold and warm times as the performance metric.
>
>>
>> - Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
>> performance improvement, no size regression
>>
>> If there is any other important benchmark/application that needs to be
>> evaluated, I'll work on that.
>>
>> The main skepticism in this thread is about whether a developer
>> intends/expects a method defined in-class to be inlined or purely uses
>> size of the method body to make this decision. I'll let CFE
developers
>> chime in on this. But irrespective of the intention, I think the data
>> suggests this is a useful signal in some good cases and has a small
>> size penalty in some bad cases. Note that if the criterion for placing
>> it in-class is purely based on size, and assuming the inline-threshold
>> is chosen to inline "small" functions, this change should
only affect
>> a small number of functions (in the inline-threshold to
>> inlinehint-threshold range) and the risk of serious size bloat is low.
>>
>> - Easwaran
>>
>> On Wed, Jun 24, 2015 at 3:15 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.
>> >
>> > & 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
>> >> >
>> >> >
>> >
>> >
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>

Reasonably Related Threads

Search for more apparently analagous threads

llvm dev - Jul 2015 - [LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

[LLVMdev] Inline hint for methods defined in-class

Reasonably Related Threads