PR tree-optimization/45605

Hi,
I've tested the other part of previous patch and comitted everything except
for the code introducing folding itself as they address other latent issues.

The problem with folders being called before cgraph&varpool are built is common
to issue I was addressing by my earlier string_constant patch that was refused
as too confusing.

Constant constructors are just special case of wider issue that with linker
plugin or WHOPR we do have more knowledge about what is locally bound than
in normal compilation.

After discussion with Richard on IRC, we got to conclusion that to address the
issue we probably should store resolution decisions into varpool and cgraph nodes.
Varpool's const_value_known and cgraph+varpool used_in_object_file flags can
then be turned into predicates looking into resolutions and DECL flags. Also
binds_local_p can be strenghtened to look up resolution info and take it into
account when dealing with weak, external and comdat decls.

I have patch for this change, but since it is tricky by itself (updates
binds_local_p logic, adds logic into lto-symtab to bookkeep what values come
from linker and what was decided in GCC, etc.) I decided to break those
changes.  This is patch that goes half way by introducing const_value_known_p
predicate while keeping current way of passing const_value_known flag from WPA
to LTRANS.

I updated the predicate to work on CONST_DECL, automatic vars, other decls and
vars w/o varpool entries allocated (yet), so it can be used more conveniently.
Now when one has non function/method DECL, it is possible use this predicate to
test whether it makes sense to look at DECL_INITIAL to get the constructor.
One no longer needs to special case constant pool, CONST_DECL, and check that
variable is static before attempting to do so.

This finally lets us to do the statement folding during gimplification without
ICEs.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* cgraph.h (const_value_known_p): Declare.
	(varpool_decide_const_value_known): Remove.
	* tree-ssa-ccp.c (get_base_constructor): Use it.
	* lto-cgraph.c (compute_ltrans_boundary): Likewise.
	* expr.c (string_constant): Likewise.
	* tree-ssa-loop-ivcanon.c (constant_after_peeling): Likewise.
	* ipa.c (ipa_discover_readonly_nonaddressable_var,
	function_and_variable_visibility): Likewise.
	* gimplify.c (gimplify_call_expr): Likewise.
	* gimple-fold.c (get_symbol_constant_value): Likewise.
	* varpool.c (varpool_decide_const_value_known): Replace by...
	(const_value_known_p): ... this one; handle other kinds of DECLs
	too and work for automatic vars.
	(varpool_finalize_decl): Use const_value_known_p.

	* lto.c (lto_promote_cross_file_statics): Use const_value_known_p.

	* g++.dg/tree-ssa/pr45605.C: New testcase.

On Mon, 20 Sep 2010, Jan Hubicka wrote:

> Hi,
> I've tested the other part of previous patch and comitted everything except
> for the code introducing folding itself as they address other latent issues.
> 
> The problem with folders being called before cgraph&varpool are built is common
> to issue I was addressing by my earlier string_constant patch that was refused
> as too confusing.
> 
> Constant constructors are just special case of wider issue that with linker
> plugin or WHOPR we do have more knowledge about what is locally bound than
> in normal compilation.
> 
> After discussion with Richard on IRC, we got to conclusion that to address the
> issue we probably should store resolution decisions into varpool and cgraph nodes.
> Varpool's const_value_known and cgraph+varpool used_in_object_file flags can
> then be turned into predicates looking into resolutions and DECL flags. Also
> binds_local_p can be strenghtened to look up resolution info and take it into
> account when dealing with weak, external and comdat decls.
> 
> I have patch for this change, but since it is tricky by itself (updates
> binds_local_p logic, adds logic into lto-symtab to bookkeep what values come
> from linker and what was decided in GCC, etc.) I decided to break those
> changes.  This is patch that goes half way by introducing const_value_known_p
> predicate while keeping current way of passing const_value_known flag from WPA
> to LTRANS.
> 
> I updated the predicate to work on CONST_DECL, automatic vars, other decls and
> vars w/o varpool entries allocated (yet), so it can be used more conveniently.
> Now when one has non function/method DECL, it is possible use this predicate to
> test whether it makes sense to look at DECL_INITIAL to get the constructor.
> One no longer needs to special case constant pool, CONST_DECL, and check that
> variable is static before attempting to do so.
> 
> This finally lets us to do the statement folding during gimplification without
> ICEs.
> 
> Bootstrapped/regtested x86_64-linux, OK?

