Improve handling of COMDAT vtables and virtual functoins
Richard Guenther
richard.guenther@gmail.com
Tue Oct 26 21:27:00 GMT 2010
On Tue, Oct 26, 2010 at 3:33 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> during the summit it turned out that virtual functions, despite the fact that their
> address is taken, can actually be unshared since the equivalence comparsion on pointers
> to virtual functions is not possible.
> Similarly for the virtual function tables.
>
> This patch excends the localization we already do for COMDATs with no address taken
> to handle these cases too.
> It turns out that same vtable can be extern in some units and comdat in others. This leads
> to problem because the linker does not see the comdat definition of the functoin and thus
> it resolves it as undefined. Our resolution code is bypassed then and we miss the vtable.
> This scenario is avoided by change in lto_symtab_resolve_symbols to do our own reoslution
> when linker didn't provided prevailing definition.
>
> Bootstrapped/regtested x86_64-linux. The patch brings some Firefox code size improvement
> (not overly large one, about 0.5%). OK?
THe lto-symtab.c change doesn't build:
> }
> ! /* If linker plugin also provided us resolution info and failed, keep info
> ! it provided. */
> ! if (((lto_symtab_entry_t) *slot)->resolution != LDPR_UNKNOWN)
> ! return;
> !
> ! if (prevailing
Missing closing paren.
Ok if it bootstraps&tests with that fixed.
Thanks,
Richard.
> goto found;
>
> /* Do a second round choosing one from the replaceable prevailing decls. */
> Index: cgraph.h
> ===================================================================
> *** cgraph.h (revision 165978)
> --- cgraph.h (working copy)
> *************** void varpool_node_set_remove (varpool_no
> *** 695,700 ****
> --- 695,701 ----
> void dump_varpool_node_set (FILE *, varpool_node_set);
> void debug_varpool_node_set (varpool_node_set);
> void ipa_discover_readonly_nonaddressable_vars (void);
> + bool cgraph_comdat_node_localizable (struct cgraph_node *);
>
> /* In predict.c */
> bool cgraph_maybe_hot_edge_p (struct cgraph_edge *e);
> Index: lto-streamer-out.c
> ===================================================================
> *** lto-streamer-out.c (revision 165978)
> --- lto-streamer-out.c (working copy)
> *************** produce_symtab (struct output_block *ob,
> *** 2487,2493 ****
> if (DECL_EXTERNAL (node->decl))
> continue;
> if (DECL_COMDAT (node->decl)
> ! && cgraph_can_remove_if_no_direct_calls_p (node))
> continue;
> if (node->alias || node->global.inlined_to)
> continue;
> --- 2487,2493 ----
> if (DECL_EXTERNAL (node->decl))
> continue;
> if (DECL_COMDAT (node->decl)
> ! && cgraph_comdat_node_localizable (node))
> continue;
> if (node->alias || node->global.inlined_to)
> continue;
> *************** produce_symtab (struct output_block *ob,
> *** 2501,2507 ****
> if (!DECL_EXTERNAL (node->decl))
> continue;
> if (DECL_COMDAT (node->decl)
> ! && cgraph_can_remove_if_no_direct_calls_p (node))
> continue;
> if (node->alias || node->global.inlined_to)
> continue;
> --- 2501,2507 ----
> if (!DECL_EXTERNAL (node->decl))
> continue;
> if (DECL_COMDAT (node->decl)
> ! && cgraph_comdat_node_localizable (node))
> continue;
> if (node->alias || node->global.inlined_to)
> continue;
> *************** produce_symtab (struct output_block *ob,
> *** 2516,2521 ****
> --- 2516,2527 ----
> vnode = lto_varpool_encoder_deref (varpool_encoder, i);
> if (DECL_EXTERNAL (vnode->decl))
> continue;
> + /* COMDAT virtual tables can be unshared. Do not declare them
> + in the LTO symbol table to prevent linker from forcing them
> + into the output. */
> + if (DECL_COMDAT (vnode->decl)
> + && (!vnode->finalized || DECL_VIRTUAL_P (vnode->decl)))
> + continue;
> if (vnode->alias)
> continue;
> write_symbol (cache, &stream, vnode->decl, seen, false);
> *************** produce_symtab (struct output_block *ob,
> *** 2527,2532 ****
> --- 2533,2541 ----
> vnode = lto_varpool_encoder_deref (varpool_encoder, i);
> if (!DECL_EXTERNAL (vnode->decl))
> continue;
> + if (DECL_COMDAT (vnode->decl)
> + && (!vnode->finalized || DECL_VIRTUAL_P (vnode->decl)))
> + continue;
> if (vnode->alias)
> continue;
> write_symbol (cache, &stream, vnode->decl, seen, false);
> Index: ipa.c
> ===================================================================
> *** ipa.c (revision 165978)
> --- ipa.c (working copy)
> *************** ipa_discover_readonly_nonaddressable_var
> *** 598,603 ****
> --- 598,633 ----
> fprintf (dump_file, "\n");
> }
>
> + /* COMDAT functions must be shared only if they have address taken,
> + otherwise we can produce our own private implementation with
> + -fwhole-program.
> + Return true when turning COMDAT functoin static can not lead to wrong
> + code when the resulting object links with a library defining same COMDAT.
> +
> + Virtual functions do have their addresses taken from the vtables,
> + but in C++ there is no way to compare their addresses for equality. */
> +
> + bool
> + cgraph_comdat_node_localizable (struct cgraph_node *node)
> + {
> + if ((node->address_taken && !DECL_VIRTUAL_P (node->decl))
> + || !node->analyzed)
> + return false;
> + if (node->same_comdat_group)
> + {
> + struct cgraph_node *next;
> +
> + /* If more than one function is in the same COMDAT group, it must
> + be shared even if just one function in the comdat group has
> + address taken. */
> + for (next = node->same_comdat_group;
> + next != node; next = next->same_comdat_group)
> + if (next->address_taken && !DECL_VIRTUAL_P (next->decl))
> + return false;
> + }
> + return true;
> + }
> +
> /* Return true when function NODE should be considered externally visible. */
>
> static bool
> *************** cgraph_externally_visible_p (struct cgra
> *** 623,628 ****
> --- 653,667 ----
> if (lookup_attribute ("externally_visible", DECL_ATTRIBUTES (node->decl)))
> return true;
>
> + /* When doing LTO or whole program, we can bring COMDAT functoins static.
> + This improves code quality and we know we will duplicate them at most twice
> + (in the case that we are not using plugin and link with object file
> + implementing same COMDAT) */
> + if ((in_lto_p || whole_program)
> + && DECL_COMDAT (node->decl)
> + && cgraph_comdat_node_localizable (node))
> + return false;
> +
> /* See if we have linker information about symbol not being used or
> if we need to make guess based on the declaration.
>
> *************** cgraph_externally_visible_p (struct cgra
> *** 645,671 ****
> ;
> else if (!whole_program)
> return true;
> - /* COMDAT functions must be shared only if they have address taken,
> - otherwise we can produce our own private implementation with
> - -fwhole-program. */
> - else if (DECL_COMDAT (node->decl))
> - {
> - if (node->address_taken || !node->analyzed)
> - return true;
> - if (node->same_comdat_group)
> - {
> - struct cgraph_node *next;
> -
> - /* If more than one function is in the same COMDAT group, it must
> - be shared even if just one function in the comdat group has
> - address taken. */
> - for (next = node->same_comdat_group;
> - next != node;
> - next = next->same_comdat_group)
> - if (next->address_taken || !next->analyzed)
> - return true;
> - }
> - }
>
> if (MAIN_NAME_P (DECL_NAME (node->decl)))
> return true;
> --- 684,689 ----
> *************** varpool_externally_visible_p (struct var
> *** 711,716 ****
> --- 729,743 ----
> if (!alias && vnode->resolution == LDPR_PREVAILING_DEF_IRONLY)
> return false;
>
> + /* As a special case, the COMDAT virutal tables can be unshared.
> + In LTO mode turn vtables into static variables. The variable is readonly,
> + so this does not enable more optimization, but referring static var
> + is faster for dynamic linking. Also this match logic hidding vtables
> + from LTO symbol tables. */
> + if ((in_lto_p || flag_whole_program)
> + && DECL_COMDAT (vnode->decl) && DECL_VIRTUAL_P (vnode->decl))
> + return false;
> +
> /* When doing link time optimizations, hidden symbols become local. */
> if (in_lto_p
> && (DECL_VISIBILITY (vnode->decl) == VISIBILITY_HIDDEN
>
More information about the Gcc-patches
mailing list