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 9c] callgraph: handle __RTL functions


On Tue, 2017-01-17 at 13:35 +0100, Jan Hubicka wrote:
> > On Mon, Jan 16, 2017 at 10:25 PM, Jeff Law <law@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


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