Ok.

Thanks,
Richard.

> Honza
> 
> 	* cgraph.h (const_value_known_p): Declare.
> 	(varpool_decide_const_value_known): Remove.
> 	* tree-ssa-ccp.c (get_base_constructor): Use it.
> 	* lto-cgraph.c (compute_ltrans_boundary): Likewise.
> 	* expr.c (string_constant): Likewise.
> 	* tree-ssa-loop-ivcanon.c (constant_after_peeling): Likewise.
> 	* ipa.c (ipa_discover_readonly_nonaddressable_var,
> 	function_and_variable_visibility): Likewise.
> 	* gimplify.c (gimplify_call_expr): Likewise.
> 	* gimple-fold.c (get_symbol_constant_value): Likewise.
> 	* varpool.c (varpool_decide_const_value_known): Replace by...
> 	(const_value_known_p): ... this one; handle other kinds of DECLs
> 	too and work for automatic vars.
> 	(varpool_finalize_decl): Use const_value_known_p.
> 
> 	* lto.c (lto_promote_cross_file_statics): Use const_value_known_p.
> 
> 	* g++.dg/tree-ssa/pr45605.C: New testcase.
> Index: cgraph.h
> ===================================================================
> --- cgraph.h	(revision 164400)
> +++ cgraph.h	(working copy)
> @@ -728,7 +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);
> +bool const_value_known_p (tree);
>  
>  /* Walk all reachable static variables.  */
>  #define FOR_EACH_STATIC_VARIABLE(node) \
> Index: testsuite/g++.dg/tree-ssa/pr45605.C
> ===================================================================
> --- testsuite/g++.dg/tree-ssa/pr45605.C	(revision 0)
> +++ testsuite/g++.dg/tree-ssa/pr45605.C	(revision 0)
> @@ -0,0 +1,37 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-ssa" } */
> +extern "C" void abort(); 
> +bool destructor_called = false; 
> +
> +struct B { 
> +    virtual void Run(){}; 
> +}; 
> +
> +struct D : public B { 
> +    virtual void Run() 
> +      { 
> +        struct O { 
> +            ~O() { destructor_called = true; }; 
> +        } o; 
> +
> +        struct Raiser { 
> +            Raiser()  throw( int ) {throw 1;}; 
> +        } raiser; 
> +      }; 
> +}; 
> +
> +int main() { 
> +    try { 
> +      D d; 
> +      static_cast<B&>(d).Run(); 
> +    } catch (...) {} 
> +
> +    if (!destructor_called) 
> +      abort (); 
> +} 
> +
> +
> +
> +/* We should devirtualize call to D::Run */
> +/* { dg-final { scan-tree-dump-times "D::Run (" 1 "ssa"} } */
> +/* { dg-final { cleanup-tree-dump "ssa" } } */
> Index: tree-ssa-ccp.c
> ===================================================================
> --- tree-ssa-ccp.c	(revision 164400)
> +++ tree-ssa-ccp.c	(working copy)
> @@ -1342,9 +1342,7 @@ get_base_constructor (tree base, tree *o
>    switch (TREE_CODE (base))
>      {
>      case VAR_DECL:
> -      if (!TREE_READONLY (base)
> -	  || ((TREE_STATIC (base) || DECL_EXTERNAL (base))
> -	      && !varpool_get_node (base)->const_value_known))
> +      if (!const_value_known_p (base))
>  	return NULL_TREE;
>  
>        /* Fallthru.  */
> Index: lto-cgraph.c
> ===================================================================
> --- lto-cgraph.c	(revision 164400)
> +++ lto-cgraph.c	(working copy)
> @@ -813,8 +813,7 @@ compute_ltrans_boundary (struct lto_out_
>        if (DECL_INITIAL (vnode->decl)
>  	  && !lto_varpool_encoder_encode_initializer_p (varpool_encoder,
>  						        vnode)
> -	  && (DECL_IN_CONSTANT_POOL (vnode->decl)
> -	      || vnode->const_value_known))
> +	  && const_value_known_p (vnode->decl))
>  	{
>  	  lto_set_varpool_encoder_encode_initializer (varpool_encoder, vnode);
>  	  add_references (encoder, varpool_encoder, &vnode->ref_list);
> Index: expr.c
> ===================================================================
> --- expr.c	(revision 164400)
> +++ expr.c	(working copy)
> @@ -9851,16 +9851,10 @@ string_constant (tree arg, tree *ptr_off
>        int length;
>  
>        /* Variables initialized to string literals can be handled too.  */
> -      if (DECL_INITIAL (array) == NULL_TREE
> +      if (!const_value_known_p (array)
>  	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
>  	return 0;
>  
> -      /* If they are read-only, non-volatile and bind locally.  */
> -      if (! TREE_READONLY (array)
> -	  || TREE_SIDE_EFFECTS (array)
> -	  || ! targetm.binds_local_p (array))
> -	return 0;
> -
>        /* Avoid const char foo[4] = "abcde";  */
>        if (DECL_SIZE_UNIT (array) == NULL_TREE
>  	  || TREE_CODE (DECL_SIZE_UNIT (array)) != INTEGER_CST
> Index: tree-ssa-loop-ivcanon.c
> ===================================================================
> --- tree-ssa-loop-ivcanon.c	(revision 164400)
> +++ tree-ssa-loop-ivcanon.c	(working copy)
> @@ -162,10 +162,8 @@ constant_after_peeling (tree op, gimple 
>        /* First make fast look if we see constant array inside.  */
>        while (handled_component_p (base))
>  	base = TREE_OPERAND (base, 0);
> -      if ((DECL_P (base)
> -      	   && TREE_STATIC (base)
> -	   && TREE_READONLY (base)
> -	   && varpool_get_node (base)->const_value_known)
> +      if ((DECL_P (base) == VAR_DECL
> +	   && const_value_known_p (base))
>  	  || CONSTANT_CLASS_P (base))
>  	{
>  	  /* If so, see if we understand all the indices.  */
> Index: ipa.c
> ===================================================================
> --- ipa.c	(revision 164400)
> +++ ipa.c	(working copy)
> @@ -565,7 +565,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);
> +	    vnode->const_value_known |= const_value_known_p (vnode->decl);
>  	  }
>        }
>    if (dump_file)
> @@ -774,7 +774,7 @@ function_and_variable_visibility (bool w
>  	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);
> +     vnode->const_value_known |= const_value_known_p (vnode->decl);
>      }
>    for (vnode = varpool_nodes_queue; vnode; vnode = vnode->next_needed)
>      {
> @@ -809,7 +809,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);
> +     vnode->const_value_known |= const_value_known_p (vnode->decl);
>       gcc_assert (TREE_STATIC (vnode->decl));
>      }
>    pointer_set_destroy (aliased_nodes);
> Index: gimplify.c
> ===================================================================
> --- gimplify.c	(revision 164400)
> +++ gimplify.c	(working copy)
> @@ -2479,8 +2479,11 @@ gimplify_call_expr (tree *expr_p, gimple
>      {
>        /* The CALL_EXPR in *EXPR_P is already in GIMPLE form, so all we
>  	 have to do is replicate it as a GIMPLE_CALL tuple.  */
> +      gimple_stmt_iterator gsi;
>        call = gimple_build_call_from_tree (*expr_p);
>        gimplify_seq_add_stmt (pre_p, call);
> +      gsi = gsi_last (*pre_p);
> +      fold_stmt (&gsi);
>        *expr_p = NULL_TREE;
>      }
>  
> Index: gimple-fold.c
> ===================================================================
> --- gimple-fold.c	(revision 164402)
> +++ gimple-fold.c	(working copy)
> @@ -122,9 +122,7 @@ canonicalize_constructor_val (tree cval)
>  tree
>  get_symbol_constant_value (tree sym)
>  {
> -  if ((TREE_STATIC (sym) || DECL_EXTERNAL (sym))
> -      && (TREE_CODE (sym) == CONST_DECL
> -	  || varpool_get_node (sym)->const_value_known))
> +  if (const_value_known_p (sym))
>      {
>        tree val = DECL_INITIAL (sym);
>        if (val)
> Index: lto/lto.c
> ===================================================================
> --- lto/lto.c	(revision 164400)
> +++ lto/lto.c	(working copy)
> @@ -1008,7 +1008,7 @@ lto_promote_cross_file_statics (void)
>  	 from this partition that are not in this partition.
>  	 This needs to be done recursively.  */
>        for (vnode = varpool_nodes; vnode; vnode = vnode->next)
> -	if ((vnode->const_value_known || DECL_IN_CONSTANT_POOL (vnode->decl))
> +	if (const_value_known_p (vnode->decl)
>  	    && DECL_INITIAL (vnode->decl)
>  	    && !varpool_node_in_set_p (vnode, vset)
>  	    && referenced_from_this_partition_p (&vnode->ref_list, set, vset)
> Index: varpool.c
> ===================================================================
> --- varpool.c	(revision 164400)
> +++ varpool.c	(working copy)
> @@ -359,21 +359,42 @@ 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.  */
> +/* Return if DECL is constant and its initial value is known (so we can do
> +   constant folding using DECL_INITIAL (decl)).  */
>  
>  bool
> -varpool_decide_const_value_known (struct varpool_node *node)
> +const_value_known_p (tree decl)
>  {
> -  tree decl = node->decl;
> +  struct varpool_node *vnode;
> +
> +  if (TREE_CODE (decl) == PARM_DECL
> +      || TREE_CODE (decl) == RESULT_DECL)
> +    return false;
> +
> +  if (TREE_CODE (decl) == CONST_DECL
> +      || DECL_IN_CONSTANT_POOL (decl))
> +    return true;
>  
> -  gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
>    gcc_assert (TREE_CODE (decl) == VAR_DECL);
> +
>    if (!TREE_READONLY (decl))
>      return false;
> +
> +  /* Gimplifier takes away constructors of local vars  */
> +  if (!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
> +    return DECL_INITIAL (decl) != NULL;
> +
> +  gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
> +
> +  /* In WHOPR mode we can put variable into one partition
> +     and make it external in the other partition.  In this
> +     case we still know the value, but it can't be determined
> +     from DECL flags.  For this reason we keep const_value_known
> +     flag in varpool nodes.  */
> +  if ((vnode = varpool_get_node (decl))
> +      && vnode->const_value_known)
> +    return true;
> +
>    /* Variables declared 'const' without an initializer
>       have zero as the initializer if they may not be
>       overridden at link or run time.  */
> @@ -423,7 +444,7 @@ varpool_finalize_decl (tree decl)
>       there.  */
>    else if (TREE_PUBLIC (decl) && !DECL_COMDAT (decl) && !DECL_EXTERNAL (decl))
>      varpool_mark_needed_node (node);
> -  node->const_value_known |= varpool_decide_const_value_known (node);
> +  node->const_value_known |= const_value_known_p (node->decl);
>    if (cgraph_global_info_ready)
>      varpool_assemble_pending_decls ();
>  }
> 
>

On Mon, Sep 20, 2010 at 6:50 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> I've tested the other part of previous patch and comitted everything except
> for the code introducing folding itself as they address other latent issues.
>
> The problem with folders being called before cgraph&varpool are built is common
> to issue I was addressing by my earlier string_constant patch that was refused
> as too confusing.
>
> Constant constructors are just special case of wider issue that with linker
> plugin or WHOPR we do have more knowledge about what is locally bound than
> in normal compilation.
>
> After discussion with Richard on IRC, we got to conclusion that to address the
> issue we probably should store resolution decisions into varpool and cgraph nodes.
> Varpool's const_value_known and cgraph+varpool used_in_object_file flags can
> then be turned into predicates looking into resolutions and DECL flags. Also
> binds_local_p can be strenghtened to look up resolution info and take it into
> account when dealing with weak, external and comdat decls.
>
> I have patch for this change, but since it is tricky by itself (updates
> binds_local_p logic, adds logic into lto-symtab to bookkeep what values come
> from linker and what was decided in GCC, etc.) I decided to break those
> changes.  This is patch that goes half way by introducing const_value_known_p
> predicate while keeping current way of passing const_value_known flag from WPA
> to LTRANS.
>
> I updated the predicate to work on CONST_DECL, automatic vars, other decls and
> vars w/o varpool entries allocated (yet), so it can be used more conveniently.
> Now when one has non function/method DECL, it is possible use this predicate to
> test whether it makes sense to look at DECL_INITIAL to get the constructor.
> One no longer needs to special case constant pool, CONST_DECL, and check that
> variable is static before attempting to do so.
>
> This finally lets us to do the statement folding during gimplification without
> ICEs.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
>        * cgraph.h (const_value_known_p): Declare.
>        (varpool_decide_const_value_known): Remove.
>        * tree-ssa-ccp.c (get_base_constructor): Use it.
>        * lto-cgraph.c (compute_ltrans_boundary): Likewise.
>        * expr.c (string_constant): Likewise.
>        * tree-ssa-loop-ivcanon.c (constant_after_peeling): Likewise.
>        * ipa.c (ipa_discover_readonly_nonaddressable_var,
>        function_and_variable_visibility): Likewise.
>        * gimplify.c (gimplify_call_expr): Likewise.
>        * gimple-fold.c (get_symbol_constant_value): Likewise.
>        * varpool.c (varpool_decide_const_value_known): Replace by...
>        (const_value_known_p): ... this one; handle other kinds of DECLs
>        too and work for automatic vars.
>        (varpool_finalize_decl): Use const_value_known_p.
>
>        * lto.c (lto_promote_cross_file_statics): Use const_value_known_p.
>
>        * g++.dg/tree-ssa/pr45605.C: New testcase.

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45738

H.J.

On Tue, Sep 21, 2010 at 5:25 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Sep 20, 2010 at 6:50 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi,
>> I've tested the other part of previous patch and comitted everything except
>> for the code introducing folding itself as they address other latent issues.
>>
>> The problem with folders being called before cgraph&varpool are built is common
>> to issue I was addressing by my earlier string_constant patch that was refused
>> as too confusing.
>>
>> Constant constructors are just special case of wider issue that with linker
>> plugin or WHOPR we do have more knowledge about what is locally bound than
>> in normal compilation.
>>
>> After discussion with Richard on IRC, we got to conclusion that to address the
>> issue we probably should store resolution decisions into varpool and cgraph nodes.
>> Varpool's const_value_known and cgraph+varpool used_in_object_file flags can
>> then be turned into predicates looking into resolutions and DECL flags. Also
>> binds_local_p can be strenghtened to look up resolution info and take it into
>> account when dealing with weak, external and comdat decls.
>>
>> I have patch for this change, but since it is tricky by itself (updates
>> binds_local_p logic, adds logic into lto-symtab to bookkeep what values come
>> from linker and what was decided in GCC, etc.) I decided to break those
>> changes.  This is patch that goes half way by introducing const_value_known_p
>> predicate while keeping current way of passing const_value_known flag from WPA
>> to LTRANS.
>>
>> I updated the predicate to work on CONST_DECL, automatic vars, other decls and
>> vars w/o varpool entries allocated (yet), so it can be used more conveniently.
>> Now when one has non function/method DECL, it is possible use this predicate to
>> test whether it makes sense to look at DECL_INITIAL to get the constructor.
>> One no longer needs to special case constant pool, CONST_DECL, and check that
>> variable is static before attempting to do so.
>>
>> This finally lets us to do the statement folding during gimplification without
>> ICEs.
>>
>> Bootstrapped/regtested x86_64-linux, OK?
>>
>> Honza
>>
>>        * cgraph.h (const_value_known_p): Declare.
>>        (varpool_decide_const_value_known): Remove.
>>        * tree-ssa-ccp.c (get_base_constructor): Use it.
>>        * lto-cgraph.c (compute_ltrans_boundary): Likewise.
>>        * expr.c (string_constant): Likewise.
>>        * tree-ssa-loop-ivcanon.c (constant_after_peeling): Likewise.
>>        * ipa.c (ipa_discover_readonly_nonaddressable_var,
>>        function_and_variable_visibility): Likewise.
>>        * gimplify.c (gimplify_call_expr): Likewise.
>>        * gimple-fold.c (get_symbol_constant_value): Likewise.
>>        * varpool.c (varpool_decide_const_value_known): Replace by...
>>        (const_value_known_p): ... this one; handle other kinds of DECLs
>>        too and work for automatic vars.
>>        (varpool_finalize_decl): Use const_value_known_p.
>>
>>        * lto.c (lto_promote_cross_file_statics): Use const_value_known_p.
>>
>>        * g++.dg/tree-ssa/pr45605.C: New testcase.
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45738
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45741

Hi,
this patch fixes both problem (and makes string_constant work on CONST_DECLs too)

Bootstrapped/regetested x86_64-linux, OK (with the testcases added into compile testsuite)

Honza
	* expr.c (string_constant): Accept CONST_DECL too; check that
	DECL_INITIAL is set.
	* varpool.c (const_value_known_p): DEBUG_EXPR_DECL 
	don't have DECL_INITIAL.
Index: expr.c
===================================================================
*** expr.c	(revision 164523)
--- expr.c	(working copy)
*************** string_constant (tree arg, tree *ptr_off
*** 9854,9865 ****
        *ptr_offset = fold_convert (sizetype, offset);
        return array;
      }
!   else if (TREE_CODE (array) == VAR_DECL)
      {
        int length;
  
        /* Variables initialized to string literals can be handled too.  */
        if (!const_value_known_p (array)
  	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
  	return 0;
  
--- 9854,9867 ----
        *ptr_offset = fold_convert (sizetype, offset);
        return array;
      }
!   else if (TREE_CODE (array) == VAR_DECL
! 	   || TREE_CODE (array) != CONST_DECL)
      {
        int length;
  
        /* Variables initialized to string literals can be handled too.  */
        if (!const_value_known_p (array)
+ 	  || !DECL_INITIAL (array)
  	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
  	return 0;
  
Index: varpool.c
===================================================================
*** varpool.c	(revision 164523)
--- varpool.c	(working copy)
*************** const_value_known_p (tree decl)
*** 368,374 ****
    struct varpool_node *vnode;
  
    if (TREE_CODE (decl) == PARM_DECL
!       || TREE_CODE (decl) == RESULT_DECL)
      return false;
  
    if (TREE_CODE (decl) == CONST_DECL
--- 368,375 ----
    struct varpool_node *vnode;
  
    if (TREE_CODE (decl) == PARM_DECL
!       || TREE_CODE (decl) == RESULT_DECL
!       || TREE_CODE (decl) == DEBUG_EXPR_DECL)
      return false;
  
    if (TREE_CODE (decl) == CONST_DECL

On Wed, 22 Sep 2010, Jan Hubicka wrote:

> Hi,
> this patch fixes both problem (and makes string_constant work on CONST_DECLs too)
> 
> Bootstrapped/regetested x86_64-linux, OK (with the testcases added into compile testsuite)
> 
> Honza
> 	* expr.c (string_constant): Accept CONST_DECL too; check that
> 	DECL_INITIAL is set.
> 	* varpool.c (const_value_known_p): DEBUG_EXPR_DECL 
> 	don't have DECL_INITIAL.
> Index: expr.c
> ===================================================================
> *** expr.c	(revision 164523)
> --- expr.c	(working copy)
> *************** string_constant (tree arg, tree *ptr_off
> *** 9854,9865 ****
>         *ptr_offset = fold_convert (sizetype, offset);
>         return array;
>       }
> !   else if (TREE_CODE (array) == VAR_DECL)
>       {
>         int length;
>   
>         /* Variables initialized to string literals can be handled too.  */
>         if (!const_value_known_p (array)
>   	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
>   	return 0;
>   
> --- 9854,9867 ----
>         *ptr_offset = fold_convert (sizetype, offset);
>         return array;
>       }
> !   else if (TREE_CODE (array) == VAR_DECL
> ! 	   || TREE_CODE (array) != CONST_DECL)

Surely a typo.  Ok if a fixed version still works.

Richard.

>       {
>         int length;
>   
>         /* Variables initialized to string literals can be handled too.  */
>         if (!const_value_known_p (array)
> + 	  || !DECL_INITIAL (array)
>   	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
>   	return 0;
>   
> Index: varpool.c
> ===================================================================
> *** varpool.c	(revision 164523)
> --- varpool.c	(working copy)
> *************** const_value_known_p (tree decl)
> *** 368,374 ****
>     struct varpool_node *vnode;
>   
>     if (TREE_CODE (decl) == PARM_DECL
> !       || TREE_CODE (decl) == RESULT_DECL)
>       return false;
>   
>     if (TREE_CODE (decl) == CONST_DECL
> --- 368,375 ----
>     struct varpool_node *vnode;
>   
>     if (TREE_CODE (decl) == PARM_DECL
> !       || TREE_CODE (decl) == RESULT_DECL
> !       || TREE_CODE (decl) == DEBUG_EXPR_DECL)
>       return false;
>   
>     if (TREE_CODE (decl) == CONST_DECL
> 
>

On Wed, Sep 22, 2010 at 04:52:35PM +0200, Jan Hubicka wrote:
> 	* expr.c (string_constant): Accept CONST_DECL too; check that
> 	DECL_INITIAL is set.
> 	* varpool.c (const_value_known_p): DEBUG_EXPR_DECL 
> 	don't have DECL_INITIAL.
> Index: expr.c
> ===================================================================
> *** expr.c	(revision 164523)
> --- expr.c	(working copy)
> *************** string_constant (tree arg, tree *ptr_off
> *** 9854,9865 ****
>         *ptr_offset = fold_convert (sizetype, offset);
>         return array;
>       }
> !   else if (TREE_CODE (array) == VAR_DECL)
>       {
>         int length;
>   
>         /* Variables initialized to string literals can be handled too.  */
>         if (!const_value_known_p (array)
>   	  || TREE_CODE (DECL_INITIAL (array)) != STRING_CST)
>   	return 0;
>   
> --- 9854,9867 ----
>         *ptr_offset = fold_convert (sizetype, offset);
>         return array;
>       }
> !   else if (TREE_CODE (array) == VAR_DECL
> ! 	   || TREE_CODE (array) != CONST_DECL)

Did you mean || TREE_CODE (array) == CONST_DECL ?
Because otherwise it is the same as
else if (TREE_CODE (array) != CONST_DECL)
and the VAR_DECL check there doesn't make any sense.

> *** varpool.c	(revision 164523)
> --- varpool.c	(working copy)
> *************** const_value_known_p (tree decl)
> *** 368,374 ****
>     struct varpool_node *vnode;
>   
>     if (TREE_CODE (decl) == PARM_DECL
> !       || TREE_CODE (decl) == RESULT_DECL)
>       return false;
>   
>     if (TREE_CODE (decl) == CONST_DECL
> --- 368,375 ----
>     struct varpool_node *vnode;
>   
>     if (TREE_CODE (decl) == PARM_DECL
> !       || TREE_CODE (decl) == RESULT_DECL
> !       || TREE_CODE (decl) == DEBUG_EXPR_DECL)
>       return false;

What about other DECL_Ps, like FUNCTION_DECL, LABEL_DECL,
etc., are you sure they never hit this function?

	Jakub

> > *** varpool.c	(revision 164523)
> > --- varpool.c	(working copy)
> > *************** const_value_known_p (tree decl)
> > *** 368,374 ****
> >     struct varpool_node *vnode;
> >   
> >     if (TREE_CODE (decl) == PARM_DECL
> > !       || TREE_CODE (decl) == RESULT_DECL)
> >       return false;
> >   
> >     if (TREE_CODE (decl) == CONST_DECL
> > --- 368,375 ----
> >     struct varpool_node *vnode;
> >   
> >     if (TREE_CODE (decl) == PARM_DECL
> > !       || TREE_CODE (decl) == RESULT_DECL
> > !       || TREE_CODE (decl) == DEBUG_EXPR_DECL)
> >       return false;
> 
> What about other DECL_Ps, like FUNCTION_DECL, LABEL_DECL,
> etc., are you sure they never hit this function?

Hmm, i was thinking of FUNCTION_DECL and concluded that they should not appear
in memory references, but I guess for some sick testcases with some of view
converts they can.

I will change the test to test for VAR_DECL and CONST_DECL only.  Originally I
wanted to have some sanity checking that we are not calling function in wrong
context, but I guess it is safer to just test for decls where DECL_INITIAL is
defined and has intended behaviour.

I updated it in the patch I am testing and intend to commit it once
testing converges.
> 
> 	Jakub

On Tue, Sep 21, 2010 at 10:38 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Sep 21, 2010 at 5:25 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Sep 20, 2010 at 6:50 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> Hi,
>>> I've tested the other part of previous patch and comitted everything except
>>> for the code introducing folding itself as they address other latent issues.
>>>
>>> The problem with folders being called before cgraph&varpool are built is common
>>> to issue I was addressing by my earlier string_constant patch that was refused
>>> as too confusing.
>>>
>>> Constant constructors are just special case of wider issue that with linker
>>> plugin or WHOPR we do have more knowledge about what is locally bound than
>>> in normal compilation.
>>>
>>> After discussion with Richard on IRC, we got to conclusion that to address the
>>> issue we probably should store resolution decisions into varpool and cgraph nodes.
>>> Varpool's const_value_known and cgraph+varpool used_in_object_file flags can
>>> then be turned into predicates looking into resolutions and DECL flags. Also
>>> binds_local_p can be strenghtened to look up resolution info and take it into
>>> account when dealing with weak, external and comdat decls.
>>>
>>> I have patch for this change, but since it is tricky by itself (updates
>>> binds_local_p logic, adds logic into lto-symtab to bookkeep what values come
>>> from linker and what was decided in GCC, etc.) I decided to break those
>>> changes.  This is patch that goes half way by introducing const_value_known_p
>>> predicate while keeping current way of passing const_value_known flag from WPA
>>> to LTRANS.
>>>
>>> I updated the predicate to work on CONST_DECL, automatic vars, other decls and
>>> vars w/o varpool entries allocated (yet), so it can be used more conveniently.
>>> Now when one has non function/method DECL, it is possible use this predicate to
>>> test whether it makes sense to look at DECL_INITIAL to get the constructor.
>>> One no longer needs to special case constant pool, CONST_DECL, and check that
>>> variable is static before attempting to do so.
>>>
>>> This finally lets us to do the statement folding during gimplification without
>>> ICEs.
>>>
>>> Bootstrapped/regtested x86_64-linux, OK?
>>>
>>> Honza
>>>
>>>        * cgraph.h (const_value_known_p): Declare.
>>>        (varpool_decide_const_value_known): Remove.
>>>        * tree-ssa-ccp.c (get_base_constructor): Use it.
>>>        * lto-cgraph.c (compute_ltrans_boundary): Likewise.
>>>        * expr.c (string_constant): Likewise.
>>>        * tree-ssa-loop-ivcanon.c (constant_after_peeling): Likewise.
>>>        * ipa.c (ipa_discover_readonly_nonaddressable_var,
>>>        function_and_variable_visibility): Likewise.
>>>        * gimplify.c (gimplify_call_expr): Likewise.
>>>        * gimple-fold.c (get_symbol_constant_value): Likewise.
>>>        * varpool.c (varpool_decide_const_value_known): Replace by...
>>>        (const_value_known_p): ... this one; handle other kinds of DECLs
>>>        too and work for automatic vars.
>>>        (varpool_finalize_decl): Use const_value_known_p.
>>>
>>>        * lto.c (lto_promote_cross_file_statics): Use const_value_known_p.
>>>
>>>        * g++.dg/tree-ssa/pr45605.C: New testcase.
>>
>> This caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45738
>>
>
> This also caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45741
>

This also caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46360