Message ID | 20101004004418.GD8569@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
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); > } >
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); }