Hi,
this patch fixes correctness issue in
cgraph_node::can_remove_if_no_direct_calls_p and
cgraph_node::will_be_removed_from_program_if_no_direct_calls_p WRT to comdat
groups.
The predicate basically test if only uses of a symbol are calls and if the
visibility prevents inlining. It forgets about comdat group possibly holding
symbols together. This leads to inliner and ICF breaking comdat groups
in some cases.
The patch adds special path to both predicates to walk the whole comdat group
and try to prove that all references are either direct calls to given symbol
or internal to the group and that all symbols can be removed otherwise.
The change to can_remove_node_now_p_1 is just partial, but it will just prevent
some comdat groups from being removed and they are hoepfully rare enough so I
do not need to care abou tthis until next stage1.
I reproduced the ICE only with libreoffice FDO failure with -fdeclone-ctros
-flto.
There is quite a subtle series of events needed to get a wrong code.
It may make sense to backport this to 4.9 but it can't build libreoffice with
FDO and I do not have any other reproducer.
Bootstrapped/regtested x86_64-linux, comitted.
Honza
* cgraph.c (cgraph_node::can_remove_if_no_direct_calls_p): Rewrite
for correct comdat handling.
(cgraph_node::will_be_removed_from_program_if_no_direct_calls_p):
Likewise.
* cgraph.h (call_for_symbol_and_aliases): Fix formating.
(used_from_object_file_p_worker): Remove.
(cgraph_node::only_called_directly_or_alised): Add
used_from_object_file_p.
* ipa-inline-analysis.c (growth_likely_positive): Optimie.
* ipa-inline-transform.c (can_remove_node_now_p_1): Use
can_remove_if_no_direct_calls_and_refs_p.
Index: cgraph.c
===================================================================
--- cgraph.c (revision 221170)
+++ cgraph.c (working copy)
@@ -2411,18 +2411,57 @@ nonremovable_p (cgraph_node *node, void
return !node->can_remove_if_no_direct_calls_and_refs_p ();
}
-/* Return true when function cgraph_node and its aliases can be removed from
- callgraph if all direct calls are eliminated. */
+/* Return true if whole comdat group can be removed if there are no direct
+ calls to THIS. */
bool
cgraph_node::can_remove_if_no_direct_calls_p (void)
{
- /* Extern inlines can always go, we will use the external definition. */
- if (DECL_EXTERNAL (decl))
- return true;
- if (address_taken)
+ struct ipa_ref *ref;
+
+ /* For local symbols or non-comdat group it is the same as
+ can_remove_if_no_direct_calls_p. */
+ if (!externally_visible || !same_comdat_group)
+ {
+ if (DECL_EXTERNAL (decl))
+ return true;
+ if (address_taken)
+ return false;
+ return !call_for_symbol_and_aliases (nonremovable_p, NULL, true);
+ }
+
+ /* Otheriwse check if we can remove the symbol itself and then verify
+ that only uses of the comdat groups are direct call to THIS
+ or its aliases. */
+ if (!can_remove_if_no_direct_calls_and_refs_p ())
return false;
- return !call_for_symbol_and_aliases (nonremovable_p, NULL, true);
+
+ /* Check that all refs come from within the comdat group. */
+ for (int i = 0; iterate_referring (i, ref); i++)
+ if (ref->referring->get_comdat_group () != get_comdat_group ())
+ return false;
+
+ struct cgraph_node *target = ultimate_alias_target ();
+ for (cgraph_node *next = dyn_cast<cgraph_node *> (same_comdat_group);
+ next != this; next = dyn_cast<cgraph_node *> (next->same_comdat_group))
+ {
+ if (!externally_visible)
+ continue;
+ if (!next->alias
+ && !next->can_remove_if_no_direct_calls_and_refs_p ())
+ return false;
+
+ /* If we see different symbol than THIS, be sure to check calls. */
+ if (next->ultimate_alias_target () != target)
+ for (cgraph_edge *e = next->callers; e; e = e->next_caller)
+ if (e->caller->get_comdat_group () != get_comdat_group ())
+ return false;
+
+ for (int i = 0; next->iterate_referring (i, ref); i++)
+ if (ref->referring->get_comdat_group () != get_comdat_group ())
+ return false;
+ }
+ return true;
}
/* Return true when function cgraph_node can be expected to be removed
@@ -2442,19 +2481,47 @@ cgraph_node::can_remove_if_no_direct_cal
bool
cgraph_node::will_be_removed_from_program_if_no_direct_calls_p (void)
{
+ struct ipa_ref *ref;
gcc_assert (!global.inlined_to);
+ if (DECL_EXTERNAL (decl))
+ return true;
- if (call_for_symbol_and_aliases (used_from_object_file_p_worker,
- NULL, true))
- return false;
if (!in_lto_p && !flag_whole_program)
- return only_called_directly_p ();
- else
{
- if (DECL_EXTERNAL (decl))
- return true;
- return can_remove_if_no_direct_calls_p ();
+ /* If the symbol is in comdat group, we need to verify that whole comdat
+ group becomes unreachable. Technically we could skip references from
+ within the group, too. */
+ if (!only_called_directly_p ())
+ return false;
+ if (same_comdat_group && externally_visible)
+ {
+ struct cgraph_node *target = ultimate_alias_target ();
+ for (cgraph_node *next = dyn_cast<cgraph_node *> (same_comdat_group);
+ next != this;
+ next = dyn_cast<cgraph_node *> (next->same_comdat_group))
+ {
+ if (!externally_visible)
+ continue;
+ if (!next->alias
+ && !next->only_called_directly_p ())
+ return false;
+
+ /* If we see different symbol than THIS,
+ be sure to check calls. */
+ if (next->ultimate_alias_target () != target)
+ for (cgraph_edge *e = next->callers; e; e = e->next_caller)
+ if (e->caller->get_comdat_group () != get_comdat_group ())
+ return false;
+
+ for (int i = 0; next->iterate_referring (i, ref); i++)
+ if (ref->referring->get_comdat_group () != get_comdat_group ())
+ return false;
+ }
+ }
+ return true;
}
+ else
+ return can_remove_if_no_direct_calls_p ();
}
Index: cgraph.h
===================================================================
--- cgraph.h (revision 221170)
+++ cgraph.h (working copy)
@@ -258,8 +258,8 @@ public:
When INCLUDE_OVERWRITABLE is false, overwritable aliases and thunks are
skipped. */
bool call_for_symbol_and_aliases (bool (*callback) (symtab_node *, void *),
- void *data,
- bool include_overwrite);
+ void *data,
+ bool include_overwrite);
/* If node can not be interposable by static or dynamic linker to point to
different definition, return this symbol. Otherwise look for alias with
@@ -1187,12 +1187,6 @@ public:
returns cgraph_node::get (DECL). */
static cgraph_node * create_same_body_alias (tree alias, tree decl);
- /* Worker for cgraph_can_remove_if_no_direct_calls_p. */
- static bool used_from_object_file_p_worker (cgraph_node *node, void *)
- {
- return node->used_from_object_file_p ();
- }
-
/* Verify whole cgraph structure. */
static void DEBUG_FUNCTION verify_cgraph_nodes (void);
@@ -2736,6 +2730,7 @@ cgraph_node::only_called_directly_or_ali
&& !DECL_VIRTUAL_P (decl)
&& !DECL_STATIC_CONSTRUCTOR (decl)
&& !DECL_STATIC_DESTRUCTOR (decl)
+ && !used_from_object_file_p ()
&& !externally_visible);
}
Index: ipa-inline-analysis.c
===================================================================
--- ipa-inline-analysis.c (revision 221170)
+++ ipa-inline-analysis.c (working copy)
@@ -4007,6 +4007,8 @@ growth_likely_positive (struct cgraph_no
struct cgraph_edge *e;
gcc_checking_assert (edge_growth > 0);
+ if (DECL_EXTERNAL (node->decl))
+ return true;
/* Unlike for functions called once, we play unsafe with
COMDATs. We can allow that since we know functions
in consideration are small (and thus risk is small) and
@@ -4014,18 +4016,13 @@ growth_likely_positive (struct cgraph_no
functions may or may not disappear when eliminated from
current unit. With good probability making aggressive
choice in all units is going to make overall program
- smaller.
-
- Consequently we ask cgraph_can_remove_if_no_direct_calls_p
- instead of
- cgraph_will_be_removed_from_program_if_no_direct_calls */
- if (DECL_EXTERNAL (node->decl)
- || !node->can_remove_if_no_direct_calls_p ())
- return true;
-
- if (!node->will_be_removed_from_program_if_no_direct_calls_p ()
- && (!DECL_COMDAT (node->decl)
- || !node->can_remove_if_no_direct_calls_p ()))
+ smaller. */
+ if (DECL_COMDAT (node->decl))
+ {
+ if (!node->can_remove_if_no_direct_calls_p ())
+ return true;
+ }
+ else if (!node->will_be_removed_from_program_if_no_direct_calls_p ())
return true;
max_callers = inline_summaries->get (node)->size * 4 / edge_growth + 2;
Index: ipa-inline-transform.c
===================================================================
--- ipa-inline-transform.c (revision 221170)
+++ ipa-inline-transform.c (working copy)
@@ -112,9 +112,12 @@ can_remove_node_now_p_1 (struct cgraph_n
}
/* FIXME: When address is taken of DECL_EXTERNAL function we still
can remove its offline copy, but we would need to keep unanalyzed node in
- the callgraph so references can point to it. */
+ the callgraph so references can point to it.
+
+ Also for comdat group we can ignore references inside a group as we
+ want to prove the group as a whole to be dead. */
return (!node->address_taken
- && node->can_remove_if_no_direct_calls_p ()
+ && node->can_remove_if_no_direct_calls_and_refs_p ()
/* Inlining might enable more devirtualizing, so we want to remove
those only after all devirtualizable virtual calls are processed.
Lacking may edges in callgraph we just preserve them post
