This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Clean up early inliner
On Wed, 7 Apr 2010, Jan Hubicka wrote:
> >
> > This cleans up the early inliner, simplifying it and making it the
> > one that resolves all always-inline inlines. It also should remove
> > the need to run the IPA inliner when not optimizing or not inlining.
> >
> > The single early inlining cleanup that is left is the interaction
> > with profiling.
> >
> > Boostrap and regtest running on x86_64-unknown-linux-gnu, ok for
> > trunk?
>
> Are we going to set some politics concerning always inline and indirect calls?
> I.e. early inliner can handle with some cases of indirect calls. Code you
> remove in IPA inliner is able to handle always inline functions having callbacks
> that are always inline. I am not convinced we need to make any promises here,
> but we are losing some "feature".
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.
> 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,
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.
> > *************** cgraph_decide_inlining_of_small_function
> > *** 1127,1132 ****
> > --- 1124,1211 ----
> > BITMAP_FREE (updated_nodes);
> > }
> >
> > + /* Flatten NODE from the IPA inliner. */
> > +
> > + static void
> > + cgraph_flatten (struct cgraph_node *node)
>
> When the flattening code is broken out, I guess early inliner could do the job too.
> That will result in early optimizations having more chance to further simplify the
> code. But I don't care much (how many uses this found except for tramp3d?)
I seriously don't know.
> > + {
> > + struct cgraph_edge *e;
> > + void *old_mode;
> > +
> > + old_mode = node->aux;
> > +
> > + node->aux = (void *)(size_t) INLINE_ALL;
> > +
> > + for (e = node->callees; e; e = e->next_callee)
> > + {
> > + struct cgraph_node *callee = e->callee;
> > + void *callee_mode = callee->aux;
> > +
> > + if (e->call_stmt_cannot_inline_p)
> > + continue;
> > +
> > + /* We've hit cycle? It is time to give up. */
> > + if (callee_mode)
> > + {
> > + if (dump_file)
> > + fprintf (dump_file,
> > + "Not inlining %s into %s to avoid cycle.\n",
> > + cgraph_node_name (callee),
> > + cgraph_node_name (e->caller));
> > + e->inline_failed = (e->callee->local.disregard_inline_limits
> > + ? CIF_RECURSIVE_INLINING : CIF_UNSPECIFIED);
> > + continue;
> > + }
> > +
> > + /* When the edge is already inlined, we just need to recurse into
> > + it in order to fully flatten the leaves. */
> > + if (!e->inline_failed)
> > + {
> > + callee->aux = (void *)(size_t) INLINE_ALL;
> > + cgraph_flatten (e->callee);
> > + callee->aux = callee_mode;
> > + continue;
> > + }
> > +
> > + if (cgraph_recursive_inlining_p (node, e->callee, &e->inline_failed))
> > + {
> > + if (dump_file)
> > + fprintf (dump_file, "Not inlining: recursive call.\n");
> > + continue;
> > + }
> > + if (!tree_can_inline_p (e))
> > + {
> > + if (dump_file)
> > + fprintf (dump_file, "Not inlining: %s",
> > + cgraph_inline_failed_string (e->inline_failed));
> > + continue;
> > + }
> > + if (!e->callee->analyzed)
> > + {
> > + if (dump_file)
> > + fprintf (dump_file,
> > + "Not inlining: Function body no longer available.\n");
> > + continue;
> > + }
>
> Funcition bodies should not disappear anymore. I guess this is first test
> that checks if function body is around, so I guess you should put it first for
> effectivity and change working to "Function body not available".
Fixed.
> > + /* When to run IPA inlining. Inlining of always-inline functions
> > + happens during early inlining. */
> > +
> > + static bool
> > + gate_cgraph_decide_inlining (void)
> > + {
> > + /* We'd like to skip this if not optimizing or not inlining as
> > + all always-inline functions have been processed by early
> > + inlining already. But this breaks EH with C++ somehow with
> > +
> > + g++.dg/torture/pr31863.C: In destructor 'Serializer<unsigned int, Loki::Typelist<ClassSpec<unsigned int, A040, 40u>, Loki::NullType> >::~Serializer()':
> > + g++.dg/torture/pr31863.C:231:7: error: statement marked for throw, but doesn't
> > + Serializer<unsigned int, ClassSpec<unsigned int, A040, 40u> >::~Serializer (this.18352_8, D.118411_7);
> > +
> > + so leave it on unconditionally for now. */
>
> This is because inliner calls the fixup_cfg pass that is needed for other
> reasons too. I guess we can just make new IPA pass that will have only
> transform method that will call the fixups scheduled after IPA_INLINER.
> (that is probablyy cleaner too)
I see.
> Thanks for working on this :)
Richard.