Re: [PATCH 7/7] Plug ipa-prop escape analysis into gimple_call_arg_flags

[Jan Hubicka <hubicka at ucw dot cz>]

> Well, allocate_struct_function has a abstract_p argument for that.  But
> yes, a simple patch like
> 
> Index: gcc/function.c
> ===================================================================
> --- gcc/function.c      (revision 210845)
> +++ gcc/function.c      (working copy)
> @@ -64,6 +64,7 @@ along with GCC; see the file COPYING3.
>  #include "params.h"
>  #include "bb-reorder.h"
>  #include "shrink-wrap.h"
> +#include "cgraph.h"
> 
>  /* So we can assign to cfun in this file.  */
>  #undef cfun
> @@ -4512,6 +4513,8 @@ allocate_struct_function (tree fndecl, b
> 
>    if (fndecl != NULL_TREE)
>      {
> +      if (!abstract_p)
> +       cgraph_get_create_node (fndecl);
>        DECL_STRUCT_FUNCTION (fndecl) = cfun;
>        cfun->decl = fndecl;
>        current_function_funcdef_no = get_next_funcdef_no ();
> 
> ICEs during bootstrap with (at least)
> 
> /space/rguenther/src/svn/trunk/libgcc/config/i386/cpuinfo.c:405:1:
> error: node differs from symtab decl hashtable
>  }
>  ^
> __get_cpuid_max.constprop.0/42 (__get_cpuid_max.constprop) @0x7ff486232290
>   Type: function definition analyzed
>   Visibility: artificial
>   previous sharing asm name: 43
>   References:
>   Referring:
>   Function __get_cpuid_max.constprop/42 is inline copy in __get_cpuid_output/40
>   Availability: local
>   First run: 0
>   Function flags: local only_called_at_startup
>   Called by: __get_cpuid_output/40 (1.00 per call) (inlined)
>   Calls:
> /space/rguenther/src/svn/trunk/libgcc/config/i386/cpuinfo.c:405:1:
> internal compiler error: verify_cgraph_node failed
> 
> so I guess we would need to have a way to create a "dummy" cgraph
> node first and later populate it properly.

Yep, I was wondering about following:

struct GTY(()) tree_decl_with_vis {
 struct tree_decl_with_rtl common;
 tree assembler_name;
 tree section_name;
 tree comdat_group;

for majority of decl_with_vis we create we won't have section name/assembler
name and comdat group.

We already do memory optimization for INIT_PRIORITY in the following way:
#define DECL_HAS_INIT_PRIORITY_P(NODE) \
  (VAR_DECL_CHECK (NODE)->decl_with_vis.init_priority_p)
#define DECL_INIT_PRIORITY(NODE) \
  (decl_init_priority_lookup (NODE))

where init priority sits on separate hashtab.

I think at the moment we can't move assembler_name/section_name/comat_group
all to symbol table, because we do need the for off-symbol table things
(CONST_DECL, LABEL_DECL, off symtab VAR_DECL).
But I would suggest dropping them to off-tree hashtables, too, and have
pointer in symtab.
The accessor would be then something like

tree get_comdat_group (tree node)
{
  if (!node->decl_with_vis.comdat_group_p)
    return NULL;
  if (node->decl_with_vis.symtab_node)
    return node->decl_with_vis.symtab_node->comdat_group;
  decl_comdat_group_loop (node);
}
> 
> But as we currently have a back-pointer from struct function to fndecl
> it would be nice to hook the cgraph node in there - that way we get
> away without any extra pointer (we could even save symtab decl
> pointer and create a cyclic fndecl -> cgraph -> function -> fndecl
> chain ...).
> 
> I'm fine with enlarging tree_function_decl for now - ideally we'd push
> stuff from it elsewhere (like target and optimization option tree nodes,
> or most of the visibility and symbol related stuff).  Not sure why
> tree_type_decl inherits from tree_decl_non_common (and thus
> tree_decl_with_vis).  Probably because of the non-common parts
> being (ab-)used by FEs.  Otherwise I'd say simply put a symtab
> node pointer into tree_decl_with_vis ... (can we move
> section_name and comdat_group more easily than assembler_name?)

Lets see, I suppose putting it on side first is good incremental step.
Then we need to revisit frontends to avoid defining those too early,
that might be relatively simple to do (at least for C++ FE, I think it all
is defined at a time we call import/export decl.)

Honza
> 
> Richard.
> 
> > Honza
> >>
> >> Richard.
> >>
> >> >I think we may be on better track moving DECL_ASSEMBLER_NAME that is
> >> >calculated later,
> >> >but then we have problem with DECL_ASSEMBLER_NAME being set for
> >> >assembler names and
> >> >const decls, too that still go around symtab.
> >> >Given that decl_assembler_name is a function, I suppose we could go
> >> >with extra conditoinal
> >> >in there.
> >> >
> >> >Getting struct function out of frontend busyness would be nice indeed,
> >> >too, but probably
> >> >should be independent of Martin's work here.
> >> >
> >> >Honza
> >> >>
> >> >> Thanks,
> >> >> Richard.
> >> >>
> >> >> > Thanks,
> >> >> >
> >> >> > Martin
> >> >> >
> >> >> >>
> >> >> >> > +       }
> >> >> >> >      }
> >> >> >> > +
> >> >> >> > +  return ret;
> >> >> >> >  }
> >> >> >> >
> >> >> >> >  /* Detects return flags for the call STMT.  */
> >> >> >> >
> >>