diff mbox

[RFC/RFA] Const variable initializer folding

Message ID 20100831230839.GI1664@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Aug. 31, 2010, 11:08 p.m. UTC
Hi.
at various places we need to fold const static variable via knowledge of its
initializer.  The precise conditions when we do so hoever vary. For example

const int a=5;
const int b[2]={0,1};
main()
{
  return a+b[0];
}

compiled with -O2 -fpic gets a folded to 5, but b[0] is not folded to 0.

In order to fix darwin WHOPR partitioning I need variables with extern linkage
but known initializer.  Even this is supported for scalars, but not for arrays,
see i.e. g++.dg/opt/static3.C but not for arrays or structures.

I am quite convinced that with WHOPR partitioning, we can't derive the fact
from current DECL flags. 

static const int a;

Allows folding referenced of A to 0. However with partitioning it might become:
__attribute__ ((visibility ("hidden"))) extern const int a;
... and we would lose knowledge that A is 0.

For this reason patch adds const_value_known into varpool structure and makes
existing folders to use it.  Note that there is other place folding initialiers
in expand.c I would like to remove (tree optimizers can do the same) but that
would need some extra work.

There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
because fortran adds into varpool variable that is !TREE_STATIC &&
!DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
this area of FE to know what it is supposed to do.

Does this seem to make sense?  Patch was bootstrapped/regtested x86_64-linux and
darwin.

Honza

	* cgraph.h (struct varpool_node): Add const_value_known.
	(varpool_decide_const_value_known): Declare.
	* tree-ssa-ccp.c (fold_const_aggregate_ref): Update initializer folding.
	* lto-cgraph.c (lto_output_varpool_node): Store const_value_known.
	(input_varpool_node): Restore const_value_known.
	* tree-ssa-loop-ivcanon (constant_after_peeling): Check varpool for
	initializer folding.
	* ipa.c (ipa_discover_readonly_nonaddressable_var,
	function_and_variable_visibility): Compute const_value_known.
	* gimple-fold.c (get_symbol_constant_value): Use varpool for initializer
	folding.
	* varpool.c (varpool_decide_const_value_known): New function.

Comments

Richard Henderson Aug. 31, 2010, 11:12 p.m. UTC | #1
On 08/31/2010 04:08 PM, Jan Hubicka wrote:
> There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
> because fortran adds into varpool variable that is !TREE_STATIC &&
> !DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
> this area of FE to know what it is supposed to do.

FYI, I debugged this slightly further and the variable in question
is a PARM_DECL.  I didn't try to figure out why a PARM_DECL was
showing up on the global variable list.


r~
Jan Hubicka Sept. 1, 2010, 12:06 a.m. UTC | #2
> On 08/31/2010 04:08 PM, Jan Hubicka wrote:
> > There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
> > because fortran adds into varpool variable that is !TREE_STATIC &&
> > !DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
> > this area of FE to know what it is supposed to do.
> 
> FYI, I debugged this slightly further and the variable in question
> is a PARM_DECL.  I didn't try to figure out why a PARM_DECL was
> showing up on the global variable list.

It is because fortran calls rest_of_decl_compilation.  I did not check if it was
parm_decl, but for me it was variable "var" from the testcase. Perhaps we just need
FE to prevent calling r_o_d_c on parm decls then? :)

Honza
> 
> 
> r~
Jan Hubicka Sept. 2, 2010, 7:50 p.m. UTC | #3
Hi,
i re-tested the patch with the rest_of_decl_compilation fix and it does pass now.
Note that i am not terribly sure in what conditions we want to make the transform.
This patch chnages previous behaviour in two ways:

1) scalars and arrays/aggregates are now handled equivalently (same was as
scalars previously, so in partiuclar one can not overwrite initializer at
dynamic linking time)
2) COMDATs are handled as initializer being known since I use DECL_REPLACEABLE_P
instead of targetm.binds_local_p

