This is the mail archive of the gcc-patches@gcc.gnu.org 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


> I don't think we are losing a feature that worked reliably.  You still
> can be lucky with the patch, though.  In fact I think we should warn
> whenever the address of an always-inline function is taken.

For the case of always inline callback of always inline function, this
shouhd've worked reliably. I can imagine this would even make sense for
stuff like for_each_rtx if that was not recursive and cheap enough.  I think
Martin added a testcase to testsuite for this too, but I guess it is now
the lucky one.

Another case where we can now fail is when always inline is passed as callback:
__attribute__ ((always_inline)) int a(void)
{
}

int
t(int (*a)(void))
{
  return a();
}

with some extra effort needed to make topological sorter to put t before a.

But we can indeed declare those just invalid and warn (as well as warn for always
inline externally visible functions that are sort of insane too).


> > I am leaving soon, so give me some time to thing about the cyclic graphs.
> > I am not completely happy about the fact that topological sorter now ignore
> > all callers of always inline functions, at least that it does so unconditionally.
> > It will lead to some pesimization of those.
> > (i.e. if you have A that calls always inline B that calls C, topological sorter
> > will no longer make C early optimized before A that will result in early inliner
> > not inlining the whole chaing A->B->C.  This is probably something you don't want
> > when you care about performance enough to play with always inlines)
> 
> Well.  With that situation you absolutely want to inline B into all
> callers first (otherwise you effectively make inline copy of C in B
> always-inline, disregarding any costs), only then consider C.  Thus,

Not really. Early inliner is basically inlining for size (modulo the
early_insns fuzz) so it would end up inlining C into all functions wher B was
inlined if it was performed again.

(if we are serious about early_insns fuzz, I guess early inliner ought
to then ignore all always_inline functions and handle inlining of C by
iteration after deciding to inline B into A).

> the sorting we really want is unordered (B, C), A.  Note that the
> breaking of cycles at the right points is required to do a sane
> and complete job with always-inlines.

Yep, one needs to break cycles, but since most of callgraph are acyclic, it is
IMO bad idea to pesimize those.  We already have code to find strongly
connected components, so we can probably ignore the callees only for always
inline functions participating in those.

Honza


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