On Aug 26, 2009, at 12:01 PM, Devang Patel wrote:>>> I do not understand how the "inlinehint" will help. How will it >>> influence the inliner ? >> >> The hint should make it more attractive to inline. I don't know >> the details >> yet and they will require some experimenting. >> > > In that case you want to add hint to A also. AFAIU, attractiveness of > A and B should match and inliner won't know whether the function body > is inside the class definition or not.For background it is useful to know that a lot of this discussion is based on the assumption that the inliner will be making the wrong decisions. Dale comes at this from the premise that the inliner will always make the wrong decision and therefore it is useful to give the user some way to influence the inliners heuristic (other than attr(noinline/alwaysinline)). There are three questions to consider: 1. whether a function is "semantically inline" should make it more attractive to inline. 2. whether a function that is "syntactically inline" should make it more attractive to inline. 3. whether neither should matter and the inliner should just look at the structure and size of the code, ignoring both syntactic and semantic inline hints (But still listening to always and noinline attributes of course). To me personally, I have a hard time believing that either hint is really trustworthy and would rather invest effort into seeing how far we can get with #3. My reason for thinking this is twofold: first, many people slap the inline keyword on stuff without really thinking about how big the code will be. In the case of C++, because of abstraction and other things, the code may be compiled to something much larger than the source looks. The second part of this is that there are a lot of reasons for things to be defined inline in C++ even if we don't want it to actually be inlined. For example, lib/VMCore/ConstantsContext.h:349 has a massive inline function that is there because it is part of a template specialization and *it is more convenient* to define it inline. I really don't think that method should be inlined, but it makes the source code tidier to do that. There are several other "good" examples in that file as well. Since templates all have to be defined in header files for them to be instantiated, we frequently get large functions defined inline even though they shouldn't. One trivial example is SmallVector::insert(iterator,size_t, T). I know/hope that the proposal isn't for the inlinehint to be a synonym for "force inline", it would just raise the threshold to increase the likeliness that it would be inlined. The question is whether "something being c++ inline" in any way is really trustworthy, and if so, whether we should look at syntactic vs semantic inline. -Chris
On Aug 26, 2009, at 1:09 PM, Chris Lattner wrote:> . Dale comes at this from the premise that the inliner will > always make the wrong decision and therefore it is useful to give the > user some way to influence the inliners heuristic (other than > attr(noinline/alwaysinline)).Sorry "will always make the wrong decision *in some cases* and therefore..." -Chris
On Aug 26, 2009, at 4:09 PM, Chris Lattner wrote: [...]> > The second part of this is that there are a lot of reasons for things > to be defined inline in C++ even if we don't want it to actually be > inlined.I don't think those are _good_ reasons though: If one doesn't want a C+ + function to be inlined, one shouldn't define it inline.> For example, lib/VMCore/ConstantsContext.h:349 has a > massive inline function that is there because it is part of a template > specialization and *it is more convenient* to define it inline. I > really don't think that method should be inlined, but it makes the > source code tidier to do that. There are several other "good" > examples in that file as well. > > Since templates all have to be defined in header files for them to be > instantiated, we frequently get large functions defined inline even > though they shouldn't. One trivial example is > SmallVector::insert(iterator,size_t, T).I think that's the same issue as above: If you don't think it's a good candidate for inlining, it shouldn't be defined inline.> I know/hope that the proposal isn't for the inlinehint to be a synonym > for "force inline", it would just raise the threshold to increase the > likeliness that it would be inlined. The question is whether > "something being c++ inline" in any way is really trustworthy, and if > so, whether we should look at syntactic vs semantic inline.FWIW, I've been involved in a couple of attempts by commercial compilers to relegate "inline" to the same status as "register" -- an obsolete hint ignored by the compiler -- and so far that always proved to be unpractical because some critical calls that were previously inlined were no longer being inlined after the change. (That's just annecdotal, of course: LLVM may have gotten good enough to make it practical. If that's the case, I still think it's too early to write C ++ code with that assumption.) Daveed
On Wed, Aug 26, 2009 at 1:09 PM, Chris Lattner<clattner at apple.com> wrote:> I know/hope that the proposal isn't for the inlinehint to be a synonym for > "force inline", it would just raise the threshold to increase the likeliness > that it would be inlined.One alternative is to give the functions with hint first chance but not change the threshold. Inliner works in two phase. In first phase, only use function with inlinehint as candidates.>The question is whether "something being c++ > inline" in any way is really trustworthy, and if so, whether we should look > at syntactic vs semantic inline.- Devang
On Wed, Aug 26, 2009 at 2:50 PM, Devang Patel<devang.patel at gmail.com> wrote:> On Wed, Aug 26, 2009 at 1:09 PM, Chris Lattner<clattner at apple.com> wrote: > >> I know/hope that the proposal isn't for the inlinehint to be a synonym for >> "force inline", it would just raise the threshold to increase the likeliness >> that it would be inlined. > > One alternative is to give the functions with hint first chance but > not change the threshold. Inliner works in two phase. In first phase, > only use function with inlinehint as candidates.Truthfully, I think we need a thorough discussion our current inlining heuristics before we try to consider what inlinehint adds to the picture. The inlining code has been mostly untouched for a very long time. Some limitations I can think of off the top of my head: Inlining doesn't take into account whether a load from a constant pointer is constant. Inlining is purely bottom-up: we inline large functions into small functions when it would be better to inline the small function. The cost metrics for constant-propagating into a branch or switch are extremely imprecise. Cost metrics can easily end up being negative (i.e. we always inline) in cases where inlining will lead to arbitrarily large increases in code size. We only use one inlining pass normally; IIRC, the gcc devs found that an additional early inlining pass is beneficial. There are probably other issues which aren't obvious to me because I'm not an expert in this sort of thing... -Eli
On Aug 26, 2009, at 2:31 PM, David Vandevoorde wrote:> > >> I know/hope that the proposal isn't for the inlinehint to be a >> synonym >> for "force inline", it would just raise the threshold to increase the >> likeliness that it would be inlined. The question is whether >> "something being c++ inline" in any way is really trustworthy, and if >> so, whether we should look at syntactic vs semantic inline. > > > FWIW, I've been involved in a couple of attempts by commercial > compilers to relegate "inline" to the same status as "register" -- an > obsolete hint ignored by the compiler -- and so far that always proved > to be unpractical because some critical calls that were previously > inlined were no longer being inlined after the change. (That's just > annecdotal, of course: LLVM may have gotten good enough to make it > practical. If that's the case, I still think it's too early to > write C > ++ code with that assumption.)It's actually the other way around. llvm has always ignored the "inline" keyword and now we are finding out we are missing some important cases. Obviously the only correct solution is to make the inliner smarter so it can identify cases which are profitable that's currently ignoring. No one is arguing against that. On the other hand, that is most definitely not a simple solution. So we need an intermediate step. The current plan is to make it behave a bit more like gcc (yeah I know it's not exactly where we want to go) so the "inline" keyword only impacts -O2 compilation. We should make conservative changes and then re-evaluate once we have more data. That means people who use llvm to compile C++ programs should definitely report any kind of performance changes once this change goes into effect. Evan> > Daveed > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
David Vandevoorde a écrit :> > I don't think those are _good_ reasons though: If one doesn't want a C+ > + function to be inlined, one shouldn't define it inline. > >You must not have written a lot of C++ template then. You don't have the choice in this case, just check your STL header.> > FWIW, I've been involved in a couple of attempts by commercial > compilers to relegate "inline" to the same status as "register" -- an > obsolete hint ignored by the compiler -- and so far that always proved > to be unpractical because some critical calls that were previously > inlined were no longer being inlined after the change. (That's just > annecdotal, of course: LLVM may have gotten good enough to make it > practical. If that's the case, I still think it's too early to write C > ++ code with that assumption.) >There is often a keyword force_inline or alwais_inline if needed Cédric
David Vandevoorde wrote:> FWIW, I've been involved in a couple of attempts by commercial > compilers to relegate "inline" to the same status as "register" -- an > obsolete hint ignored by the compiler -- and so far that always proved > to be unpractical because some critical calls that were previously > inlined were no longer being inlined after the change. (That's just > annecdotal, of course: LLVM may have gotten good enough to make it > practical. If that's the case, I still think it's too early to write C > ++ code with that assumption.)If it's critical that a function be inlined, what's wrong with applying the always-inline attribute? Nick