> Evidently not, but I'm also not sure that 'unanimous consensus' is an
> appropriate standard.
>
>> and -- most embarrassingly -- seems to
>> completely miss out the *actual* semantic impact of inline
>> functions. [Chandler, we should get that FAQ entry fixed.]
>
> The problem is that it is not just *that* FAQ, Google also ranks this one
> pretty highly:
>
>
> http://www.cprogramming.com/tips/tip/member-functions-are-inline-by-default-cincrement
>
> The same is true in plenty of books too. For example, from the first book
> Google book search finds for "inline member function": "Object-oriented
> Programming: Using C++ for Engineering and Technology" by Goran Svenk (2003),
> says, "To create an inline member function, it is only necessary to place the
> function's definition inside the class. It is not necessary to use the inline
> keyword in this case, as is the case with non-member functions. Some
> functions -- long functions, recursive functions, ... -- cannot be expanded
> inline. If a member function falls into one of these categories and its
> definition is placed within the body of the class, the compiler will usually
> issue a warning message and process the function as a non-inline member
> function."
>
> I don't particularly like this explanation, and the last statement may not be
> entirely correct, but the point relevant to our discussion is clear. There is
> a significant portion of the skilled C++ programming community that shares
> the view for which I'm advocating. And to be clear, I'm advocating this
> viewpoint because I believe it is the prevailing viewpoint among C++
> programmers. The fact that these FAQs, books, etc. agree with me I simply
> take as evidence that it is the prevailing viewpoint.
>
yes -- developers (except for compiler developers) won't know the
differences between 'inline functions' vs 'inline specified
functions', not to mention the exact intention of the standard is
subject to debate.
David
> As I've said, I don't particularly like the current state of affairs -- it is
> particularly problematic for the creation of header-only libraries: If you
> wish to create a header-only library, and you follow reasonable coding
> practices, the functions that you define in the class definitions are likely
> to be small and the functions you define outside the class definitions are
> likely to be larger. This, I think, is the most readable approach. However,
> even if you don't put the inline keyword on the in-class definitions, you do
> need to put it explicitly on the (larger) out-of-class definitions to avoid
> ODR violation. This is exactly the opposite of what you want: those larger
> functions are precisely the ones likely to be less suitable for inlining.
>
> However, if we're trying to fix that problem by not treating in-class
> functions definitions the same way we treat other functions with the inline
> keyword, we're missing most of the problem, and creating a quirky
> implementation in the process. The real problem is large functions that need
> the inline keyword to avoid ODR violation. But for that, we need a separate
> solution.
>
>>
>>
>> And I think you're still missing the point: whether we agree that the
>> standard says there's an inline hint here is not even relevant, in
>> and of itself. We should base our inlining on whatever gives the
>> best results (by whatever criteria we judge those results); the
>> standard gives us no normative requirements in this regard.
>
> I agree, but the standard does set a user expectation in this regard. And we
> should have a really good reason to violate that expectation, and we don't.
> The data seems to suggest that we're currently not doing the optimal thing,
> and that aligning our inlining heuristic with that expectation yields better
> performance results. That having been said, the fact that this expectation
> comes directly from the standard, gives it significantly more weight than it
> might have otherwise. That's my point: I'm not saying that our current
> implementation is not conforming, but I'm saying that it is providing a
> sub-optimal user experience by not satisfying the expectations that users
> have in this regard; expectations that come directly from the standard.
>
> Thanks again,
> Hal
>
>>
>>
>> What’s the deal with inline functions: When the compiler
>> inline-expands a function call, the function’s code gets inserted...
>> There are several ways to designate that a function is inline, some
>> of which involve the inline keyword, others do not. No matter how
>> you designate a function as inline, it is a request that ...
>>
>> Is there another way to tell the compiler to make a member function
>> inline?: Yep: define the member function in the class body itself:
>> ... This is often more convenient than the alternative of defining
>> your inline functions outside the class body. However, although it
>> is easier on the person who writes the class, it is harder on all
>> the readers since it mixes what a class does (the external behavior)
>> with how it does it ...
>>
>> -Hal
>>
>>
>>
>>
>> > the terms are "declared with an inline specifier" and
>> > > "inline function"
>> > >
>> >
>> > To be pedantic, "declared with an inline specifier" does not seem
>> > to
>> > occur in the standard. The closest thing I see is 7.1.2p2, "A
>> > function declaration (8.3.5, 9.3, 11.3) with an inline specifier
>> > declares an inline function.", and that *defines* the term "inline
>> > function".
>> >
>> > >
>> > > In any case, the point is: it is not reasonable to use the
>> > > standard's
>> > > wording to justify when to provide an inline hint.
>> >
>> > It is perfectly reasonable; the standard contains a facility
>> > designed
>> > explicitly for this purpose, and it happens to be the facility
>> > under
>> > discussion.
>> >
>> > > Even if we agreed
>> > > that it said that any inline function is intended to undergo
>> > > inline
>> > > substitution, that is only a non-binding suggestion due to the
>> > > as-if
>> > > rule, and we are under no obligation to base our inlining
>> > > decision
>> > > on it.
>> > >
>> >
>> > Agreed.
>> >
>> > >
>> > >
>> > >
>> > > > Developers see those and rely on those to give compiler the
>> > > > hints.
>> > >
>> > >
>> > >
>> > > Likewise here, different developers have different expectations,
>> > > so
>> > > I
>> > > don't think we can use this argument to make the decision.
>> > >
>> > >
>> > > There seem to be two relevant factors that should affect our
>> > > decision:
>> > >
>> > >
>> > > 1) What signals best indicate that inlining is beneficial for
>> > > existing code? That is, which heuristics make us optimize better?
>> > > and
>> >
>> > It is not quite that simple. We also need to give a high-quality
>> > user
>> > experience, and that means taking advantage of language facilities
>> > designed to provide optimization hints in a consistent way. Now we
>> > can debate what consistent is... ;)
>> >
>> > > 2) Which signals are reasonable for us to use, given the current
>> > > and
>> > > expected future state of the C++ language? We don't want people
>> > > contorting their code in order to get it well-optimized (such as
>> > > moving functions out of their class definition because they turn
>> > > out
>> > > to be somewhat non-trivial, and inlining them is harmful),
>> > > and we
>> > > want to allow inline hints to be given in ways that are
>> > > orthogonal
>> > > to program semantics (the inline specifier is not good for this,
>> > > because it also carries real language semantics, not just an
>> > > optimization hint).
>> > >
>> >
>> > I agree this is an unfortunate combination of properties for
>> > 'inline'
>> > to have. But it was not done by accident, and the intent of the
>> > language seems to be not to give us the option of treating them
>> > separately and still provide a consistent user experience. We
>> > certainly *can* and still have a conforming implementation... we
>> > could also only consider the first 100 inline specifiers for
>> > hinting
>> > and ignore the rest. But, in my experience, users have expectations
>> > that inline functions are (at least more likely) to be inlined than
>> > other functions of similar size, and this includes functions
>> > defined
>> > in the class definition. Furthermore, the data seems to indicate
>> > that this is the right approach.
>> >
>> > I also don't want users to burden their code with the 'inline'
>> > keyword on nearly every (or every) in-class function definition
>> > because, even though defining it there is supposed to make it an
>> > inline function, that will make it a 'really inline' inline
>> > function.
>> >
>> > Thanks again,
>> > Hal
>> >
>> > >
>> > >
>> > >
>> > >
>> > > > Most importantly, paragraph 3 says:
>> > > >
>> > > > "A function defined within a class definition is an inline
>> > > > function.
>> > > >
>> > > >
>> > > >
>> > > > ... the same is *not* true for a function definition that
>> > > > appears
>> > > > within a class definition. That is merely an inline function
>> > > > (that
>> > > > is, it can be defined in multiple translation units).
>> > > >
>> > > >
>> > > > 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.
>> > > >
>> > > > > 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.
>> > > >
>> > > > >
>> > > > > 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.
>> > > >
>> > > > >
>> > > > > That doesn't mean that the core concept of hinting to the
>> > > > > optimizer
>> > > > > that a
>> > > > > particular function is a particularly good candidate for
>> > > > > inlining
>> > > > > is without
>> > > > > value.
>> > > >
>> > > > yes.
>> > > >
>> > > > >While I think it is a bad practice that we shouldn't encourage
>> > > > >in
>> > > > > code (especially portable code)
>> > > >
>> > > > yes -- there are indeed programmers who use this casually
>> > > > without
>> > > > considering performance.
>> > > >
>> > > > > I can see the desire to at least have *some*
>> > > > > attribute which is nothing more or less than a hint to the
>> > > > > optimizer to
>> > > > > inline harder[1].
>> > > >
>> > > > yes -- there are programmers who use the attribute consciously.
>> > > >
>> > > > > It would help people work around inliner bugs in the short
>> > > > > term, and even help debug inliner-rooted optimization
>> > > > > problems.
>> > > >
>> > > > I think it is a good hint to the compiler even in the longer
>> > > > term.
>> > > > With PGO, we should minimize the reliance on the hint though.
>> > > >
>> > > > >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?
>> > > >
>> > > > > 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.
>> > > >
>> > > > >
>> > > > > [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.
>> > > >
>> > > > This is not a short term vs long term situation. It is
>> > > > basically
>> > > > a
>> > > > bug
>> > > > fix that FE drops the attribute. If it exposes inliner
>> > > > heuristic
>> > > > bug,
>> > > > that should be fixed/tuned separately. With the hint correctly
>> > > > passed
>> > > > in, Easwaran will do further tuning including time based
>> > > > analysis.
>> > > >
>> > > > >
>> > > > >>
>> > > > >> - 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.
>> > > >
>> > > > David
>> > > >
>> > > >
>> > > > >
>> > > > > -Chandler
>> > > > _______________________________________________
>> > > > cfe-commits mailing list
>> > > > cfe-commits@cs.uiuc.edu
>> > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> > > >
>> > > >
>> > > > _______________________________________________
>> > > > cfe-commits mailing list
>> > > > cfe-commits@cs.uiuc.edu
>> > > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Hal Finkel
>> > > Assistant Computational Scientist
>> > > Leadership Computing Facility
>> > > Argonne National Laboratory
>> > >
>> > >
>> >
>> > --
>> > Hal Finkel
>> > Assistant Computational Scientist
>> > Leadership Computing Facility
>> > Argonne National Laboratory
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits@cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >
>>
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>>
>>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
