diff mbox

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

Message ID 20140521131634.646352575@virgil.suse.cz
State New
Headers show

Commit Message

Martin Jambor May 21, 2014, 1:16 p.m. UTC
Hi,

this demonstrates how results of ipa-prop escape analysis from
previous patches can be used at a later stage of compilation by
directly returning them from gimple_call_arg_flags which currently
relies on fnspec annotations.

Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
I have only had a brief look at behavior of this in SPEC 2006 and for
example in astar 1.19% of invocations of gimple_call_arg_flags return
noescape where we previously never did and in calculix this increases
from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
far less often still but for example in gamess that number raises from
5.21% to 7.66%.

Thanks,

Martin


2014-04-30  Martin Jambor  <mjambor@suse.cz>

	* gimple.c: Include cgraph.h.
	(gimple_call_arg_flags): Also query bitmaps in cgraph_node.

Comments

Richard Biener May 21, 2014, 2:27 p.m. UTC | #1
On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> this demonstrates how results of ipa-prop escape analysis from
> previous patches can be used at a later stage of compilation by
> directly returning them from gimple_call_arg_flags which currently
> relies on fnspec annotations.
>
> Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
> I have only had a brief look at behavior of this in SPEC 2006 and for
> example in astar 1.19% of invocations of gimple_call_arg_flags return
> noescape where we previously never did and in calculix this increases
> from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
> far less often still but for example in gamess that number raises from
> 5.21% to 7.66%.
>
> Thanks,
>
> Martin
>
>
> 2014-04-30  Martin Jambor  <mjambor@suse.cz>
>
>         * gimple.c: Include cgraph.h.
>         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
>
> Index: src/gcc/gimple.c
> ===================================================================
> --- src.orig/gcc/gimple.c
> +++ src/gcc/gimple.c
> @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
>  #include "demangle.h"
>  #include "langhooks.h"
>  #include "bitmap.h"
> -
> +#include "cgraph.h"
>
>  /* All the tuples have their operand vector (if present) at the very bottom
>     of the structure.  Therefore, the offset required to find the
> @@ -1349,32 +1349,50 @@ int
>  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
>  {
>    tree attr = gimple_call_fnspec (stmt);
> +  int ret;
>
> -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> -    return 0;
> -
> -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
>      {
> -    case 'x':
> -    case 'X':
> -      return EAF_UNUSED;
> -
> -    case 'R':
> -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'r':
> -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> -
> -    case 'W':
> -      return EAF_DIRECT | EAF_NOESCAPE;
> -
> -    case 'w':
> -      return EAF_NOESCAPE;
> +      switch (TREE_STRING_POINTER (attr)[1 + arg])
> +       {
> +       case 'x':
> +       case 'X':
> +         ret = EAF_UNUSED;
> +         break;
> +       case 'R':
> +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> +         break;
> +       case 'r':
> +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
> +         break;
> +       case 'W':
> +         ret = EAF_DIRECT | EAF_NOESCAPE;
> +         break;
> +       case 'w':
> +         ret = EAF_NOESCAPE;
> +         break;
> +       case '.':
> +       default:
> +         ret = 0;
> +       }
> +    }
> +  else
> +    ret = 0;
>
> -    case '.':
> -    default:
> -      return 0;
> +  tree callee_decl = gimple_call_fndecl (stmt);
> +  if (callee_decl)
> +    {
> +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
> +      if (callee_node)
> +       {
> +         if (cgraph_param_noescape_p (callee_node, arg))
> +           ret |= EAF_NOESCAPE;
> +         if (cgraph_param_noclobber_p (callee_node, arg))
> +           ret |= EAF_NOCLOBBER;

That's quite expensive.  I guess we need a better way to store
those?

> +       }
>      }
> +
> +  return ret;
>  }
>
>  /* Detects return flags for the call STMT.  */
>
Martin Jambor May 22, 2014, 12:49 p.m. UTC | #2
Hi,

On Wed, May 21, 2014 at 04:27:32PM +0200, Richard Biener wrote:
> On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > this demonstrates how results of ipa-prop escape analysis from
> > previous patches can be used at a later stage of compilation by
> > directly returning them from gimple_call_arg_flags which currently
> > relies on fnspec annotations.
> >
> > Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
> > I have only had a brief look at behavior of this in SPEC 2006 and for
> > example in astar 1.19% of invocations of gimple_call_arg_flags return
> > noescape where we previously never did and in calculix this increases
> > from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
> > far less often still but for example in gamess that number raises from
> > 5.21% to 7.66%.
> >
> > Thanks,
> >
> > Martin
> >
> >
> > 2014-04-30  Martin Jambor  <mjambor@suse.cz>
> >
> >         * gimple.c: Include cgraph.h.
> >         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
> >
> > Index: src/gcc/gimple.c
> > ===================================================================
> > --- src.orig/gcc/gimple.c
> > +++ src/gcc/gimple.c
> > @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
> >  #include "demangle.h"
> >  #include "langhooks.h"
> >  #include "bitmap.h"
> > -
> > +#include "cgraph.h"
> >
> >  /* All the tuples have their operand vector (if present) at the very bottom
> >     of the structure.  Therefore, the offset required to find the
> > @@ -1349,32 +1349,50 @@ int
> >  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
> >  {
> >    tree attr = gimple_call_fnspec (stmt);
> > +  int ret;
> >
> > -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> > -    return 0;
> > -
> > -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> > +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
> >      {
> > -    case 'x':
> > -    case 'X':
> > -      return EAF_UNUSED;
> > -
> > -    case 'R':
> > -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> > -
> > -    case 'r':
> > -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> > -
> > -    case 'W':
> > -      return EAF_DIRECT | EAF_NOESCAPE;
> > -
> > -    case 'w':
> > -      return EAF_NOESCAPE;
> > +      switch (TREE_STRING_POINTER (attr)[1 + arg])
> > +       {
> > +       case 'x':
> > +       case 'X':
> > +         ret = EAF_UNUSED;
> > +         break;
> > +       case 'R':
> > +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> > +         break;
> > +       case 'r':
> > +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
> > +         break;
> > +       case 'W':
> > +         ret = EAF_DIRECT | EAF_NOESCAPE;
> > +         break;
> > +       case 'w':
> > +         ret = EAF_NOESCAPE;
> > +         break;
> > +       case '.':
> > +       default:
> > +         ret = 0;
> > +       }
> > +    }
> > +  else
> > +    ret = 0;
> >
> > -    case '.':
> > -    default:
> > -      return 0;
> > +  tree callee_decl = gimple_call_fndecl (stmt);
> > +  if (callee_decl)
> > +    {
> > +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
> > +      if (callee_node)
> > +       {
> > +         if (cgraph_param_noescape_p (callee_node, arg))
> > +           ret |= EAF_NOESCAPE;
> > +         if (cgraph_param_noclobber_p (callee_node, arg))
> > +           ret |= EAF_NOCLOBBER;
> 
> That's quite expensive.  I guess we need a better way to store
> those?

if we want to avoid the cgraph_node lookup, then I think we need to
store this information in the decl or struct function.  That is
certainly possible and might even be more appropriate.

Thanks,

Martin

> 
> > +       }
> >      }
> > +
> > +  return ret;
> >  }
> >
> >  /* Detects return flags for the call STMT.  */
> >
Richard Biener May 22, 2014, 1:34 p.m. UTC | #3
On Thu, May 22, 2014 at 2:49 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Wed, May 21, 2014 at 04:27:32PM +0200, Richard Biener wrote:
>> On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
>> > Hi,
>> >
>> > this demonstrates how results of ipa-prop escape analysis from
>> > previous patches can be used at a later stage of compilation by
>> > directly returning them from gimple_call_arg_flags which currently
>> > relies on fnspec annotations.
>> >
>> > Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
>> > I have only had a brief look at behavior of this in SPEC 2006 and for
>> > example in astar 1.19% of invocations of gimple_call_arg_flags return
>> > noescape where we previously never did and in calculix this increases
>> > from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
>> > far less often still but for example in gamess that number raises from
>> > 5.21% to 7.66%.
>> >
>> > Thanks,
>> >
>> > Martin
>> >
>> >
>> > 2014-04-30  Martin Jambor  <mjambor@suse.cz>
>> >
>> >         * gimple.c: Include cgraph.h.
>> >         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
>> >
>> > Index: src/gcc/gimple.c
>> > ===================================================================
>> > --- src.orig/gcc/gimple.c
>> > +++ src/gcc/gimple.c
>> > @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
>> >  #include "demangle.h"
>> >  #include "langhooks.h"
>> >  #include "bitmap.h"
>> > -
>> > +#include "cgraph.h"
>> >
>> >  /* All the tuples have their operand vector (if present) at the very bottom
>> >     of the structure.  Therefore, the offset required to find the
>> > @@ -1349,32 +1349,50 @@ int
>> >  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
>> >  {
>> >    tree attr = gimple_call_fnspec (stmt);
>> > +  int ret;
>> >
>> > -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
>> > -    return 0;
>> > -
>> > -  switch (TREE_STRING_POINTER (attr)[1 + arg])
>> > +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
>> >      {
>> > -    case 'x':
>> > -    case 'X':
>> > -      return EAF_UNUSED;
>> > -
>> > -    case 'R':
>> > -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
>> > -
>> > -    case 'r':
>> > -      return EAF_NOCLOBBER | EAF_NOESCAPE;
>> > -
>> > -    case 'W':
>> > -      return EAF_DIRECT | EAF_NOESCAPE;
>> > -
>> > -    case 'w':
>> > -      return EAF_NOESCAPE;
>> > +      switch (TREE_STRING_POINTER (attr)[1 + arg])
>> > +       {
>> > +       case 'x':
>> > +       case 'X':
>> > +         ret = EAF_UNUSED;
>> > +         break;
>> > +       case 'R':
>> > +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
>> > +         break;
>> > +       case 'r':
>> > +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
>> > +         break;
>> > +       case 'W':
>> > +         ret = EAF_DIRECT | EAF_NOESCAPE;
>> > +         break;
>> > +       case 'w':
>> > +         ret = EAF_NOESCAPE;
>> > +         break;
>> > +       case '.':
>> > +       default:
>> > +         ret = 0;
>> > +       }
>> > +    }
>> > +  else
>> > +    ret = 0;
>> >
>> > -    case '.':
>> > -    default:
>> > -      return 0;
>> > +  tree callee_decl = gimple_call_fndecl (stmt);
>> > +  if (callee_decl)
>> > +    {
>> > +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
>> > +      if (callee_node)
>> > +       {
>> > +         if (cgraph_param_noescape_p (callee_node, arg))
>> > +           ret |= EAF_NOESCAPE;
>> > +         if (cgraph_param_noclobber_p (callee_node, arg))
>> > +           ret |= EAF_NOCLOBBER;
>>
>> That's quite expensive.  I guess we need a better way to store
>> those?
>
> if we want to avoid the cgraph_node lookup, then I think we need to
> store this information in the decl or struct function.  That is
> certainly possible and might even be more appropriate.

Can we?  If the body is not readily available we only have decl and
cgraph-node, not struct function.

I suppose we could exchange the struct function pointer in
tree_function_decl for a cgraph_node pointer and put
the struct function pointer into the cgraph_node.

Of course that may have impacts on FEs who might create
struct function before creating a cgraph node.  But at least
it would avoid enlarging FUNCTION_DECL.

In the end most of the tree_decl_with_vis stuff should move over to symtab
and var-decls should get a varpool_node pointer as well.

Back to the call flags stuff - I also meant the representation of the
"fn spec" attribute.  Rather than parsing that again and again move
it to a better place (which you seem to invent?) and better unified
representation.

Can you try if removing the cgraph hash is possible with the
struct function pointer idea?

Thanks,
Richard.

> Thanks,
>
> Martin
>
>>
>> > +       }
>> >      }
>> > +
>> > +  return ret;
>> >  }
>> >
>> >  /* Detects return flags for the call STMT.  */
>> >
Jan Hubicka May 22, 2014, 3:24 p.m. UTC | #4
> 
> Can we?  If the body is not readily available we only have decl and
> cgraph-node, not struct function.
> 
> I suppose we could exchange the struct function pointer in
> tree_function_decl for a cgraph_node pointer and put
> the struct function pointer into the cgraph_node.
> 
> Of course that may have impacts on FEs who might create
> struct function before creating a cgraph node.  But at least
> it would avoid enlarging FUNCTION_DECL.
> 
> In the end most of the tree_decl_with_vis stuff should move over to symtab
> and var-decls should get a varpool_node pointer as well.
> 
> Back to the call flags stuff - I also meant the representation of the
> "fn spec" attribute.  Rather than parsing that again and again move
> it to a better place (which you seem to invent?) and better unified
> representation.
> 
> Can you try if removing the cgraph hash is possible with the
> struct function pointer idea?

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).
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.  */
> >> >
Richard Biener May 22, 2014, 3:36 p.m. UTC | #5
On May 22, 2014 5:24:33 PM CEST, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 
>> Can we?  If the body is not readily available we only have decl and
>> cgraph-node, not struct function.
>> 
>> I suppose we could exchange the struct function pointer in
>> tree_function_decl for a cgraph_node pointer and put
>> the struct function pointer into the cgraph_node.
>> 
>> Of course that may have impacts on FEs who might create
>> struct function before creating a cgraph node.  But at least
>> it would avoid enlarging FUNCTION_DECL.
>> 
>> In the end most of the tree_decl_with_vis stuff should move over to
>symtab
>> and var-decls should get a varpool_node pointer as well.
>> 
>> Back to the call flags stuff - I also meant the representation of the
>> "fn spec" attribute.  Rather than parsing that again and again move
>> it to a better place (which you seem to invent?) and better unified
>> representation.
>> 
>> Can you try if removing the cgraph hash is possible with the
>> struct function pointer idea?
>
>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.

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.  */
>> >> >
Jan Hubicka May 22, 2014, 6:11 p.m. UTC | #6
> >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 :(

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.  */
> >> >> >
>
Jan Hubicka May 26, 2014, 1:01 a.m. UTC | #7
> On Thu, May 22, 2014 at 2:49 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > On Wed, May 21, 2014 at 04:27:32PM +0200, Richard Biener wrote:
> >> On Wed, May 21, 2014 at 3:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> >> > Hi,
> >> >
> >> > this demonstrates how results of ipa-prop escape analysis from
> >> > previous patches can be used at a later stage of compilation by
> >> > directly returning them from gimple_call_arg_flags which currently
> >> > relies on fnspec annotations.
> >> >
> >> > Bootstrapped and tested on x86_64-linux and also passes LTO bootstrap.
> >> > I have only had a brief look at behavior of this in SPEC 2006 and for
> >> > example in astar 1.19% of invocations of gimple_call_arg_flags return
> >> > noescape where we previously never did and in calculix this increases
> >> > from 15.62% (from annotations) to 18.14%.  Noclobber flag is reported
> >> > far less often still but for example in gamess that number raises from
> >> > 5.21% to 7.66%.
> >> >
> >> > Thanks,
> >> >
> >> > Martin
> >> >
> >> >
> >> > 2014-04-30  Martin Jambor  <mjambor@suse.cz>
> >> >
> >> >         * gimple.c: Include cgraph.h.
> >> >         (gimple_call_arg_flags): Also query bitmaps in cgraph_node.
> >> >
> >> > Index: src/gcc/gimple.c
> >> > ===================================================================
> >> > --- src.orig/gcc/gimple.c
> >> > +++ src/gcc/gimple.c
> >> > @@ -47,7 +47,7 @@ along with GCC; see the file COPYING3.
> >> >  #include "demangle.h"
> >> >  #include "langhooks.h"
> >> >  #include "bitmap.h"
> >> > -
> >> > +#include "cgraph.h"
> >> >
> >> >  /* All the tuples have their operand vector (if present) at the very bottom
> >> >     of the structure.  Therefore, the offset required to find the
> >> > @@ -1349,32 +1349,50 @@ int
> >> >  gimple_call_arg_flags (const_gimple stmt, unsigned arg)
> >> >  {
> >> >    tree attr = gimple_call_fnspec (stmt);
> >> > +  int ret;
> >> >
> >> > -  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
> >> > -    return 0;
> >> > -
> >> > -  switch (TREE_STRING_POINTER (attr)[1 + arg])
> >> > +  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
> >> >      {
> >> > -    case 'x':
> >> > -    case 'X':
> >> > -      return EAF_UNUSED;
> >> > -
> >> > -    case 'R':
> >> > -      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > -
> >> > -    case 'r':
> >> > -      return EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > -
> >> > -    case 'W':
> >> > -      return EAF_DIRECT | EAF_NOESCAPE;
> >> > -
> >> > -    case 'w':
> >> > -      return EAF_NOESCAPE;
> >> > +      switch (TREE_STRING_POINTER (attr)[1 + arg])
> >> > +       {
> >> > +       case 'x':
> >> > +       case 'X':
> >> > +         ret = EAF_UNUSED;
> >> > +         break;
> >> > +       case 'R':
> >> > +         ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > +         break;
> >> > +       case 'r':
> >> > +         ret = EAF_NOCLOBBER | EAF_NOESCAPE;
> >> > +         break;
> >> > +       case 'W':
> >> > +         ret = EAF_DIRECT | EAF_NOESCAPE;
> >> > +         break;
> >> > +       case 'w':
> >> > +         ret = EAF_NOESCAPE;
> >> > +         break;
> >> > +       case '.':
> >> > +       default:
> >> > +         ret = 0;
> >> > +       }
> >> > +    }
> >> > +  else
> >> > +    ret = 0;
> >> >
> >> > -    case '.':
> >> > -    default:
> >> > -      return 0;
> >> > +  tree callee_decl = gimple_call_fndecl (stmt);
> >> > +  if (callee_decl)
> >> > +    {
> >> > +      cgraph_node *callee_node = cgraph_get_node (callee_decl);
> >> > +      if (callee_node)
> >> > +       {
> >> > +         if (cgraph_param_noescape_p (callee_node, arg))
> >> > +           ret |= EAF_NOESCAPE;
> >> > +         if (cgraph_param_noclobber_p (callee_node, arg))
> >> > +           ret |= EAF_NOCLOBBER;
> >>
> >> That's quite expensive.  I guess we need a better way to store
> >> those?
> >
> > if we want to avoid the cgraph_node lookup, then I think we need to
> > store this information in the decl or struct function.  That is
> > certainly possible and might even be more appropriate.
> 
> Can we?  If the body is not readily available we only have decl and
> cgraph-node, not struct function.

Yep, indeed I think cgraph_node is good place to home this so it can
work with partitioning.   With the weekend changes to have direct pointer,
I guess the performance problems are gone.

My plan is to add a template that makes it easy to annotate symbol nodes
either by array (as we do by hand now in ipa-inline/ipa-prop and other places)
or by hashtable (for sparse data, like thunk information, or comdat information)
and move some of stuff we currently have in cgraph there
(rtl info, thunks, aliases, comdats, nested function tree can all go). For
performance critical stuff I have no problem adding it into cgraph nodes
themselves.

If someone wants to beat me and write GGC friendly template for this,
I would very welcome it.
> 
> I suppose we could exchange the struct function pointer in
> tree_function_decl for a cgraph_node pointer and put
> the struct function pointer into the cgraph_node.
> 
> Of course that may have impacts on FEs who might create
> struct function before creating a cgraph node.  But at least
> it would avoid enlarging FUNCTION_DECL.
> 
> In the end most of the tree_decl_with_vis stuff should move over to symtab
> and var-decls should get a varpool_node pointer as well.
> 
> Back to the call flags stuff - I also meant the representation of the
> "fn spec" attribute.  Rather than parsing that again and again move
> it to a better place (which you seem to invent?) and better unified
> representation.
> 
> Can you try if removing the cgraph hash is possible with the
> struct function pointer idea?

If there are no problems, I plan to move also DECL_SECTION and do
some cleanups to my weekend change (in particular I do not like the
way it works with duplicate_decl. Perhaps we don't really need to
duplicate this info).
We can experiment with assembler name and struct function next.

Honza
> 
> Thanks,
> Richard.
> 
> > Thanks,
> >
> > Martin
> >
> >>
> >> > +       }
> >> >      }
> >> > +
> >> > +  return ret;
> >> >  }
> >> >
> >> >  /* Detects return flags for the call STMT.  */
> >> >
diff mbox

Patch

Index: src/gcc/gimple.c
===================================================================
--- src.orig/gcc/gimple.c
+++ src/gcc/gimple.c
@@ -47,7 +47,7 @@  along with GCC; see the file COPYING3.
 #include "demangle.h"
 #include "langhooks.h"
 #include "bitmap.h"
-
+#include "cgraph.h"
 
 /* All the tuples have their operand vector (if present) at the very bottom
    of the structure.  Therefore, the offset required to find the
@@ -1349,32 +1349,50 @@  int
 gimple_call_arg_flags (const_gimple stmt, unsigned arg)
 {
   tree attr = gimple_call_fnspec (stmt);
+  int ret;
 
-  if (!attr || 1 + arg >= (unsigned) TREE_STRING_LENGTH (attr))
-    return 0;
-
-  switch (TREE_STRING_POINTER (attr)[1 + arg])
+  if (attr && 1 + arg < (unsigned) TREE_STRING_LENGTH (attr))
     {
-    case 'x':
-    case 'X':
-      return EAF_UNUSED;
-
-    case 'R':
-      return EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
-
-    case 'r':
-      return EAF_NOCLOBBER | EAF_NOESCAPE;
-
-    case 'W':
-      return EAF_DIRECT | EAF_NOESCAPE;
-
-    case 'w':
-      return EAF_NOESCAPE;
+      switch (TREE_STRING_POINTER (attr)[1 + arg])
+	{
+	case 'x':
+	case 'X':
+	  ret = EAF_UNUSED;
+	  break;
+	case 'R':
+	  ret = EAF_DIRECT | EAF_NOCLOBBER | EAF_NOESCAPE;
+	  break;
+	case 'r':
+	  ret = EAF_NOCLOBBER | EAF_NOESCAPE;
+	  break;
+	case 'W':
+	  ret = EAF_DIRECT | EAF_NOESCAPE;
+	  break;
+	case 'w':
+	  ret = EAF_NOESCAPE;
+	  break;
+	case '.':
+	default:
+	  ret = 0;
+	}
+    }
+  else
+    ret = 0;
 
-    case '.':
-    default:
-      return 0;
+  tree callee_decl = gimple_call_fndecl (stmt);
+  if (callee_decl)
+    {
+      cgraph_node *callee_node = cgraph_get_node (callee_decl);
+      if (callee_node)
+	{
+	  if (cgraph_param_noescape_p (callee_node, arg))
+	    ret |= EAF_NOESCAPE;
+	  if (cgraph_param_noclobber_p (callee_node, arg))
+	    ret |= EAF_NOCLOBBER;
+	}
     }
+
+  return ret;
 }
 
 /* Detects return flags for the call STMT.  */