OK?
Honza
> 
> There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
> because fortran adds into varpool variable that is !TREE_STATIC &&
> !DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
> this area of FE to know what it is supposed to do.
> 
> Does this seem to make sense?  Patch was bootstrapped/regtested x86_64-linux and
> darwin.
> 
> Honza
> 
> 	* cgraph.h (struct varpool_node): Add const_value_known.
> 	(varpool_decide_const_value_known): Declare.
> 	* tree-ssa-ccp.c (fold_const_aggregate_ref): Update initializer folding.
> 	* lto-cgraph.c (lto_output_varpool_node): Store const_value_known.
> 	(input_varpool_node): Restore const_value_known.
> 	* tree-ssa-loop-ivcanon (constant_after_peeling): Check varpool for
> 	initializer folding.
> 	* ipa.c (ipa_discover_readonly_nonaddressable_var,
> 	function_and_variable_visibility): Compute const_value_known.
> 	* gimple-fold.c (get_symbol_constant_value): Use varpool for initializer
> 	folding.
> 	* varpool.c (varpool_decide_const_value_known): New function.
> Index: gcc/cgraph.h
> ===================================================================
> --- gcc/cgraph.h	(revision 163636)
> +++ gcc/cgraph.h	(working copy)
> @@ -503,6 +503,8 @@ struct GTY((chain_next ("%h.next"), chai
>       During WPA output it is used to mark nodes that are present in
>       multiple partitions.  */
>    unsigned in_other_partition : 1;
> +  /* True when variable is constant and its value is known.  */
> +  unsigned int const_value_known : 1;
>  };
>  
>  /* Every top level asm statement is put into a cgraph_asm_node.  */
> @@ -726,6 +728,7 @@ void varpool_empty_needed_queue (void);
>  bool varpool_extra_name_alias (tree, tree);
>  const char * varpool_node_name (struct varpool_node *node);
>  void varpool_reset_queue (void);
> +bool varpool_decide_const_value_known (struct varpool_node *node);
>  
>  /* Walk all reachable static variables.  */
>  #define FOR_EACH_STATIC_VARIABLE(node) \
> Index: gcc/tree-ssa-ccp.c
> ===================================================================
> --- gcc/tree-ssa-ccp.c	(revision 163636)
> +++ gcc/tree-ssa-ccp.c	(working copy)
> @@ -1348,7 +1348,8 @@ fold_const_aggregate_ref (tree t)
>  	case VAR_DECL:
>  	  if (!TREE_READONLY (base)
>  	      || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
> -	      || !targetm.binds_local_p (base))
> +	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> +		  && !varpool_get_node (base)->const_value_known))
>  	    return NULL_TREE;
>  
>  	  ctor = DECL_INITIAL (base);
> @@ -1435,7 +1436,8 @@ fold_const_aggregate_ref (tree t)
>  	case VAR_DECL:
>  	  if (!TREE_READONLY (base)
>  	      || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
> -	      || !targetm.binds_local_p (base))
> +	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> +		  && !varpool_get_node (base)->const_value_known))
>  	    return NULL_TREE;
>  
>  	  ctor = DECL_INITIAL (base);
> @@ -1509,7 +1511,8 @@ fold_const_aggregate_ref (tree t)
>  
>  	  if (!TREE_READONLY (base)
>  	      || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
> -	      || !targetm.binds_local_p (base))
> +	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> +		  && !varpool_get_node (base)->const_value_known))
>  	    return NULL_TREE;
>  
>  	  ctor = DECL_INITIAL (base);
> Index: gcc/lto-cgraph.c
> ===================================================================
> --- gcc/lto-cgraph.c	(revision 163636)
> +++ gcc/lto-cgraph.c	(working copy)
> @@ -575,6 +575,7 @@ lto_output_varpool_node (struct lto_simp
>    bp_pack_value (&bp, node->force_output, 1);
>    bp_pack_value (&bp, node->finalized, 1);
>    bp_pack_value (&bp, node->alias, 1);
> +  bp_pack_value (&bp, node->const_value_known, 1);
>    gcc_assert (!node->alias || !node->extra_name);
>    gcc_assert (node->finalized || !node->analyzed);
>    gcc_assert (node->needed);
> @@ -1106,9 +1112,10 @@ input_varpool_node (struct lto_file_decl
>    node->force_output = bp_unpack_value (&bp, 1);
>    node->finalized = bp_unpack_value (&bp, 1);
>    node->alias = bp_unpack_value (&bp, 1);
> +  node->const_value_known = bp_unpack_value (&bp, 1);
>    node->analyzed = node->finalized; 
>    node->used_from_other_partition = bp_unpack_value (&bp, 1);
>    node->in_other_partition = bp_unpack_value (&bp, 1);
>    aliases_p = bp_unpack_value (&bp, 1);
>    if (node->finalized)
>      varpool_mark_needed_node (node);
> Index: gcc/tree-ssa-loop-ivcanon.c
> ===================================================================
> --- gcc/tree-ssa-loop-ivcanon.c	(revision 163636)
> +++ gcc/tree-ssa-loop-ivcanon.c	(working copy)
> @@ -165,9 +165,7 @@ constant_after_peeling (tree op, gimple 
>        if ((DECL_P (base)
>        	   && TREE_STATIC (base)
>  	   && TREE_READONLY (base)
> -           && (DECL_INITIAL (base)
> -	       || (!DECL_EXTERNAL (base)
> -		   && targetm.binds_local_p (base))))
> +	   && varpool_get_node (base)->const_value_known)
>  	  || CONSTANT_CLASS_P (base))
>  	{
>  	  /* If so, see if we understand all the indices.  */
> Index: gcc/ipa.c
> ===================================================================
> --- gcc/ipa.c	(revision 163636)
> +++ gcc/ipa.c	(working copy)
> @@ -561,6 +561,7 @@ ipa_discover_readonly_nonaddressable_var
>  	    if (dump_file)
>  	      fprintf (dump_file, " %s (read-only)", varpool_node_name (vnode));
>  	    TREE_READONLY (vnode->decl) = 1;
> +	    vnode->const_value_known |= varpool_decide_const_value_known (vnode);
>  	  }
>        }
>    if (dump_file)
> @@ -767,6 +768,9 @@ function_and_variable_visibility (bool w
>  	      || ! (ADDR_SPACE_GENERIC_P
>  		    (TYPE_ADDR_SPACE (TREE_TYPE (vnode->decl))))))
>  	DECL_COMMON (vnode->decl) = 0;
> +     /* Even extern variables might have initializers known.
> +	See, for example testsuite/g++.dg/opt/static3.C  */
> +     vnode->const_value_known |= varpool_decide_const_value_known (vnode);
>      }
>    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
>      {
> @@ -801,6 +805,7 @@ function_and_variable_visibility (bool w
>  	  gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
>  	  cgraph_make_decl_local (vnode->decl);
>  	}
> +     vnode->const_value_known |= varpool_decide_const_value_known (vnode);
>       gcc_assert (TREE_STATIC (vnode->decl));
>      }
>    pointer_set_destroy (aliased_nodes);
> Index: gcc/gimple-fold.c
> ===================================================================
> --- gcc/gimple-fold.c	(revision 163636)
> +++ gcc/gimple-fold.c	(working copy)
> @@ -38,9 +38,9 @@ along with GCC; see the file COPYING3.  
>  tree
>  get_symbol_constant_value (tree sym)
>  {
> -  if (TREE_STATIC (sym)
> -      && (TREE_READONLY (sym)
> -	  || TREE_CODE (sym) == CONST_DECL))
> +  if ((TREE_STATIC (sym) || DECL_EXTERNAL (sym))
> +      && (TREE_CODE (sym) == CONST_DECL
> +	  || varpool_get_node (sym)->const_value_known))
>      {
>        tree val = DECL_INITIAL (sym);
>        if (val)
> @@ -65,8 +65,6 @@ get_symbol_constant_value (tree sym)
>  	 have zero as the initializer if they may not be
>  	 overridden at link or run time.  */
>        if (!val
> -	  && !DECL_EXTERNAL (sym)
> -	  && targetm.binds_local_p (sym)
>            && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
>  	       || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
>  	return fold_convert (TREE_TYPE (sym), integer_zero_node);
> Index: gcc/varpool.c
> ===================================================================
> --- gcc/varpool.c	(revision 163636)
> +++ gcc/varpool.c	(working copy)
> @@ -357,6 +357,37 @@ decide_is_variable_needed (struct varpoo
>    return true;
>  }
>  
> +/* Return if NODE is constant and its initial value is known (so we can do
> +   constant folding).  The decision depends on whole program decisions
> +   and can not be recomputed at ltrans stage for variables from other
> +   partitions.  For this reason the new value should be always combined
> +   with the previous knowledge.  */
> +
> +bool
> +varpool_decide_const_value_known (struct varpool_node *node)
> +{
> +  tree decl = node->decl;
> +
> +  gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
> +  gcc_assert (TREE_CODE (decl) == VAR_DECL);
> +  if (!TREE_READONLY (decl))
> +    return false;
> +  /* Variables declared 'const' without an initializer
> +     have zero as the initializer if they may not be
> +     overridden at link or run time.  */
> +  if (!DECL_INITIAL (decl)
> +      && (DECL_EXTERNAL (decl)
> +	  || DECL_REPLACEABLE_P (decl)))
> +    return false;
> +
> +  /* Variables declared `const' with an initializer are considered
> +     to not be overwritable with different initializer by default. 
> +
> +     ??? Previously we behaved so for scalar variables but not for array
> +     accesses.  */
> +  return true;
> +}
> +
>  /* Mark DECL as finalized.  By finalizing the declaration, frontend instruct the
>     middle end to output the variable to asm file, if needed or externally
>     visible.  */
Richard Biener Sept. 3, 2010, 9 a.m. UTC | #4
On Thu, 2 Sep 2010, Jan Hubicka wrote:

> Hi,
> i re-tested the patch with the rest_of_decl_compilation fix and it does pass now.
> Note that i am not terribly sure in what conditions we want to make the transform.
> This patch chnages previous behaviour in two ways:
> 
> 1) scalars and arrays/aggregates are now handled equivalently (same was as
> scalars previously, so in partiuclar one can not overwrite initializer at
> dynamic linking time)
> 2) COMDATs are handled as initializer being known since I use DECL_REPLACEABLE_P
> instead of targetm.binds_local_p
> 
> OK?

Ok.

Thanks,
Richard.

> Honza
> > 
> > There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
> > because fortran adds into varpool variable that is !TREE_STATIC &&
> > !DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
> > this area of FE to know what it is supposed to do.
> > 
> > Does this seem to make sense?  Patch was bootstrapped/regtested x86_64-linux and
> > darwin.
> > 
> > Honza
> > 
> > 	* cgraph.h (struct varpool_node): Add const_value_known.
> > 	(varpool_decide_const_value_known): Declare.
> > 	* tree-ssa-ccp.c (fold_const_aggregate_ref): Update initializer folding.
> > 	* lto-cgraph.c (lto_output_varpool_node): Store const_value_known.
> > 	(input_varpool_node): Restore const_value_known.
> > 	* tree-ssa-loop-ivcanon (constant_after_peeling): Check varpool for
> > 	initializer folding.
> > 	* ipa.c (ipa_discover_readonly_nonaddressable_var,
> > 	function_and_variable_visibility): Compute const_value_known.
> > 	* gimple-fold.c (get_symbol_constant_value): Use varpool for initializer
> > 	folding.
> > 	* varpool.c (varpool_decide_const_value_known): New function.
> > Index: gcc/cgraph.h
> > ===================================================================
> > --- gcc/cgraph.h	(revision 163636)
> > +++ gcc/cgraph.h	(working copy)
> > @@ -503,6 +503,8 @@ struct GTY((chain_next ("%h.next"), chai
> >       During WPA output it is used to mark nodes that are present in
> >       multiple partitions.  */
> >    unsigned in_other_partition : 1;
> > +  /* True when variable is constant and its value is known.  */
> > +  unsigned int const_value_known : 1;
> >  };
> >  
> >  /* Every top level asm statement is put into a cgraph_asm_node.  */
> > @@ -726,6 +728,7 @@ void varpool_empty_needed_queue (void);
> >  bool varpool_extra_name_alias (tree, tree);
> >  const char * varpool_node_name (struct varpool_node *node);
> >  void varpool_reset_queue (void);
> > +bool varpool_decide_const_value_known (struct varpool_node *node);
> >  
> >  /* Walk all reachable static variables.  */
> >  #define FOR_EACH_STATIC_VARIABLE(node) \
> > Index: gcc/tree-ssa-ccp.c
> > ===================================================================
> > --- gcc/tree-ssa-ccp.c	(revision 163636)
> > +++ gcc/tree-ssa-ccp.c	(working copy)
> > @@ -1348,7 +1348,8 @@ fold_const_aggregate_ref (tree t)
> >  	case VAR_DECL:
> >  	  if (!TREE_READONLY (base)
> >  	      || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
> > -	      || !targetm.binds_local_p (base))
> > +	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> > +		  && !varpool_get_node (base)->const_value_known))
> >  	    return NULL_TREE;
> >  
> >  	  ctor = DECL_INITIAL (base);
> > @@ -1435,7 +1436,8 @@ fold_const_aggregate_ref (tree t)
> >  	case VAR_DECL:
> >  	  if (!TREE_READONLY (base)
> >  	      || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
> > -	      || !targetm.binds_local_p (base))
> > +	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> > +		  && !varpool_get_node (base)->const_value_known))
> >  	    return NULL_TREE;
> >  
> >  	  ctor = DECL_INITIAL (base);
> > @@ -1509,7 +1511,8 @@ fold_const_aggregate_ref (tree t)
> >  
> >  	  if (!TREE_READONLY (base)
> >  	      || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
> > -	      || !targetm.binds_local_p (base))
> > +	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> > +		  && !varpool_get_node (base)->const_value_known))
> >  	    return NULL_TREE;
> >  
> >  	  ctor = DECL_INITIAL (base);
> > Index: gcc/lto-cgraph.c
> > ===================================================================
> > --- gcc/lto-cgraph.c	(revision 163636)
> > +++ gcc/lto-cgraph.c	(working copy)
> > @@ -575,6 +575,7 @@ lto_output_varpool_node (struct lto_simp
> >    bp_pack_value (&bp, node->force_output, 1);
> >    bp_pack_value (&bp, node->finalized, 1);
> >    bp_pack_value (&bp, node->alias, 1);
> > +  bp_pack_value (&bp, node->const_value_known, 1);
> >    gcc_assert (!node->alias || !node->extra_name);
> >    gcc_assert (node->finalized || !node->analyzed);
> >    gcc_assert (node->needed);
> > @@ -1106,9 +1112,10 @@ input_varpool_node (struct lto_file_decl
> >    node->force_output = bp_unpack_value (&bp, 1);
> >    node->finalized = bp_unpack_value (&bp, 1);
> >    node->alias = bp_unpack_value (&bp, 1);
> > +  node->const_value_known = bp_unpack_value (&bp, 1);
> >    node->analyzed = node->finalized; 
> >    node->used_from_other_partition = bp_unpack_value (&bp, 1);
> >    node->in_other_partition = bp_unpack_value (&bp, 1);
> >    aliases_p = bp_unpack_value (&bp, 1);
> >    if (node->finalized)
> >      varpool_mark_needed_node (node);
> > Index: gcc/tree-ssa-loop-ivcanon.c
> > ===================================================================
> > --- gcc/tree-ssa-loop-ivcanon.c	(revision 163636)
> > +++ gcc/tree-ssa-loop-ivcanon.c	(working copy)
> > @@ -165,9 +165,7 @@ constant_after_peeling (tree op, gimple 
> >        if ((DECL_P (base)
> >        	   && TREE_STATIC (base)
> >  	   && TREE_READONLY (base)
> > -           && (DECL_INITIAL (base)
> > -	       || (!DECL_EXTERNAL (base)
> > -		   && targetm.binds_local_p (base))))
> > +	   && varpool_get_node (base)->const_value_known)
> >  	  || CONSTANT_CLASS_P (base))
> >  	{
> >  	  /* If so, see if we understand all the indices.  */
> > Index: gcc/ipa.c
> > ===================================================================
> > --- gcc/ipa.c	(revision 163636)
> > +++ gcc/ipa.c	(working copy)
> > @@ -561,6 +561,7 @@ ipa_discover_readonly_nonaddressable_var
> >  	    if (dump_file)
> >  	      fprintf (dump_file, " %s (read-only)", varpool_node_name (vnode));
> >  	    TREE_READONLY (vnode->decl) = 1;
> > +	    vnode->const_value_known |= varpool_decide_const_value_known (vnode);
> >  	  }
> >        }
> >    if (dump_file)
> > @@ -767,6 +768,9 @@ function_and_variable_visibility (bool w
> >  	      || ! (ADDR_SPACE_GENERIC_P
> >  		    (TYPE_ADDR_SPACE (TREE_TYPE (vnode->decl))))))
> >  	DECL_COMMON (vnode->decl) = 0;
> > +     /* Even extern variables might have initializers known.
> > +	See, for example testsuite/g++.dg/opt/static3.C  */
> > +     vnode->const_value_known |= varpool_decide_const_value_known (vnode);
> >      }
> >    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
> >      {
> > @@ -801,6 +805,7 @@ function_and_variable_visibility (bool w
> >  	  gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
> >  	  cgraph_make_decl_local (vnode->decl);
> >  	}
> > +     vnode->const_value_known |= varpool_decide_const_value_known (vnode);
> >       gcc_assert (TREE_STATIC (vnode->decl));
> >      }
> >    pointer_set_destroy (aliased_nodes);
> > Index: gcc/gimple-fold.c
> > ===================================================================
> > --- gcc/gimple-fold.c	(revision 163636)
> > +++ gcc/gimple-fold.c	(working copy)
> > @@ -38,9 +38,9 @@ along with GCC; see the file COPYING3.  
> >  tree
> >  get_symbol_constant_value (tree sym)
> >  {
> > -  if (TREE_STATIC (sym)
> > -      && (TREE_READONLY (sym)
> > -	  || TREE_CODE (sym) == CONST_DECL))
> > +  if ((TREE_STATIC (sym) || DECL_EXTERNAL (sym))
> > +      && (TREE_CODE (sym) == CONST_DECL
> > +	  || varpool_get_node (sym)->const_value_known))
> >      {
> >        tree val = DECL_INITIAL (sym);
> >        if (val)
> > @@ -65,8 +65,6 @@ get_symbol_constant_value (tree sym)
> >  	 have zero as the initializer if they may not be
> >  	 overridden at link or run time.  */
> >        if (!val
> > -	  && !DECL_EXTERNAL (sym)
> > -	  && targetm.binds_local_p (sym)
> >            && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
> >  	       || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
> >  	return fold_convert (TREE_TYPE (sym), integer_zero_node);
> > Index: gcc/varpool.c
> > ===================================================================
> > --- gcc/varpool.c	(revision 163636)
> > +++ gcc/varpool.c	(working copy)
> > @@ -357,6 +357,37 @@ decide_is_variable_needed (struct varpoo
> >    return true;
> >  }
> >  
> > +/* Return if NODE is constant and its initial value is known (so we can do
> > +   constant folding).  The decision depends on whole program decisions
> > +   and can not be recomputed at ltrans stage for variables from other
> > +   partitions.  For this reason the new value should be always combined
> > +   with the previous knowledge.  */
> > +
> > +bool
> > +varpool_decide_const_value_known (struct varpool_node *node)
> > +{
> > +  tree decl = node->decl;
> > +
> > +  gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
> > +  gcc_assert (TREE_CODE (decl) == VAR_DECL);
> > +  if (!TREE_READONLY (decl))
> > +    return false;
> > +  /* Variables declared 'const' without an initializer
> > +     have zero as the initializer if they may not be
> > +     overridden at link or run time.  */
> > +  if (!DECL_INITIAL (decl)
> > +      && (DECL_EXTERNAL (decl)
> > +	  || DECL_REPLACEABLE_P (decl)))
> > +    return false;
> > +
> > +  /* Variables declared `const' with an initializer are considered
> > +     to not be overwritable with different initializer by default. 
> > +
> > +     ??? Previously we behaved so for scalar variables but not for array
> > +     accesses.  */
> > +  return true;
> > +}
> > +
> >  /* Mark DECL as finalized.  By finalizing the declaration, frontend instruct the
> >     middle end to output the variable to asm file, if needed or externally
> >     visible.  */
> 
>
H.J. Lu Sept. 10, 2010, 4:29 a.m. UTC | #5
On Tue, Aug 31, 2010 at 4:08 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi.
> at various places we need to fold const static variable via knowledge of its
> initializer.  The precise conditions when we do so hoever vary. For example
>
> const int a=5;
> const int b[2]={0,1};
> main()
> {
>  return a+b[0];
> }
>
> compiled with -O2 -fpic gets a folded to 5, but b[0] is not folded to 0.
>
> In order to fix darwin WHOPR partitioning I need variables with extern linkage
> but known initializer.  Even this is supported for scalars, but not for arrays,
> see i.e. g++.dg/opt/static3.C but not for arrays or structures.
>
> I am quite convinced that with WHOPR partitioning, we can't derive the fact
> from current DECL flags.
>
> static const int a;
>
> Allows folding referenced of A to 0. However with partitioning it might become:
> __attribute__ ((visibility ("hidden"))) extern const int a;
> ... and we would lose knowledge that A is 0.
>
> For this reason patch adds const_value_known into varpool structure and makes
> existing folders to use it.  Note that there is other place folding initialiers
> in expand.c I would like to remove (tree optimizers can do the same) but that
> would need some extra work.
>
> There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
> because fortran adds into varpool variable that is !TREE_STATIC &&
> !DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
> this area of FE to know what it is supposed to do.
>
> Does this seem to make sense?  Patch was bootstrapped/regtested x86_64-linux and
> darwin.
>
> Honza
>
>        * cgraph.h (struct varpool_node): Add const_value_known.
>        (varpool_decide_const_value_known): Declare.
>        * tree-ssa-ccp.c (fold_const_aggregate_ref): Update initializer folding.
>        * lto-cgraph.c (lto_output_varpool_node): Store const_value_known.
>        (input_varpool_node): Restore const_value_known.
>        * tree-ssa-loop-ivcanon (constant_after_peeling): Check varpool for
>        initializer folding.
>        * ipa.c (ipa_discover_readonly_nonaddressable_var,
>        function_and_variable_visibility): Compute const_value_known.
>        * gimple-fold.c (get_symbol_constant_value): Use varpool for initializer
>        folding.
>        * varpool.c (varpool_decide_const_value_known): New function.

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45626
H.J. Lu Sept. 21, 2010, 12:01 a.m. UTC | #6
On Thu, Sep 9, 2010 at 9:29 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Aug 31, 2010 at 4:08 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi.
>> at various places we need to fold const static variable via knowledge of its
>> initializer.  The precise conditions when we do so hoever vary. For example
>>
>> const int a=5;
>> const int b[2]={0,1};
>> main()
>> {
>>  return a+b[0];
>> }
>>
>> compiled with -O2 -fpic gets a folded to 5, but b[0] is not folded to 0.
>>
>> In order to fix darwin WHOPR partitioning I need variables with extern linkage
>> but known initializer.  Even this is supported for scalars, but not for arrays,
>> see i.e. g++.dg/opt/static3.C but not for arrays or structures.
>>
>> I am quite convinced that with WHOPR partitioning, we can't derive the fact
>> from current DECL flags.
>>
>> static const int a;
>>
>> Allows folding referenced of A to 0. However with partitioning it might become:
>> __attribute__ ((visibility ("hidden"))) extern const int a;
>> ... and we would lose knowledge that A is 0.
>>
>> For this reason patch adds const_value_known into varpool structure and makes
>> existing folders to use it.  Note that there is other place folding initialiers
>> in expand.c I would like to remove (tree optimizers can do the same) but that
>> would need some extra work.
>>
>> There is one failure with the patch, gfortran.dg/cray_pointers_5.f90.  It is
>> because fortran adds into varpool variable that is !TREE_STATIC &&
>> !DECL_EXTERNAL that seems like a bug to me, but I am not familiar enough with
>> this area of FE to know what it is supposed to do.
>>
>> Does this seem to make sense?  Patch was bootstrapped/regtested x86_64-linux and
>> darwin.
>>
>> Honza
>>
>>        * cgraph.h (struct varpool_node): Add const_value_known.
>>        (varpool_decide_const_value_known): Declare.
>>        * tree-ssa-ccp.c (fold_const_aggregate_ref): Update initializer folding.
>>        * lto-cgraph.c (lto_output_varpool_node): Store const_value_known.
>>        (input_varpool_node): Restore const_value_known.
>>        * tree-ssa-loop-ivcanon (constant_after_peeling): Check varpool for
>>        initializer folding.
>>        * ipa.c (ipa_discover_readonly_nonaddressable_var,
>>        function_and_variable_visibility): Compute const_value_known.
>>        * gimple-fold.c (get_symbol_constant_value): Use varpool for initializer
>>        folding.
>>        * varpool.c (varpool_decide_const_value_known): New function.
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45626
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45712
diff mbox

Patch

Index: gcc/cgraph.h
===================================================================
--- gcc/cgraph.h	(revision 163636)
+++ gcc/cgraph.h	(working copy)
@@ -503,6 +503,8 @@  struct GTY((chain_next ("%h.next"), chai
      During WPA output it is used to mark nodes that are present in
      multiple partitions.  */
   unsigned in_other_partition : 1;
+  /* True when variable is constant and its value is known.  */
+  unsigned int const_value_known : 1;
 };
 
 /* Every top level asm statement is put into a cgraph_asm_node.  */
@@ -726,6 +728,7 @@  void varpool_empty_needed_queue (void);
 bool varpool_extra_name_alias (tree, tree);
 const char * varpool_node_name (struct varpool_node *node);
 void varpool_reset_queue (void);
+bool varpool_decide_const_value_known (struct varpool_node *node);
 
 /* Walk all reachable static variables.  */
 #define FOR_EACH_STATIC_VARIABLE(node) \
Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c	(revision 163636)
+++ gcc/tree-ssa-ccp.c	(working copy)
@@ -1348,7 +1348,8 @@  fold_const_aggregate_ref (tree t)
 	case VAR_DECL:
 	  if (!TREE_READONLY (base)
 	      || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
-	      || !targetm.binds_local_p (base))
+	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
+		  && !varpool_get_node (base)->const_value_known))
 	    return NULL_TREE;
 
 	  ctor = DECL_INITIAL (base);
@@ -1435,7 +1436,8 @@  fold_const_aggregate_ref (tree t)
 	case VAR_DECL:
 	  if (!TREE_READONLY (base)
 	      || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
-	      || !targetm.binds_local_p (base))
+	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
+		  && !varpool_get_node (base)->const_value_known))
 	    return NULL_TREE;
 
 	  ctor = DECL_INITIAL (base);
