Re: PR tree-optimization/45605

[Richard Guenther <rguenther at suse dot de>]

On Fri, 17 Sep 2010, Jan Hubicka wrote:

> Hi,
> this patch fixes missed devirtualization seen in the attached testcase.
> (shamelessly stolen from the testsuite).
> 
> The problem is that we never actually try to fold the statement at gimple level
> and thus we never try gimple_fold_obj_type_ref_known_binfo.  Adding it to
> gimple_fold_obj_type_ref_known_binfo does not solve the problem since
> gimple_fold_obj_type_ref_known_binfo.  returns NULL.
> 
> This is because of code I added into it to prevent referring static function
> from other ltrans units.  We call the function too early and cgraph is not built
> yet and consequently it thinks function is not there.
> 
> This patch adds static_object_in_other_unit_p that has fixed logic of detecting
> this case and combine it with the other problem I encounter with my folding patch,
> where we pick up values from external vtables that reffer to external static variables.
> C++ represent these with EXTERN and STATIC flags together.
> 
> The patch saves whopping 300 bytes on Mozilla binnary ;)
> 
> Bootstrapped/regtested x86_64-linux, OK?

Ok.

Thanks,
Richard.

> 
> /* { dg-do compile } */
> /* { dg-options "-O1 -fdump-tree-ssa" } */
> 
> extern "C" { void abort(); }
> 
> struct A
> {
>   int d;
> 
>   A ()                     { d = 123; }
>   A (const A & o)          { d = o.d;  }
>   A (volatile const A & o) { d = o.d + 2; }
> };
> 
> A bar()
> {
>   volatile A l;
>   return l;
> }
> 
> main()
> {
>   A a = bar ();
> 
>   if (a.d != 125)
>     abort();
> 
>   return 0;
> }
> /* We should devirtualize call to D::Run */
> /* { dg-final { scan-tree-dump-times "D::Run (" 1 "ssa"} } */
> /* { dg-final { cleanup-tree-dump "ssa" } } */
> 
> 2010-09-17  Jan Hubicka  <jh@suse.cz>
> 	    Martin Jambor <mjabor@suse.cz>
> 	    Richard Guenther <rguenther@suse.de>
> 
> 	* gimple-fold.c (static_object_in_other_unit_p): New function.
> 	(canonicalize_constructor_val): Use it.
> 	(gimple_fold_obj_type_ref_known_binfo): Likewise.
> 	* gimplify.c (gimplify_call_expr): Use fold_stmt.
> 	* cgraphunit.c (cgraph_analyze_function): Allocate bitmap obstack.
> Index: gimple-fold.c
> ===================================================================
> --- gimple-fold.c	(revision 164344)
> +++ gimple-fold.c	(working copy)
> @@ -31,6 +31,60 @@
>  #include "tree-ssa-propagate.h"
>  #include "target.h"
>  
> +/* Return true when DECL is static object in other partition.
> +   In that case we must prevent folding as we can't refer to
> +   the symbol.
> +
> +   We can get into it in two ways:
> +     1) when analyzing C++ virtual tables.
> +	C++ virtual tables do have known constructors even
> +	when they are keyed to other compilation unit.
> +	Those tables can contain pointers to methods and vars
> +	in other units.  Those methods have both STATIC and EXTERNAL
> +	set.
> +     2) In WHOPR mode devirtualization might lead to reference
> +	to method that was partitioned elsehwere.
> +	In this case we have static VAR_DECL or FUNCTION_DECL
> +	that has no corresponding callgraph/varpool node
> +	declaring the body.  */
> +	
> +static bool
> +static_object_in_other_unit_p (tree decl)
> +{
> +  struct varpool_node *vnode;
> +  struct cgraph_node *node;
> +  if (!TREE_STATIC (decl)
> +      || TREE_PUBLIC (decl) || DECL_COMDAT (decl))
> +    return false;
> +  /* External flag is set, so we deal with C++ reference
> +     to static object from other file.  */
> +  if (DECL_EXTERNAL (decl))
> +    {
> +      /* Just be sure it is not big in frontend setting
> +	 flags incorrectly.  Those variables should never
> +	 be finalized.  */
> +      gcc_checking_assert (!(vnode = varpool_get_node (decl))
> +			   || !vnode->finalized);
> +      return true;
> +    }
> +  /* We are not at ltrans stage; so don't worry about WHOPR.  */
> +  if (!flag_ltrans)
> +    return false;
> +  if (TREE_CODE (decl) == FUNCTION_DECL)
> +    {
> +      node = cgraph_get_node (decl);
> +      if (!node || !node->analyzed)
> +	return true;
> +    }
> +  else if (TREE_CODE (decl) == VAR_DECL)
> +    {
> +      vnode = varpool_get_node (decl);
> +      if (!vnode || !vnode->finalized)
> +	return true;
> +    }
> +  return false;
> +}
> +
>  /* CVAL is value taken from DECL_INITIAL of variable.  Try to transorm it into
>     acceptable form for is_gimple_min_invariant.   */
>  
> @@ -50,6 +104,11 @@
>    if (TREE_CODE (cval) == ADDR_EXPR)
>      {
>        tree base = get_base_address (TREE_OPERAND (cval, 0));
> +      if (base
> +	  && (TREE_CODE (base) == VAR_DECL
> +	      || TREE_CODE (base) == FUNCTION_DECL)
> +	  && static_object_in_other_unit_p (base))
> + 	return NULL;
>        if (base && TREE_CODE (base) == VAR_DECL)
>  	add_referenced_var (base);
>      }
> @@ -1370,7 +1428,6 @@
>  {
>    HOST_WIDE_INT i;
>    tree v, fndecl;
> -  struct cgraph_node *node;
>  
>    v = BINFO_VIRTUALS (known_binfo);
>    i = 0;
> @@ -1382,13 +1439,11 @@
>      }
>  
>    fndecl = TREE_VALUE (v);
> -  node = cgraph_get_node (fndecl);
>    /* When cgraph node is missing and function is not public, we cannot
>       devirtualize.  This can happen in WHOPR when the actual method
>       ends up in other partition, because we found devirtualization
>       possibility too late.  */
> -  if ((!node || (!node->analyzed && !node->in_other_partition))
> -      && (!TREE_PUBLIC (fndecl) || DECL_COMDAT (fndecl)))
> +  if (static_object_in_other_unit_p (fndecl))
>      return NULL;
>    return build_fold_addr_expr (fndecl);
>  }
> Index: gimplify.c
> ===================================================================
> --- gimplify.c	(revision 164344)
> +++ gimplify.c	(working copy)
> @@ -2479,8 +2479,11 @@
>      {
>        /* 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: cgraphunit.c
> ===================================================================
> --- cgraphunit.c	(revision 164344)
> +++ cgraphunit.c	(working copy)
> @@ -861,6 +867,7 @@
>    static struct varpool_node *first_analyzed_var;
>    struct cgraph_node *node, *next;
>  
> +  bitmap_obstack_initialize (NULL);
>    process_function_and_variable_attributes (first_processed,
>  					    first_analyzed_var);
>    first_processed = cgraph_nodes;
> @@ -971,6 +978,7 @@
>        fprintf (cgraph_dump_file, "\n\nReclaimed ");
>        dump_cgraph (cgraph_dump_file);
>      }
> +  bitmap_obstack_release (NULL);
>    first_analyzed = cgraph_nodes;
>    ggc_collect ();
>  }
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex