[LLVMdev] Inline hint for methods defined in-class

Chandler Carruth chandlerc at gmail.com
Wed Jul 8 22:29:09 PDT 2015

On Tue, Jul 7, 2015 at 10:27 PM Xinliang David Li <davidxl at google.com>

> On Tue, Jul 7, 2015 at 6:06 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
> > On Tue, Jul 7, 2015 at 4:11 PM Easwaran Raman <eraman at google.com> wrote:
> >>
> >> I'm reviving this thread after a while and CCing cfe-commits as
> >> suggested by David Blaikie. I've also collected numbers building
> >> chrome (from chromium, on Linux) with and without this patch as
> >> suggested by David. I've re-posted the proposed patch and
> >> performance/size numbers collected at the top to make it easily
> >> readable for those reading it through cfe-commits.
> >
> >
> > First off, thanks for collecting the numbers and broadening the
> > distribution. Also, sorry it took me so long to get over to this thread.
> >
> > I want to lay out my stance on this issue at a theoretical and practical
> > level first. I'll follow up with thoughts on the numbers as well after
> that.
> >
> > I think that tying *any* optimizer behavior to the 'inline' keyword is
> > fundamentally the wrong direction.
> Chandler, thanks for sharing your thought -- however I don't think it
> is wrong, let alone 'fundamentally wrong'.  Despite all the analysis
> that can be done, the inliner is in the end heuristic based. In lack
> of the profile data, when inlining two calls yield the same static
> benefit and size cost, it is reasonable for the inliner to think the
> call to the function with inline hint to yield more high
> dynamic/runtime benefit -- thus it has a higher static size budget to
> burn.

This is an argument for having *some* source level hinting construct. While
I think such a construct is very risky, I did actually suggest that we add
such a hint because I recognize some of the practical necessities for it.

My primary argument is against using the 'inline' keyword as the source of
the hint, and especially using the happenstance of a method body in a class
as the source of the hint.

> >We have reasons why we have done this
> > historically, and we can't just do an immediate about face, but we
> should be
> > actively looking for ways to *reduce* the optimizer's reliance on this
> > keyword to convey any meaning whatsoever.
> yes those additional things will be done, but they are really orthogonal.

I'm not talking about additional things, I'm talking about separating the
optimization hint from the semantics and linkage changing constructs. That
does not seem orthogonal.

> >
> > The reason I think that is the correct direction is because, for better
> or
> > worse, the 'inline' keyword in C++ is not about optimization, but about
> > linkage.
> It is about both optimization and linkage. In fact the linkage simply
> serves as an implementation detail. In C++ standard 7.1.2,  paragraph
> 2 says:
> "A function declaration (8.3.5, 9.3, 11.3) with an inline specifier
> declares an inline function. The inline specifier indicates to the
> implementation that inline substitution of the function body at the
> point of call is to be preferred to the usual function call mechanism.
> An implementation is not required to perform this inline substitution
> at the point of call; however, even if this inline substitution is
> omitted, the other rules for inline functions defined by 7.1.2 shall
> still be respected."
> Developers see those and rely on those to give compiler the hints.

This is essentially a nonsense paragraph for a standardized specification
of a programming language. How hard you optimize code doesn't have any
bearing on the conformance and behavior of the program. =/

I think this paragraph is part of the historical context. I think we should
change C++ to remove it (and I will propose that change) and I think we
should change C++ to support a standardized *non*-semantic hint if folks
really want to see that in the form of a C++11-style [[attribute]].

I'm also really, really confident that most developers are not using the
wording of the standard as the basis of how they tune the performance of
their code. =/

> Most importantly, paragraph 3 says:
> "A function defined within a class definition is an inline function.
> The inline specifier shall not appear on a block scope function
> declaration.93 If the inline specifier is used in a friend
> declaration, that declaration shall be a definition or the function
> shall have previously been declared inline."
> Here we can see regardless of how optimizer will honor the hint and to
> what extent, and based on what analysis,
> it is basically incorrect to drop the attribute on the floor for
> in-class function definitions. Eswaran's fix is justified with this
> reason alone.  The side effect of changing inliner behavior is
> irrelevant.

I don't really understand what you're saying. Clang correctly carries all
of the *semantics* required for in-class method bodies. We simply don't
attach an optimization hint? I don't think this is "incorrect". Nothing in
the standard says how hard we should try (and it can't, which is why the
standard doesn't make sense to give advice here).

> It has a functional impact and can be both necessary or impossible
> > to use to meet those functional requirements. This in turn leaves
> > programmers in a lurch if the functional requirements are ever in tension
> > with the optimizer requirements.
> Not sure what you mean. Performance conscious programmers use it all the
> time.

You might *have* to use the inline keyword to get correct linkage even when
it causes the optimizer to inline and that *hurts* performance.

Similarly, you might *have* to not use the inline keyword to get correct
linkage even though you would like to give the optimizer a hint for
performance reasons (and are doing LTO, so you can).

Essentially, there is no guarantee that the semantic requirements of
linkage are correctly aligned with the desired optimizer hint.

> >
> > We're also working really hard to get more widely deployed cross-module
> > optimization strategies, in part to free programmers from the requirement
> > that they put all their performance critical code in header files. That
> > makes compilation faster, and has lots of benefits to the factoring and
> > design of the code itself. We shouldn't then create an incentive to keep
> > things in header files so that they pick up a hint to the optimizer.
> >
> > Ultimately, the world will be a better place if we can eventually move
> code
> > away from relying on the hint provided by the 'inline' keyword to the
> > optimizer.
> >
> While I would like to see that happen some day, I do think  it is an
> independent matter.

Adding further optimization hints based around the linkage further
entrenches that model. If we want to move in this direction, this patch is
a step in the *wrong* direction.

> >Codebases
> > with strong portability requirements could still (and probably should)
> > forbid or tightly control access to this kind of hint. I would want
> really
> > strong documentation about how this attribute *completely voids* your
> > performance warranty (if such a thing exists) as from version to version
> of
> > the compiler it may go from a helpful hint to a devastatingly bad hint.
> Why? If the compiler becomes smarter and smarter, the inline hint will
> become more and more irrelevant and eventually has no effect -- why
> would the performance warranty be voided? If the compiler is not yet
> smart enough, why would the compiler refuse to take the hint and
> forbid developer provide the hint?

I didn't suggest the compiler would do anything. I suggested that the idea
of hinting an optimizer about how to inline code is inherently non-portable
(its specific to an optimizer) and thus would likely be less used in code
bases with unusually strong portability concerns.

> > But
> > I think I could be persuaded to live with such a hint existing. But I'm
> > *really* uncomfortable with it being tied to something that also impacts
> > linkage or other semantics of the program.
> For consistent with standard, we should pass the attribute. Linkage is
> not affected in anyway.

I do not think that there is any standards-consistency argument for one
optimization hint over another. We already get the linkage correct for all
of these things. The only consideration is the *degree* to which we prefer
to actually do inlining in the optimizer. The standard at no point makes
guarantees about these degrees.

> >
> > [1]: Currently, the only other hint we have available is pretty terrible
> as
> > it *also* has semantic effects: the always_inline attribute.
> >
> >
> >>
> >> The proposed patch will add InlineHint to methods defined inside a
> class:
> >>
> >> --- a/lib/CodeGen/CodeGenFunction.cpp
> >> +++ b/lib/CodeGen/CodeGenFunction.cpp
> >> @@ -630,7 +630,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD,
> >>    if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
> >>      if (!CGM.getCodeGenOpts().NoInline) {
> >>        for (auto RI : FD->redecls())
> >> -        if (RI->isInlineSpecified()) {
> >> +        if (RI->isInlined()) {
> >>            Fn->addFnAttr(llvm::Attribute::InlineHint);
> >>            break;
> >>          }
> >>
> >> Here are the performance and size numbers I've collected:
> >>
> >>
> >> - C++ subset of Spec: No performance effects, < 0.1% size increase
> >> (all size numbers are text sizes returned by 'size')
> >> - Clang: 0.9% performance improvement (both -O0 and -O2 on a large .ii
> >> file) , 4.1% size increase
> >
> >
> > FWIW, this size increase seems *really* bad. I think that kills this
> > approach even in the short term.
> Re. size and performance trade-off -- 0.9% performance improvement
> should greatly win the size cost. Besides among all programs see, only
> clang sees this size increase with all the others seeing negligible
> size increase.

But the other programs don't see any performance wins.

We can likely get this 1% of runtime performance without paying that high
of a size cost. This is a real concern -- there are uses of Clang and LLVM
that are very sensitive to size regressions. And I suspect we'll see other
applications that also see the size loss.

If we could more *selectively* use a dedicated hint to get the performance
boost, we'd almost certainly not have to give up this much code size.

> >
> >>
> >> - Chrome: no performance improvement, 0.24% size increase
> >> - Google internal benchmark suite (geomean of ~20 benchmarks): ~1.8%
> >> performance improvement, no size regression
> >
> >
> > I'm also somewhat worried about the lack of any performance improvements
> > outside of the Google benchmarks. That somewhat strongly suggests that
> our
> > benchmarks are overly coupled to this hint already. The fact that neither
> > Chrome, Clang, nor SPEC improved is... not at all encouraging.
> Other than Google benchmarks, we do see Clang improve performance.
> Besides, current inliner needs to be further tuned in order to get
> more performance benefit. Passing the hint through is simply an
> enabler.  Also remember that most of SPEC benchmarks are C programs.
> C++ programs with heavy use of virtual functions may not benefit a lot
> either.

I don't really understand why that changes my point... Are you saying that
without this hint, we can't do the subsequent work on the inliner?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150709/608d05bc/attachment.html>

More information about the llvm-dev mailing list