@@ -1509,7 +1511,8 @@  fold_const_aggregate_ref (tree t)
 
 	  if (!TREE_READONLY (base)
 	      || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE
-	      || !targetm.binds_local_p (base))
+	      || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
+		  && !varpool_get_node (base)->const_value_known))
 	    return NULL_TREE;
 
 	  ctor = DECL_INITIAL (base);
Index: gcc/lto-cgraph.c
===================================================================
--- gcc/lto-cgraph.c	(revision 163636)
+++ gcc/lto-cgraph.c	(working copy)
@@ -575,6 +575,7 @@  lto_output_varpool_node (struct lto_simp
   bp_pack_value (&bp, node->force_output, 1);
   bp_pack_value (&bp, node->finalized, 1);
   bp_pack_value (&bp, node->alias, 1);
+  bp_pack_value (&bp, node->const_value_known, 1);
   gcc_assert (!node->alias || !node->extra_name);
   gcc_assert (node->finalized || !node->analyzed);
   gcc_assert (node->needed);
@@ -1106,9 +1112,10 @@  input_varpool_node (struct lto_file_decl
   node->force_output = bp_unpack_value (&bp, 1);
   node->finalized = bp_unpack_value (&bp, 1);
   node->alias = bp_unpack_value (&bp, 1);
+  node->const_value_known = bp_unpack_value (&bp, 1);
   node->analyzed = node->finalized; 
   node->used_from_other_partition = bp_unpack_value (&bp, 1);
   node->in_other_partition = bp_unpack_value (&bp, 1);
   aliases_p = bp_unpack_value (&bp, 1);
   if (node->finalized)
     varpool_mark_needed_node (node);
Index: gcc/tree-ssa-loop-ivcanon.c
===================================================================
--- gcc/tree-ssa-loop-ivcanon.c	(revision 163636)
+++ gcc/tree-ssa-loop-ivcanon.c	(working copy)
@@ -165,9 +165,7 @@  constant_after_peeling (tree op, gimple 
       if ((DECL_P (base)
       	   && TREE_STATIC (base)
 	   && TREE_READONLY (base)
-           && (DECL_INITIAL (base)
-	       || (!DECL_EXTERNAL (base)
-		   && targetm.binds_local_p (base))))
+	   && varpool_get_node (base)->const_value_known)
 	  || CONSTANT_CLASS_P (base))
 	{
 	  /* If so, see if we understand all the indices.  */
Index: gcc/ipa.c
===================================================================
--- gcc/ipa.c	(revision 163636)
+++ gcc/ipa.c	(working copy)
@@ -561,6 +561,7 @@  ipa_discover_readonly_nonaddressable_var
 	    if (dump_file)
 	      fprintf (dump_file, " %s (read-only)", varpool_node_name (vnode));
 	    TREE_READONLY (vnode->decl) = 1;
+	    vnode->const_value_known |= varpool_decide_const_value_known (vnode);
 	  }
       }
   if (dump_file)
@@ -767,6 +768,9 @@  function_and_variable_visibility (bool w
 	      || ! (ADDR_SPACE_GENERIC_P
 		    (TYPE_ADDR_SPACE (TREE_TYPE (vnode->decl))))))
 	DECL_COMMON (vnode->decl) = 0;
