Fix Mozilla build at -O3 --param inline-unit-growth=5

Hi,
Mozilla currently fails to link with LTO at -O3 --param inline-unit-growth=5.
The problem is that constant memory folding folds call into a virtual method,
but the virtual method is no longer available.
This is supposed to be handled by static_object_in_other_unit_p but the catch
is that get_base_address returns NULL on address of FUNCTION_DECL, so we never
actually test it.

While looking at the problem I also noticed one extra corner case when we need
to prevent folding: when we devirtualize call to a COMDAT function at a final
compilation stage when we already decided that the function is not needed.

This patch also adds code handling this case that is done by replacing
static_object_in_other_unit_p by can_refer_decl_in_current_unit_p.

Unforutnately due to nature of the problem, it is very difficult to reduce
a testcase.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* gimple-fold.c (static_object_in_other_unit_p): Rename to...
	(can_refer_decl_in_current_unit_p): ... this one; reverse return
	value; handle comdats too.
	(canonicalize_constructor_val): Use it; handle function_decls
	correctly.
	(gimple_fold_obj_type_ref_known_binfo): Likewise.

On Mon, Oct 4, 2010 at 2:44 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>
> Hi,
> Mozilla currently fails to link with LTO at -O3 --param inline-unit-growth=5.
> The problem is that constant memory folding folds call into a virtual method,
> but the virtual method is no longer available.
> This is supposed to be handled by static_object_in_other_unit_p but the catch
> is that get_base_address returns NULL on address of FUNCTION_DECL, so we never
> actually test it.
>
> While looking at the problem I also noticed one extra corner case when we need
> to prevent folding: when we devirtualize call to a COMDAT function at a final
> compilation stage when we already decided that the function is not needed.
>
> This patch also adds code handling this case that is done by replacing
> static_object_in_other_unit_p by can_refer_decl_in_current_unit_p.
>
> Unforutnately due to nature of the problem, it is very difficult to reduce
> a testcase.

Well, get_base_address is really odd here (I noticed that multiple times
already).  It for example happily returns SSA names but never
CONST_DECLs (and removing the SSA name case has very interesting
fallout ...).

So, I'd rather change the get_base_address tail to

  if (TREE_CODE (t) == SSA_NAME
      || DECL_P (t)
      || TREE_CODE (t) == STRING_CST
      || TREE_CODE (t) == CONSTRUCTOR
      || INDIRECT_REF_P (t)
      || TREE_CODE (t) == MEM_REF
      || TREE_CODE (t) == TARGET_MEM_REF)

than do this kind of odd stuff.

Ok with that change instead (if it actually works of course).

Thanks,
Richard.

> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
>        * gimple-fold.c (static_object_in_other_unit_p): Rename to...
>        (can_refer_decl_in_current_unit_p): ... this one; reverse return
>        value; handle comdats too.
>        (canonicalize_constructor_val): Use it; handle function_decls
>        correctly.
>        (gimple_fold_obj_type_ref_known_binfo): Likewise.
> Index: gimple-fold.c
> ===================================================================
> --- gimple-fold.c       (revision 164917)
> +++ gimple-fold.c       (working copy)
> @@ -31,11 +31,10 @@ along with GCC; see the file COPYING3.
>  #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.
> +/* Return true when DECL can be referenced from current unit.
> +   We can get declarations that are not possible to reference for
> +   various reasons:
>
> -   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.
> @@ -46,45 +45,62 @@ along with GCC; see the file COPYING3.
>        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.  */
> -
> +       declaring the body.
> +     3) COMDAT functions referred by external vtables that
> +        we devirtualize only during final copmilation stage.
> +        At this time we already decided that we will not output
> +        the function body and thus we can't reference the symbol
> +        directly.  */
> +
>  static bool
> -static_object_in_other_unit_p (tree decl)
> +can_refer_decl_in_current_unit_p (tree decl)
>  {
>   struct varpool_node *vnode;
>   struct cgraph_node *node;
>
> -  if (!TREE_STATIC (decl) || DECL_COMDAT (decl))
> -    return false;
> +  if (!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
> +    return true;
>   /* External flag is set, so we deal with C++ reference
>      to static object from other file.  */
> -  if (DECL_EXTERNAL (decl) && TREE_CODE (decl) == VAR_DECL)
> +  if (DECL_EXTERNAL (decl) && TREE_STATIC (decl)
> +      && TREE_CODE (decl) == VAR_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;
> +      return false;
>     }
> -  if (TREE_PUBLIC (decl))
> -    return false;
> +  /* When function is public, we always can introduce new reference.
> +     Exception are the COMDAT functions where introducing a direct
> +     reference imply need to include function body in the curren tunit.  */
> +  if (TREE_PUBLIC (decl) && !DECL_COMDAT (decl))
> +    return true;
>   /* We are not at ltrans stage; so don't worry about WHOPR.  */
> -  if (!flag_ltrans)
> +  if (!flag_ltrans && !DECL_COMDAT (decl))
>     return false;
> +  /* If we already output the function body, we are safe.  */
> +  if (TREE_ASM_WRITTEN (decl))
> +    return true;
>   if (TREE_CODE (decl) == FUNCTION_DECL)
>     {
>       node = cgraph_get_node (decl);
> -      if (!node || !node->analyzed)
> -       return true;
> +      /* Check that we still have function body and that we didn't took
> +         the decision to eliminate offline copy of the function yet.
> +         The second is important when devirtualization happens during final
> +         compilation stage when making a new reference no longer makes callee
> +         to be compiled.  */
> +      if (!node || !node->analyzed || node->global.inlined_to)
> +       return false;
>     }
>   else if (TREE_CODE (decl) == VAR_DECL)
>     {
>       vnode = varpool_get_node (decl);
>       if (!vnode || !vnode->finalized)
> -       return true;
> +       return false;
>     }
> -  return false;
> +  return true;
>  }
>
>  /* CVAL is value taken from DECL_INITIAL of variable.  Try to transorm it into
> @@ -105,11 +121,16 @@ canonicalize_constructor_val (tree cval)
>     }
>   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))
> +      tree base;
> +
> +      /* get_base_address returns NULL for addresses of FUNCTION_DECL.  */
> +      if (TREE_CODE (TREE_OPERAND (cval, 0)) == FUNCTION_DECL
> +         && !can_refer_decl_in_current_unit_p (TREE_OPERAND (cval, 0)))
> +       return NULL_TREE;
> +      else if ((base = get_base_address (TREE_OPERAND (cval, 0)))
> +              && (TREE_CODE (base) == VAR_DECL
> +                  || TREE_CODE (base) == FUNCTION_DECL)
> +              && !can_refer_decl_in_current_unit_p (base))
>        return NULL_TREE;
>       if (base && TREE_CODE (base) == VAR_DECL)
>        add_referenced_var (base);
> @@ -1446,7 +1467,7 @@ gimple_fold_obj_type_ref_known_binfo (HO
>      devirtualize.  This can happen in WHOPR when the actual method
>      ends up in other partition, because we found devirtualization
>      possibility too late.  */
> -  if (static_object_in_other_unit_p (fndecl))
> +  if (!can_refer_decl_in_current_unit_p (fndecl))
>     return NULL;
>   return build_fold_addr_expr (fndecl);
>  }
>