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 inclass 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 _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
AFAIK, this was fixed in r233817.
Original Message From: [hidden email] [mailto:[hidden email]] On Behalf Of Easwaran Raman Sent: Wednesday, June 17, 2015 6:59 PM To: [hidden email] Cc: David Li Subject: [LLVMdev] Inline hint for methods defined inclass 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 inclass 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 _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
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 <[hidden email]> wrote: > AFAIK, this was fixed in r233817. > > Original Message > From: [hidden email] [mailto:[hidden email]] On > Behalf Of Easwaran Raman > Sent: Wednesday, June 17, 2015 6:59 PM > To: [hidden email] > Cc: David Li > Subject: [LLVMdev] Inline hint for methods defined inclass > > 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 inclass 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 > _______________________________________________ > LLVM Developers mailing list > [hidden email] http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
Ping.
On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li <[hidden email]> 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 <[hidden email]> wrote: >> AFAIK, this was fixed in r233817. >> >> Original Message >> From: [hidden email] [mailto:[hidden email]] On >> Behalf Of Easwaran Raman >> Sent: Wednesday, June 17, 2015 6:59 PM >> To: [hidden email] >> Cc: David Li >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> 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 inclass 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 >> _______________________________________________ >> LLVM Developers mailing list >> [hidden email] http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
> Original Message > From: [hidden email] [mailto:[hidden email]] On > Behalf Of Easwaran Raman > Sent: Wednesday, June 24, 2015 9:54 AM > To: Xinliang David Li > Cc: <[hidden email]> List > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > > Ping. > > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li <[hidden email]> > 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 <[hidden email]> > wrote: > >> AFAIK, this was fixed in r233817. That was later reverted. > >> > >> Original Message > >> From: [hidden email] [mailto:[hidden email]] > On > >> Behalf Of Easwaran Raman > >> Sent: Wednesday, June 17, 2015 6:59 PM > >> To: [hidden email] > >> Cc: David Li > >> Subject: [LLVMdev] Inline hint for methods defined inclass > >> > >> 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 inclass 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 inclass 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 > >> [hidden email] http://llvm.cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> > _______________________________________________ > LLVM Developers mailing list > [hidden email] http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
The problem is that the other way around is not true: a function linkonce_odr linkage may be neither inline declared nor have inclass definition. David On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul <[hidden email]> wrote:
_______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
The method to identify functions with inclass 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 atpar with functions having the inline hint in inline cost analysis. Thanks, Easwaran On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li <[hidden email]> wrote: > The problem is that the other way around is not true: a function > linkonce_odr linkage may be neither inline declared nor have inclass > definition. > > David > > > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul > <[hidden email]> wrote: >> >> >> >> > Original Message >> > From: [hidden email] [mailto:[hidden email]] >> > On >> > Behalf Of Easwaran Raman >> > Sent: Wednesday, June 24, 2015 9:54 AM >> > To: Xinliang David Li >> > Cc: <[hidden email]> List >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> > >> > Ping. >> > >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li <[hidden email]> >> > 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 <[hidden email]> >> > wrote: >> > >> AFAIK, this was fixed in r233817. >> >> That was later reverted. >> >> > >> >> > >> Original Message >> > >> From: [hidden email] >> > >> [mailto:[hidden email]] >> > On >> > >> Behalf Of Easwaran Raman >> > >> Sent: Wednesday, June 17, 2015 6:59 PM >> > >> To: [hidden email] >> > >> Cc: David Li >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass >> > >> >> > >> 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 inclass 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 inclass 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 >> > >> [hidden email] http://llvm.cs.uiuc.edu >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> >> > _______________________________________________ >> > LLVM Developers mailing list >> > [hidden email] http://llvm.cs.uiuc.edu >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> _______________________________________________ >> LLVM Developers mailing list >> [hidden email] http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Xinliang David Li2