+     /* Even extern variables might have initializers known.
+	See, for example testsuite/g++.dg/opt/static3.C  */
+     vnode->const_value_known |= varpool_decide_const_value_known (vnode);
     }
   for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
     {
@@ -801,6 +805,7 @@  function_and_variable_visibility (bool w
 	  gcc_assert (in_lto_p || whole_program || !TREE_PUBLIC (vnode->decl));
 	  cgraph_make_decl_local (vnode->decl);
 	}
+     vnode->const_value_known |= varpool_decide_const_value_known (vnode);
      gcc_assert (TREE_STATIC (vnode->decl));
     }
   pointer_set_destroy (aliased_nodes);
Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 163636)
+++ gcc/gimple-fold.c	(working copy)
@@ -38,9 +38,9 @@  along with GCC; see the file COPYING3.  
 tree
 get_symbol_constant_value (tree sym)
 {
-  if (TREE_STATIC (sym)
-      && (TREE_READONLY (sym)
-	  || TREE_CODE (sym) == CONST_DECL))
+  if ((TREE_STATIC (sym) || DECL_EXTERNAL (sym))
+      && (TREE_CODE (sym) == CONST_DECL
+	  || varpool_get_node (sym)->const_value_known))
     {
       tree val = DECL_INITIAL (sym);
       if (val)
@@ -65,8 +65,6 @@  get_symbol_constant_value (tree sym)
 	 have zero as the initializer if they may not be
 	 overridden at link or run time.  */
       if (!val
-	  && !DECL_EXTERNAL (sym)
-	  && targetm.binds_local_p (sym)
           && (INTEGRAL_TYPE_P (TREE_TYPE (sym))
 	       || SCALAR_FLOAT_TYPE_P (TREE_TYPE (sym))))
 	return fold_convert (TREE_TYPE (sym), integer_zero_node);
Index: gcc/varpool.c
===================================================================
--- gcc/varpool.c	(revision 163636)
+++ gcc/varpool.c	(working copy)
@@ -357,6 +357,37 @@  decide_is_variable_needed (struct varpoo
   return true;
 }
 
+/* Return if NODE is constant and its initial value is known (so we can do
+   constant folding).  The decision depends on whole program decisions
+   and can not be recomputed at ltrans stage for variables from other
+   partitions.  For this reason the new value should be always combined
+   with the previous knowledge.  */
+
+bool
+varpool_decide_const_value_known (struct varpool_node *node)
+{
+  tree decl = node->decl;
+
+  gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
+  gcc_assert (TREE_CODE (decl) == VAR_DECL);
+  if (!TREE_READONLY (decl))
+    return false;
+  /* Variables declared 'const' without an initializer
+     have zero as the initializer if they may not be
+     overridden at link or run time.  */
+  if (!DECL_INITIAL (decl)
+      && (DECL_EXTERNAL (decl)
+	  || DECL_REPLACEABLE_P (decl)))
+    return false;
+
+  /* Variables declared `const' with an initializer are considered
+     to not be overwritable with different initializer by default. 
+
+     ??? Previously we behaved so for scalar variables but not for array
+     accesses.  */
+  return true;
+}
+
 /* Mark DECL as finalized.  By finalizing the declaration, frontend instruct the
    middle end to output the variable to asm file, if needed or externally
    visible.  */