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

[Richard Guenther <richard dot guenther at gmail dot com>]

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);
> ?}
>