On Tue, 2017-01-17 at 13:35 +0100, Jan Hubicka wrote:
> > On Mon, Jan 16, 2017 at 10:25 PM, Jeff Law <l...@redhat.com> wrote:
> > > On 01/09/2017 07:38 PM, David Malcolm wrote:
> > > >
> > > > The RTL backend code is full of singleton state, so we have to
> > > > handle
> > > > functions as soon as we parse them. This requires various
> > > > special-casing
> > > > in the callgraph code.
> > > >
> > > > gcc/ChangeLog:
> > > > * cgraph.h (symtab_node::native_rtl_p): New decl.
> > > > * cgraphunit.c (symtab_node::native_rtl_p): New
> > > > function.
> > > > (symtab_node::needed_p): Don't assert for early
> > > > assembly output
> > > > for __RTL functions.
> > > > (cgraph_node::finalize_function): Set "force_output"
> > > > for __RTL
> > > > functions.
> > > > (cgraph_node::analyze): Bail out early for __RTL
> > > > functions.
> > > > (analyze_functions): Update assertion to support __RTL
> > > > functions.
> > > > (cgraph_node::expand): Bail out early for __RTL
> > > > functions.
> > > > * gimple-expr.c: Include "tree-pass.h".
> > > > (gimple_has_body_p): Return false for __RTL functions.
> > > > ---
> > > > gcc/cgraph.h | 4 ++++
> > > > gcc/cgraphunit.c | 41 ++++++++++++++++++++++++++++++++++++++-
> > > > --
> > > > gcc/gimple-expr.c | 3 ++-
> > > > 3 files changed, 44 insertions(+), 4 deletions(-)
> > > >
> > >
> > > > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> > > > index 81a3ae9..ed699e1 100644
> > > > --- a/gcc/cgraphunit.c
> > > > +++ b/gcc/cgraphunit.c
> > >
> > > @@ -568,6 +591,12 @@ cgraph_node::add_new_function (tree fndecl,
> > > bool
> > > lowered)
> > > >
> > > > void
> > > > cgraph_node::analyze (void)
> > > > {
> > > > + if (native_rtl_p ())
> > > > + {
> > > > + analyzed = true;
> > > > + return;
> > > > + }
> > >
> > > So my concern here would be how this interacts with the rest of
> > > the cgraph
> > > machinery. Essentially you're saying we've built all the
> > > properties for the
> > > given code. But AFAICT that can't be true and cgraph isn't
> > > actually aware
> > > of any of the properties of the native RTL code (even such things
> > > as what
> > > functions the native RTL code might call).
> > >
> > > So I guess my question is how do you ensure that even though
> > > cgraph hasn't
> > > looked at code that we're appropriately conservative with how the
> > > file is
> > > processed? Particularly if there's other code in the source file
> > > that is
> > > expected to interact with the RTL native code?
> >
> > I think that as we're finalizing the function from the FE before
> > the
> > cgraph is built
> > (and even throw away the RTL?) we have no other choice than
> > treating a __RTL
> > function as black box which means treat it as possibly calling all
> > function in
> > the TU and reading/writing/taking the address of all decls in the
> > TU. Consider
>
> I guess RTL frontend may be arranged to mark all such decls as used
> or just require
> user to do it, like we do with asm statements.
>
> I wonder why we need to insert those definitions into cgraph at first
> place...
They're added to the cgraph by this call:
/* Add to cgraph. */
cgraph_node::finalize_function (fndecl, false);
within function_reader::create_function (in r244110, though that code
isn't called yet; it's called by the stuff in patch 9).
If I hack out that call, so that __RTL functions aren't in the cgraph,
then I see lots of failures in the kit, for example here in predict.c:
maybe_hot_frequency_p (struct function *fun, int freq)
{
struct cgraph_node *node = cgraph_node::get (fun->decl);
[...read though node, so it must be non-NULL]
Similarly, this line in varasm.c's assemble_start_function assumes that
the fndecl has a symtab node:
align = symtab_node::get (decl)->definition_alignment ();
etc.
I don't know how many other places make the assumption that cfun's
fndecl has a node in the callgraph.
Given that I want to have __RTL functions called by non-__RTL functions
(and the patch kit handles this), it seemed saner to go down the route
of adding the decl to the callgraph.
> Honza
> >
> > static int i;
> > static void foo () {}
> > int __RTL main()
> > {
> > ... call foo, access i ...
> > }
> >
> > which probably will right now optimize i and foo away and thus fail
> > to link?
> > But I think we can sort out these "details" when we run into
> > them...
> >
> > Richard.
> >
> > > Jeff
