This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Clean up early inliner

> > Note that there is also code to silence the sorry message for uninlined calls
> > to always_inlines that were previously indirect calls.  I guess if we go this
> > route, we need to check that kernel builds (or is going to be fixed for that)
> > as that is major consumer of this attribute as far as I can tell.
> > 
> > With externally visible always_inlines and without always_inline code in the
> > main inliner, we will end up sorrying on those with LTO.
> I think the idea was once the early inliner catches all cases we support
> by design, emit diagnostics for all remaining call edges to
> always-inline functions (basically, try to reclaim their bodies and
> complain if that's not possible).  Thus you'd never write their
> body to the LTO IL.

Yep, that would be OK for me.  I am little affraid of codebases that basically
define inline to be always_inline (as kernel does) but I guess those deserve
some errors and ought to be fixed if they contain nontrivial inlininig problems.

> > In practice LARGE_FUNCTION_GROW does not hit for early inlining except for
> > artifical testcases I would say (because EARLY_INLINING_INSNS is small and
> > thus the code growth is limited)
> Yeah, I guess the case I am worried about is as important as
> the case you are worrying about ;)  I just claim it shouldn't make
> any difference if in A -> B -> C we first inline C into B and then
> B into A or if we go and inline B into A first.

Yep, but it makes difference if all the inlining happens during early inlining
or if inlining B->C is deferred to late inliner because of the code in
topological sorter.  

In the second case A will get less early optimization so we would end up with
cases where making function "always_inline" instead of inline will magically 
lead to worse produced code even if inlining happens in both cases at the end.

> > Yep, that would work too (and give better solution for SCCs). But indeed such
> > clusterization would be more work.
> Maybe, I'm not sure ;)  The code as is is a little unobvious and
> undocumented ;)

Well, it is just reverse postorder generator.  I guess we can clean it up.
> > > too easy, so my patch makes it work reasonably (and I really doubt
> > > the situation that you have always-inline function calling a
> > > function that you still want to be inlined is common).
> > 
> > Yep, it is all nitpicking for sure ;)
> > I honestly have problem of deciding whether it is more common to have always
> > inlines in recursive callgraphs or always_inline functions that calls functions
> > that are candidates for normal inlining.  I guess both are relatively side
> > cases as the always_inline itself...
> Yes.  But if we want to be sure to handle all always-inlines we have
> to support during early inlining we have to get the cycle breaking 
> correct.  The main aim of the patch is to simplify the inliner code,
> untwisting incremental and IPA inlining modes.

Yep, I have no problem with cleaning this mess up ;)) Just since we run into
various interesting issues with inlines/always_inlines/flatenizing and cyclic
callgraphs I am bit paranoid.
> > Well, I guess we can go with the patch as it is and play with topological sorting
> > improvements incrementally.  It would also be nice to get the warnings at place so
> > we see how commonly indirect calls to always_inline and such appear.
> I'll see if I can implement the warnings - I have to look at diagnostic
> regression of the patch anyway it seems.

Well, if you make the patch to pass testing, consider it preaproved.  We can handle
all this stuff incrementally.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]