Xinliang David Li
2015-Jun-17 23:13 UTC
[LLVMdev] Inline hint for methods defined in-class
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.
>
> -----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.
>
> Thanks,
> Easwaran
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu 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 <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. >> >> -----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. >> >> Thanks, >> Easwaran >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >>
> -----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