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

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

Xinliang David Li

2015-Jun-24 21:58 UTC

[LLVMdev] Inline hint for methods defined in-class

On Wed, Jun 24, 2015 at 2:52 PM, Robinson, Paul
<Paul_Robinson at playstation.sony.com> wrote:>
>
>> -----Original Message-----
>> From: Xinliang David Li [mailto:davidxl at google.com]
>> Sent: Wednesday, June 24, 2015 2:50 PM
>> To: Robinson, Paul
>> Cc: Easwaran Raman; Xinliang David Li; <llvmdev at 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.
I agree the noise can come from template class template functions
where users do not have a choice.  There should ways to detect those
and filter them out.

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

Duncan P. N. Exon Smith

2015-Jun-24 23:49 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

> On 2015 Jun 24, at 14:58, Xinliang David Li <davidxl at google.com>
wrote:
> 
> On Wed, Jun 24, 2015 at 2:52 PM, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
>> 
>> 
>>> -----Original Message-----
>>> From: Xinliang David Li [mailto:davidxl at google.com]
>>> Sent: Wednesday, June 24, 2015 2:50 PM
>>> To: Robinson, Paul
>>> Cc: Easwaran Raman; Xinliang David Li; <llvmdev at
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.
> 
> I agree the noise can come from template class template functions
> where users do not have a choice.  There should ways to detect those
> and filter them out.
Users do have a choice, even for template functions.

    template <class T> struct Foo {
      void thisIsInline() {} 
      void thisIsNotInline();
      template <class U> void thisIsInlineToo() {}
      template <class U> void thisIsNotInlineEither();
    }; 
    template <class T> void Foo<T>::thisIsNotInline() {} 
    template <class T> 
    template <class U> 
    void Foo<T>::thisIsNotInlineEither() {} 

An `inline` keyword on a function defined inline in a class has always
looked like noise to me.

I think adding the `inline` keyword to methods defined inline in a class
*should* have no effect.  (Maybe this is just bias from having used GCC
for years?)

Xinliang David Li

2015-Jun-25 01:10 UTC

head link

[LLVMdev] Inline hint for methods defined in-class

On Wed, Jun 24, 2015 at 4:49 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:>
>> On 2015 Jun 24, at 14:58, Xinliang David Li <davidxl at
google.com> wrote:
>>
>> On Wed, Jun 24, 2015 at 2:52 PM, Robinson, Paul
>> <Paul_Robinson at playstation.sony.com> wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Xinliang David Li [mailto:davidxl at google.com]
>>>> Sent: Wednesday, June 24, 2015 2:50 PM
>>>> To: Robinson, Paul
>>>> Cc: Easwaran Raman; Xinliang David Li; <llvmdev at
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.
>>
>> I agree the noise can come from template class template functions
>> where users do not have a choice.  There should ways to detect those
>> and filter them out.
>
> Users do have a choice, even for template functions.
What I really meant is that user does not have choice but to put defs
inside headers. You are totally right that users do have the following
choices even for template functions:

1) in-class definition without inline keyword
2) in-class definition with inline keyword
3) out of line/class definition with inline keyword
4) out of line definition without inline keyword
>
>     template <class T> struct Foo {
>       void thisIsInline() {}
>       void thisIsNotInline();
>       template <class U> void thisIsInlineToo() {}
>       template <class U> void thisIsNotInlineEither();
>     };
>     template <class T> void Foo<T>::thisIsNotInline() {}
>     template <class T>
>     template <class U>
>     void Foo<T>::thisIsNotInlineEither() {}
>
> An `inline` keyword on a function defined inline in a class has always
> looked like noise to me.
yes.
>
> I think adding the `inline` keyword to methods defined inline in a class
> *should* have no effect.  (Maybe this is just bias from having used GCC
> for years?)
Agree. Sine adding inline keyword to in-class defs is a no-op
essentially, the in-class definition with 'inline' hint is expected.

David

llvm dev - Jun 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