diff mbox

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

Message ID CAFiYyc0Oq8DgXZBxiBQR=xJu=Fg8y56u3GS0oMBfTfQ2Uy1LyA@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener May 23, 2014, 10:03 a.m. UTC
On Thu, May 22, 2014 at 8:11 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >It won't be so easy, because struct function is really built at
>> >relatively convoluted
>> >places within frontend before cgraph node is assigned to them (I tried
>> >that few years
>> >back).
>>
>> Well, just call cgraph create node from struct Funktion allocation.
>
> That will make uninstantiated templates to land symbol table (and if you have
> aliases, also do the assembler name mangling) that is not that cool either :(

Well, allocate_struct_function has a abstract_p argument for that.  But
yes, a simple patch like


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.

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?)

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.  */
>> >> >> >
>>

Comments

Jan Hubicka May 23, 2014, 10:29 p.m. UTC | #1
> 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.  */
> >> >> >> >
> >>
diff mbox

Patch

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 ();