The problem is that the other way around is not true: a function linkonce_odr linkage may be neither inline declared nor have inclass definition. The inliner is not *prevented* from inlining a function that falls outside of those cases, as far as I know. A static filelevel function might be a good candidate,
even if it is not explicitly marked 'inline'. paulr From: Xinliang David Li [mailto:[hidden email]]
The problem is that the other way around is not true: a function linkonce_odr linkage may be neither inline declared nor have inclass definition. David On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul <[hidden email]> wrote:
A function with an inclass definition will have linkonce_odr linkage,
_______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
On Wed, Jun 24, 2015 at 1:59 PM, Robinson, Paul
<[hidden email]> wrote: >  The problem is that the other way around is not true: a function > linkonce_odr linkage may be neither inline declared nor have inclass > 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 inclass > definition? yes, I think there is a reason. Inclass 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 filelevel 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 > > paulr > > > > From: Xinliang David Li [mailto:[hidden email]] > Sent: Wednesday, June 24, 2015 12:56 PM > To: Robinson, Paul > Cc: Easwaran Raman; Xinliang David Li; <[hidden email]> List > > > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > > > > The problem is that the other way around is not true: a function > linkonce_odr linkage may be neither inline declared nor have inclass > definition. > > > > David > > > > > > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul > <[hidden email]> wrote: > > > >> Original Message >> From: [hidden email] [mailto:[hidden email]] On >> Behalf Of Easwaran Raman >> Sent: Wednesday, June 24, 2015 9:54 AM >> To: Xinliang David Li >> Cc: <[hidden email]> List >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> Ping. >> >> On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li <[hidden email]> >> 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 <[hidden email]> >> wrote: >> >> AFAIK, this was fixed in r233817. > > That was later reverted. > > >> >> >> >> Original Message >> >> From: [hidden email] [mailto:[hidden email]] >> On >> >> Behalf Of Easwaran Raman >> >> Sent: Wednesday, June 17, 2015 6:59 PM >> >> To: [hidden email] >> >> Cc: David Li >> >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> >> >> 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 inclass 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 inclass 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 >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> _______________________________________________ >> LLVM Developers mailing list >> [hidden email] http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > _______________________________________________ > LLVM Developers mailing list > [hidden email] http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Easwaran Raman
> Original Message
> From: Easwaran Raman [mailto:[hidden email]] > Sent: Wednesday, June 24, 2015 1:27 PM > To: Xinliang David Li > Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > > The method to identify functions with inclass 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 atpar 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 inclass definition. Granted that the way the C++ standard describes 'inline' it is effectively noise in that situation. paulr > > Thanks, > Easwaran > > > On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li > <[hidden email]> wrote: > > The problem is that the other way around is not true: a function > > linkonce_odr linkage may be neither inline declared nor have inclass > > definition. > > > > David > > > > > > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul > > <[hidden email]> wrote: > >> > >> > >> > >> > Original Message > >> > From: [hidden email] [mailto:llvmdev > [hidden email]] > >> > On > >> > Behalf Of Easwaran Raman > >> > Sent: Wednesday, June 24, 2015 9:54 AM > >> > To: Xinliang David Li > >> > Cc: <[hidden email]> List > >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> > > >> > Ping. > >> > > >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li > <[hidden email]> > >> > 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 > <[hidden email]> > >> > wrote: > >> > >> AFAIK, this was fixed in r233817. > >> > >> That was later reverted. > >> > >> > >> > >> > >> Original Message > >> > >> From: [hidden email] > >> > >> [mailto:[hidden email]] > >> > On > >> > >> Behalf Of Easwaran Raman > >> > >> Sent: Wednesday, June 17, 2015 6:59 PM > >> > >> To: [hidden email] > >> > >> Cc: David Li > >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass > >> > >> > >> > >> 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 inclass 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 inclass 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 > >> > >> [hidden email] http://llvm.cs.uiuc.edu > >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> > >> > >> > _______________________________________________ > >> > LLVM Developers mailing list > >> > [hidden email] http://llvm.cs.uiuc.edu > >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> > >> _______________________________________________ > >> LLVM Developers mailing list > >> [hidden email] http://llvm.cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul
<[hidden email]> wrote: >> Original Message >> From: Easwaran Raman [mailto:[hidden email]] >> Sent: Wednesday, June 24, 2015 1:27 PM >> To: Xinliang David Li >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass, which I think is not true. David > >> >> Thanks, >> Easwaran >> >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li >> <[hidden email]> wrote: >> > The problem is that the other way around is not true: a function >> > linkonce_odr linkage may be neither inline declared nor have inclass >> > definition. >> > >> > David >> > >> > >> > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul >> > <[hidden email]> wrote: >> >> >> >> >> >> >> >> > Original Message >> >> > From: [hidden email] [mailto:llvmdev >> [hidden email]] >> >> > On >> >> > Behalf Of Easwaran Raman >> >> > Sent: Wednesday, June 24, 2015 9:54 AM >> >> > To: Xinliang David Li >> >> > Cc: <[hidden email]> List >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> > >> >> > Ping. >> >> > >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li >> <[hidden email]> >> >> > 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 >> <[hidden email]> >> >> > wrote: >> >> > >> AFAIK, this was fixed in r233817. >> >> >> >> That was later reverted. >> >> >> >> > >> >> >> > >> Original Message >> >> > >> From: [hidden email] >> >> > >> [mailto:[hidden email]] >> >> > On >> >> > >> Behalf Of Easwaran Raman >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM >> >> > >> To: [hidden email] >> >> > >> Cc: David Li >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> > >> >> >> > >> 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 inclass 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 inclass 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 >> >> > >> [hidden email] http://llvm.cs.uiuc.edu >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > >> >> >> > _______________________________________________ >> >> > LLVM Developers mailing list >> >> > [hidden email] http://llvm.cs.uiuc.edu >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Robinson, Paul3
On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul
<[hidden email]> wrote: >> Original Message >> From: Easwaran Raman [mailto:[hidden email]] >> Sent: Wednesday, June 24, 2015 1:27 PM >> To: Xinliang David Li >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass definitions equivalent to having an 'inline' keyword, when combined with a higher inlinehintthreshold, 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 inclass.  Easwaran > paulr > >> >> Thanks, >> Easwaran >> >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li >> <[hidden email]> wrote: >> > The problem is that the other way around is not true: a function >> > linkonce_odr linkage may be neither inline declared nor have inclass >> > definition. >> > >> > David >> > >> > >> > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul >> > <[hidden email]> wrote: >> >> >> >> >> >> >> >> > Original Message >> >> > From: [hidden email] [mailto:llvmdev >> [hidden email]] >> >> > On >> >> > Behalf Of Easwaran Raman >> >> > Sent: Wednesday, June 24, 2015 9:54 AM >> >> > To: Xinliang David Li >> >> > Cc: <[hidden email]> List >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> > >> >> > Ping. >> >> > >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li >> <[hidden email]> >> >> > 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 >> <[hidden email]> >> >> > wrote: >> >> > >> AFAIK, this was fixed in r233817. >> >> >> >> That was later reverted. >> >> >> >> > >> >> >> > >> Original Message >> >> > >> From: [hidden email] >> >> > >> [mailto:[hidden email]] >> >> > On >> >> > >> Behalf Of Easwaran Raman >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM >> >> > >> To: [hidden email] >> >> > >> Cc: David Li >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> > >> >> >> > >> 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 inclass 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 inclass 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 >> >> > >> [hidden email] http://llvm.cs.uiuc.edu >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > >> >> >> > _______________________________________________ >> >> > LLVM Developers mailing list >> >> > [hidden email] http://llvm.cs.uiuc.edu >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
On Wed, Jun 24, 2015 at 2:20 PM, Easwaran Raman <[hidden email]> wrote: On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul 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 noninline 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
_______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Xinliang David Li
> Original Message
> From: Xinliang David Li [mailto:[hidden email]] > Sent: Wednesday, June 24, 2015 2:17 PM > To: Robinson, Paul > Cc: Easwaran Raman; Xinliang David Li; <[hidden email]> List > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > > On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul > <[hidden email]> wrote: > >> Original Message > >> From: Easwaran Raman [mailto:[hidden email]] > >> Sent: Wednesday, June 24, 2015 1:27 PM > >> To: Xinliang David Li > >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List > >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> > >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass, 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 inclass. paulr > > David > > > > >> > >> Thanks, > >> Easwaran > >> > >> > >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li > >> <[hidden email]> wrote: > >> > The problem is that the other way around is not true: a function > >> > linkonce_odr linkage may be neither inline declared nor have inclass > >> > definition. > >> > > >> > David > >> > > >> > > >> > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul > >> > <[hidden email]> wrote: > >> >> > >> >> > >> >> > >> >> > Original Message > >> >> > From: [hidden email] [mailto:llvmdev > >> [hidden email]] > >> >> > On > >> >> > Behalf Of Easwaran Raman > >> >> > Sent: Wednesday, June 24, 2015 9:54 AM > >> >> > To: Xinliang David Li > >> >> > Cc: <[hidden email]> List > >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> >> > > >> >> > Ping. > >> >> > > >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li > >> <[hidden email]> > >> >> > 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 > >> <[hidden email]> > >> >> > wrote: > >> >> > >> AFAIK, this was fixed in r233817. > >> >> > >> >> That was later reverted. > >> >> > >> >> > >> > >> >> > >> Original Message > >> >> > >> From: [hidden email] > >> >> > >> [mailto:[hidden email]] > >> >> > On > >> >> > >> Behalf Of Easwaran Raman > >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM > >> >> > >> To: [hidden email] > >> >> > >> Cc: David Li > >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass > >> >> > >> > >> >> > >> 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 inclass 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 inclass 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 > >> >> > >> [hidden email] http://llvm.cs.uiuc.edu > >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> > >> > >> >> > _______________________________________________ > >> >> > LLVM Developers mailing list > >> >> > [hidden email] http://llvm.cs.uiuc.edu > >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> > >> >> _______________________________________________ > >> >> LLVM Developers mailing list > >> >> [hidden email] http://llvm.cs.uiuc.edu > >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> > > >> > _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by David Blaikie
On Wed, Jun 24, 2015 at 2:35 PM, David Blaikie <[hidden email]> wrote:
> > > On Wed, Jun 24, 2015 at 2:20 PM, Easwaran Raman <[hidden email]> wrote: >> >> On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul >> <[hidden email]> wrote: >> >> Original Message >> >> From: Easwaran Raman [mailto:[hidden email]] >> >> Sent: Wednesday, June 24, 2015 1:27 PM >> >> To: Xinliang David Li >> >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List >> >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass definitions >> equivalent to having an 'inline' keyword, when combined with a higher >> inlinehintthreshold, 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 inclass. > > > 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 inclass 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 noninline 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 > >  David > >> >> >>  Easwaran >> >> > paulr >> > >> >> >> >> Thanks, >> >> Easwaran >> >> >> >> >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li >> >> <[hidden email]> wrote: >> >> > The problem is that the other way around is not true: a function >> >> > linkonce_odr linkage may be neither inline declared nor have inclass >> >> > definition. >> >> > >> >> > David >> >> > >> >> > >> >> > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul >> >> > <[hidden email]> wrote: >> >> >> >> >> >> >> >> >> >> >> >> > Original Message >> >> >> > From: [hidden email] [mailto:llvmdev >> >> [hidden email]] >> >> >> > On >> >> >> > Behalf Of Easwaran Raman >> >> >> > Sent: Wednesday, June 24, 2015 9:54 AM >> >> >> > To: Xinliang David Li >> >> >> > Cc: <[hidden email]> List >> >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> >> > >> >> >> > Ping. >> >> >> > >> >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li >> >> <[hidden email]> >> >> >> > 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 >> >> <[hidden email]> >> >> >> > wrote: >> >> >> > >> AFAIK, this was fixed in r233817. >> >> >> >> >> >> That was later reverted. >> >> >> >> >> >> > >> >> >> >> > >> Original Message >> >> >> > >> From: [hidden email] >> >> >> > >> [mailto:[hidden email]] >> >> >> > On >> >> >> > >> Behalf Of Easwaran Raman >> >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM >> >> >> > >> To: [hidden email] >> >> >> > >> Cc: David Li >> >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> >> > >> >> >> >> > >> 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 inclass 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 inclass 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 >> >> >> > >> [hidden email] http://llvm.cs.uiuc.edu >> >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> > >> >> >> >> > _______________________________________________ >> >> >> > LLVM Developers mailing list >> >> >> > [hidden email] http://llvm.cs.uiuc.edu >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> >> >> _______________________________________________ >> >> >> LLVM Developers mailing list >> >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > >> >> > >> _______________________________________________ >> LLVM Developers mailing list >> [hidden email] http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
> Original Message > From: Xinliang David Li [mailto:[hidden email]] > Sent: Wednesday, June 24, 2015 2:45 PM > To: David Blaikie > Cc: Easwaran Raman; Robinson, Paul; <[hidden email]> List > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > > On Wed, Jun 24, 2015 at 2:35 PM, David Blaikie <[hidden email]> wrote: > > > > > > On Wed, Jun 24, 2015 at 2:20 PM, Easwaran Raman <[hidden email]> > wrote: > >> > >> On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul > >> <[hidden email]> wrote: > >> >> Original Message > >> >> From: Easwaran Raman [mailto:[hidden email]] > >> >> Sent: Wednesday, June 24, 2015 1:27 PM > >> >> To: Xinliang David Li > >> >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List > >> >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> >> > >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass definitions > >> equivalent to having an 'inline' keyword, when combined with a higher > >> inlinehintthreshold, 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 inclass. > > > > > > 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 > inclass 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 inclass regardless of size or other suitability for inlining. Marking such methods with inlinehint is not really appropriate. paulr > > > > > 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 noninline 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 > > > > > >  David > > > >> > >> > >>  Easwaran > >> > >> > paulr > >> > > >> >> > >> >> Thanks, > >> >> Easwaran > >> >> > >> >> > >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li > >> >> <[hidden email]> 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 > >> >> > <[hidden email]> wrote: > >> >> >> > >> >> >> > >> >> >> > >> >> >> > Original Message > >> >> >> > From: [hidden email] [mailto:llvmdev > >> >> [hidden email]] > >> >> >> > On > >> >> >> > Behalf Of Easwaran Raman > >> >> >> > Sent: Wednesday, June 24, 2015 9:54 AM > >> >> >> > To: Xinliang David Li > >> >> >> > Cc: <[hidden email]> List > >> >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> >> >> > > >> >> >> > Ping. > >> >> >> > > >> >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li > >> >> <[hidden email]> > >> >> >> > 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 > >> >> <[hidden email]> > >> >> >> > wrote: > >> >> >> > >> AFAIK, this was fixed in r233817. > >> >> >> > >> >> >> That was later reverted. > >> >> >> > >> >> >> > >> > >> >> >> > >> Original Message > >> >> >> > >> From: [hidden email] > >> >> >> > >> [mailto:[hidden email]] > >> >> >> > On > >> >> >> > >> Behalf Of Easwaran Raman > >> >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM > >> >> >> > >> To: [hidden email] > >> >> >> > >> Cc: David Li > >> >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass > >> >> >> > >> > >> >> >> > >> 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 inclass 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 inclass 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 > >> >> >> > >> [hidden email] http://llvm.cs.uiuc.edu > >> >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> >> > >> > >> >> >> > _______________________________________________ > >> >> >> > LLVM Developers mailing list > >> >> >> > [hidden email] http://llvm.cs.uiuc.edu > >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> >> > >> >> >> _______________________________________________ > >> >> >> LLVM Developers mailing list > >> >> >> [hidden email] http://llvm.cs.uiuc.edu > >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> > > >> >> > > >> _______________________________________________ > >> LLVM Developers mailing list > >> [hidden email] http://llvm.cs.uiuc.edu > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > > _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Robinson, Paul3
Sorry for misinterpreting, but what is the basis for the simple fact
you mentioned? David On Wed, Jun 24, 2015 at 2:43 PM, Robinson, Paul <[hidden email]> wrote: >> Original Message >> From: Xinliang David Li [mailto:[hidden email]] >> Sent: Wednesday, June 24, 2015 2:17 PM >> To: Robinson, Paul >> Cc: Easwaran Raman; Xinliang David Li; <[hidden email]> List >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul >> <[hidden email]> wrote: >> >> Original Message >> >> From: Easwaran Raman [mailto:[hidden email]] >> >> Sent: Wednesday, June 24, 2015 1:27 PM >> >> To: Xinliang David Li >> >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List >> >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass, 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 inclass. > paulr > > >> >> David >> >> > >> >> >> >> Thanks, >> >> Easwaran >> >> >> >> >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li >> >> <[hidden email]> wrote: >> >> > The problem is that the other way around is not true: a function >> >> > linkonce_odr linkage may be neither inline declared nor have inclass >> >> > definition. >> >> > >> >> > David >> >> > >> >> > >> >> > On Wed, Jun 24, 2015 at 11:53 AM, Robinson, Paul >> >> > <[hidden email]> wrote: >> >> >> >> >> >> >> >> >> >> >> >> > Original Message >> >> >> > From: [hidden email] [mailto:llvmdev >> >> [hidden email]] >> >> >> > On >> >> >> > Behalf Of Easwaran Raman >> >> >> > Sent: Wednesday, June 24, 2015 9:54 AM >> >> >> > To: Xinliang David Li >> >> >> > Cc: <[hidden email]> List >> >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> >> > >> >> >> > Ping. >> >> >> > >> >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li >> >> <[hidden email]> >> >> >> > 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 >> >> <[hidden email]> >> >> >> > wrote: >> >> >> > >> AFAIK, this was fixed in r233817. >> >> >> >> >> >> That was later reverted. >> >> >> >> >> >> > >> >> >> >> > >> Original Message >> >> >> > >> From: [hidden email] >> >> >> > >> [mailto:[hidden email]] >> >> >> > On >> >> >> > >> Behalf Of Easwaran Raman >> >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM >> >> >> > >> To: [hidden email] >> >> >> > >> Cc: David Li >> >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> >> > >> >> >> >> > >> 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 inclass 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 inclass 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 >> >> >> > >> [hidden email] http://llvm.cs.uiuc.edu >> >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> > >> >> >> >> > _______________________________________________ >> >> >> > LLVM Developers mailing list >> >> >> > [hidden email] http://llvm.cs.uiuc.edu >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> >> >> _______________________________________________ >> >> >> LLVM Developers mailing list >> >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> > >> >> > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Robinson, Paul3
I agree with this statement.
David On Wed, Jun 24, 2015 at 2:49 PM, Robinson, Paul <[hidden email]> wrote: > > >> Original Message >> From: Xinliang David Li [mailto:[hidden email]] >> Sent: Wednesday, June 24, 2015 2:45 PM >> To: David Blaikie >> Cc: Easwaran Raman; Robinson, Paul; <[hidden email]> List >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> On Wed, Jun 24, 2015 at 2:35 PM, David Blaikie <[hidden email]> wrote: >> > >> > >> > On Wed, Jun 24, 2015 at 2:20 PM, Easwaran Raman <[hidden email]> >> wrote: >> >> >> >> On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul >> >> <[hidden email]> wrote: >> >> >> Original Message >> >> >> From: Easwaran Raman [mailto:[hidden email]] >> >> >> Sent: Wednesday, June 24, 2015 1:27 PM >> >> >> To: Xinliang David Li >> >> >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List >> >> >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> >> >> >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass definitions >> >> equivalent to having an 'inline' keyword, when combined with a higher >> >> inlinehintthreshold, 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 inclass. >> > >> > >> > 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 >> inclass 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 > inclass regardless of size or other suitability for inlining. Marking > such methods with inlinehint is not really appropriate. > paulr > >> >> > >> > 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 noninline 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 >> >> >> > >> >  David >> > >> >> >> >> >> >>  Easwaran >> >> >> >> > paulr >> >> > >> >> >> >> >> >> Thanks, >> >> >> Easwaran >> >> >> >> >> >> >> >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li >> >> >> <[hidden email]> 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 >> >> >> > <[hidden email]> wrote: >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> > Original Message >> >> >> >> > From: [hidden email] [mailto:llvmdev >> >> >> [hidden email]] >> >> >> >> > On >> >> >> >> > Behalf Of Easwaran Raman >> >> >> >> > Sent: Wednesday, June 24, 2015 9:54 AM >> >> >> >> > To: Xinliang David Li >> >> >> >> > Cc: <[hidden email]> List >> >> >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass >> >> >> >> > >> >> >> >> > Ping. >> >> >> >> > >> >> >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li >> >> >> <[hidden email]> >> >> >> >> > 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 >> >> >> <[hidden email]> >> >> >> >> > wrote: >> >> >> >> > >> AFAIK, this was fixed in r233817. >> >> >> >> >> >> >> >> That was later reverted. >> >> >> >> >> >> >> >> > >> >> >> >> >> > >> Original Message >> >> >> >> > >> From: [hidden email] >> >> >> >> > >> [mailto:[hidden email]] >> >> >> >> > On >> >> >> >> > >> Behalf Of Easwaran Raman >> >> >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM >> >> >> >> > >> To: [hidden email] >> >> >> >> > >> Cc: David Li >> >> >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass >> >> >> >> > >> >> >> >> >> > >> 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 inclass 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 inclass 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 >> >> >> >> > >> [hidden email] http://llvm.cs.uiuc.edu >> >> >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> > >> >> >> >> >> > _______________________________________________ >> >> >> >> > LLVM Developers mailing list >> >> >> >> > [hidden email] http://llvm.cs.uiuc.edu >> >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> >> >> >> >> >> _______________________________________________ >> >> >> >> LLVM Developers mailing list >> >> >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> > >> >> >> > >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> [hidden email] http://llvm.cs.uiuc.edu >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >> > LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Xinliang David Li
> Original Message > From: Xinliang David Li [mailto:[hidden email]] > Sent: Wednesday, June 24, 2015 2:50 PM > To: Robinson, Paul > Cc: Easwaran Raman; Xinliang David Li; <[hidden email]> List > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > > Sorry for misinterpreting, but what is the basis for the simple fact > you mentioned? The patch causes all inclassdefined 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 > > David > > On Wed, Jun 24, 2015 at 2:43 PM, Robinson, Paul > <[hidden email]> wrote: > >> Original Message > >> From: Xinliang David Li [mailto:[hidden email]] > >> Sent: Wednesday, June 24, 2015 2:17 PM > >> To: Robinson, Paul > >> Cc: Easwaran Raman; Xinliang David Li; <[hidden email]> List > >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> > >> On Wed, Jun 24, 2015 at 2:10 PM, Robinson, Paul > >> <[hidden email]> wrote: > >> >> Original Message > >> >> From: Easwaran Raman [mailto:[hidden email]] > >> >> Sent: Wednesday, June 24, 2015 1:27 PM > >> >> To: Xinliang David Li > >> >> Cc: Robinson, Paul; Xinliang David Li; <[hidden email]> List > >> >> Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> >> > >> >> The method to identify functions with inclass 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 atpar 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 inclass 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 inclass, 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 inclass. > > paulr > > > > > >> > >> David > >> > >> > > >> >> > >> >> Thanks, > >> >> Easwaran > >> >> > >> >> > >> >> On Wed, Jun 24, 2015 at 12:56 PM, Xinliang David Li > >> >> <[hidden email]> 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 > >> >> > <[hidden email]> wrote: > >> >> >> > >> >> >> > >> >> >> > >> >> >> > Original Message > >> >> >> > From: [hidden email] [mailto:llvmdev > >> >> [hidden email]] > >> >> >> > On > >> >> >> > Behalf Of Easwaran Raman > >> >> >> > Sent: Wednesday, June 24, 2015 9:54 AM > >> >> >> > To: Xinliang David Li > >> >> >> > Cc: <[hidden email]> List > >> >> >> > Subject: Re: [LLVMdev] Inline hint for methods defined inclass > >> >> >> > > >> >> >> > Ping. > >> >> >> > > >> >> >> > On Wed, Jun 17, 2015 at 4:13 PM, Xinliang David Li > >> >> <[hidden email]> > >> >> >> > 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 > >> >> <[hidden email]> > >> >> >> > wrote: > >> >> >> > >> AFAIK, this was fixed in r233817. > >> >> >> > >> >> >> That was later reverted. > >> >> >> > >> >> >> > >> > >> >> >> > >> Original Message > >> >> >> > >> From: [hidden email] > >> >> >> > >> [mailto:[hidden email]] > >> >> >> > On > >> >> >> > >> Behalf Of Easwaran Raman > >> >> >> > >> Sent: Wednesday, June 17, 2015 6:59 PM > >> >> >> > >> To: [hidden email] > >> >> >> > >> Cc: David Li > >> >> >> > >> Subject: [LLVMdev] Inline hint for methods defined inclass > >> >> >> > >> > >> >> >> > >> 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 inclass 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 inclass 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 > >> >> >> > >> [hidden email] http://llvm.cs.uiuc.edu > >> >> >> > >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> >> > >> > >> >> >> > _______________________________________________ > >> >> >> > LLVM Developers mailing list > >> >> >> > [hidden email] http://llvm.cs.uiuc.edu > >> >> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> >> > >> >> >> _______________________________________________ > >> >> >> LLVM Developers mailing list > >> >> >> [hidden email] http://llvm.cs.uiuc.edu > >> >> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >> >> > > >> >> > _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
In reply to this post by Robinson, Paul3
On Wed, Jun 24, 2015 at 2:16 PM Robinson, Paul <[hidden email]> wrote: > Original Message 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 paulr _______________________________________________ LLVM Developers mailing list [hidden email] http://llvm.cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev 
Free forum by Nabble  Edit this page 