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.

Thanks,
Easwaran

AFAIK, this was fixed in r233817.

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

Ping.

From: llvmdev-bounces@cs.uiuc.edu [mailto:llvmdev-bounces@cs.uiuc.edu] On
Behalf Of Easwaran Raman
Sent: Wednesday, June 24, 2015 9:54 AM
To: Xinliang David Li
Cc: <llvmdev@cs.uiuc.edu> List
Subject: Re: [LLVMdev] Inline hint for methods defined in-class

Ping.

> 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
>
>> AFAIK, this was fixed in r233817.

That was later reverted.

>>
>> From: llvmdev-bounces@cs.uiuc.edu [mailto:llvmdev-bounces@cs.uiuc.edu]
On
>> Behalf Of Easwaran Raman
>> Sent: Wednesday, June 17, 2015 6:59 PM
>> To: llvmdev@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

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

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.

Thanks,
Easwaran

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.

Sorry, why is that a problem? Did you have some other reason to want to identify functions that either specify ‘inline’ or have an in-class definition?

The inliner is not prevented from inlining a function that falls outside of those cases, as far as I know. A static file-level function might be a good candidate, even if it is not explicitly marked ‘inline’.

–paulr

> 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.

Sorry, why is that a problem? Did you have some other reason to want to
identify functions that either specify 'inline' or have an in-class
definition?

yes, I think there is a reason. In-class definitions are another way
for user to give inline hint (which depends on coding style). This
does not apply to other cases such as template function
instantiations.

The inliner is not *prevented* from inlining a function that falls outside
of those cases, as far as I know. A static file-level function might be a
good candidate, even if it is not explicitly marked 'inline'.

yes, but those other cases have explicit attribute for inliner to tell
them apart.

David

From: Easwaran Raman [mailto:eraman@google.com]
Sent: Wednesday, June 24, 2015 1:27 PM
To: Xinliang David Li
Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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

From: Easwaran Raman [mailto:eraman@google.com]
Sent: Wednesday, June 24, 2015 1:27 PM
To: Xinliang David Li
Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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

From: Easwaran Raman [mailto:eraman@google.com]
Sent: Wednesday, June 24, 2015 1:27 PM
To: Xinliang David Li
Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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.

- Easwaran

>> From: Easwaran Raman [mailto:eraman@google.com]
>> Sent: Wednesday, June 24, 2015 1:27 PM
>> To: Xinliang David Li
>> Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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)

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.

- David

From: Xinliang David Li [mailto:davidxl@google.com]
Sent: Wednesday, June 24, 2015 2:17 PM
To: Robinson, Paul
Cc: Easwaran Raman; Xinliang David Li; <llvmdev@cs.uiuc.edu> List
Subject: Re: [LLVMdev] Inline hint for methods defined in-class

>> From: Easwaran Raman [mailto:eraman@google.com]
>> Sent: Wednesday, June 24, 2015 1:27 PM
>> To: Xinliang David Li
>> Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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.

I'm not assuming any such thing. Neither my opinion about how I would
prefer the compiler to respond to the 'inline' keyword, nor the simple
fact that the patch turns some uses of the 'inline' keyword into noise,
are in any way related to how often functions are defined in-class.
--paulr

>> From: Easwaran Raman [mailto:eraman@google.com]
>> Sent: Wednesday, June 24, 2015 1:27 PM
>> To: Xinliang David Li
>> Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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)

Good observation, but not quite complete. It is true that a lot of the
in-class definitions are small functions (as the author does not
bother to provide standalone defintions). However those cases are not
interesting, as regular inline heuristics can handle already.

The interesting cases are those where user explicitly/deliberately
puts relatively large bodies in class. They also really want the body
to be visible to all TUs (so that inliner can do something) -- it is a
strong hint.

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.

That is ideal, but something we will improve independently. Assuming
inliner can not do a perfect job, not differentiating functions with
hints can result in large size growth.

David

From: Xinliang David Li [mailto:davidxl@google.com]
Sent: Wednesday, June 24, 2015 2:45 PM
To: David Blaikie
Cc: Easwaran Raman; Robinson, Paul; <llvmdev@cs.uiuc.edu> List
Subject: Re: [LLVMdev] Inline hint for methods defined in-class

>
>
>>
>> >> From: Easwaran Raman [mailto:eraman@google.com]
>> >> Sent: Wednesday, June 24, 2015 1:27 PM
>> >> To: Xinliang David Li
>> >> Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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)

Good observation, but not quite complete. It is true that a lot of the
in-class definitions are small functions (as the author does not
bother to provide standalone defintions). However those cases are not
interesting, as regular inline heuristics can handle already.

The interesting cases are those where user explicitly/deliberately
puts relatively large bodies in class. They also really want the body
to be visible to all TUs (so that inliner can do something) -- it is a
strong hint.

I think in a template class, pretty much all methods have to be defined
in-class regardless of size or other suitability for inlining. Marking
such methods with inlinehint is not really appropriate.
--paulr

Sorry for misinterpreting, but what is the basis for the simple fact
you mentioned?

David

I agree with this statement.

David

From: Xinliang David Li [mailto:davidxl@google.com]
Sent: Wednesday, June 24, 2015 2:50 PM
To: Robinson, Paul
Cc: Easwaran Raman; Xinliang David Li; <llvmdev@cs.uiuc.edu> List
Subject: Re: [LLVMdev] Inline hint for methods defined in-class

Sorry for misinterpreting, but what is the basis for the simple fact
you mentioned?

The patch causes all in-class-defined methods to be treated as if
they had the 'inline' keyword attached.
Therefore, with the patch, explicitly adding the 'inline' keyword to
these methods has no effect; it becomes noise.
--paulr

From: Easwaran Raman [mailto:eraman@google.com]
Sent: Wednesday, June 24, 2015 1:27 PM
To: Xinliang David Li
Cc: Robinson, Paul; Xinliang David Li; <llvmdev@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.

FWIW in my experience here the inline keyword is pretty much useless. It may, or may not, have been applied correctly in the first place, but after in a code base of any appreciable size or age it’s almost assuredly no different from noise when analyzing actual performance.

Other people’s mileage may vary.

-eric