* ipa-icf::merge TLC
@ 2015-02-25 9:03 Jan Hubicka
2015-02-25 13:10 ` Markus Trippelsdorf
2015-02-26 16:46 ` Jack Howarth
0 siblings, 2 replies; 26+ messages in thread
From: Jan Hubicka @ 2015-02-25 9:03 UTC (permalink / raw)
To: gcc-patches, mliska, jakub
Hi,
this patch reorganize sem_function::merge and sem_variable::merge.
I read the code in detail and found several issues that are fixed in the
following patch.
1) The logic whether address matters was wrong and ignored symbol aliases.
I separated it into symtab_node::address_matters predicate and also
added special case for C++ cdtors that are already handled specially
by ipa-visibility.
2) Turning definition to alias will result in aliases of aliases.
Because we normally do not output them, I think it is better to
flatten the structure or we may run into interesting target issues
with non-GNU assemblers.
I fixed it in symtab_node::resolve_alias.
3) check in cgraph_edge::verify_corresponds_to_fndecl used fndecl instead
of callee's flag that is useless because fndecl may have its symbol
removed.
This was hacked around in sem_function::merge by applying change
immediately that however breaks with WPA.
4) Redirection of callers ignored existence of aliases
Fixed by introduction of redirect_all_callers function. I did not reuse
similar code in clone redirection because semantic is bit different WRT
interposable aliases.
5) binds_to_local_def_p introduced to fix PR64146 is wrong; it possible
to merge interposable functions (by introducing thunk with local alias).
What is wrong with the testcase is that sem equality think they are
equivalent just because the interposable callees are equivalent.
For everything interposable sem equality should resort to comparing
functions by symtab_node::semantically_equivalent_p
The testcase can be turned into wrong code again by making the wrapper
functions non-interposable.
Because Martin has similar patch on the way I decided to not fix this
issue, so the patch currently breaks the testcase.
I would like to stop snowballing here. Either I will XFAIL the testcase
momentarily or Martin will get his patch in tomorrow.
6) logic about discarded symbols had weird code:
- if (original->resolution == LDPR_PREEMPTED_REG
- || original->resolution == LDPR_PREEMPTED_IR)
- original_discardable = true;
It took me a while to remember why it is there. Basically it is about
the case where ORIGINAL definition is known to not appear in the final
binary, because linker preemted it by something else.
In this case introducing an alias is counter-effective and should be
avoided.
Martin: It would be nice if the ORIGINAL was always chosen in a way
that it binds_to_local_def if possible.
8) Thunks should not be produced for empty functions and such.
9) Logic about COMDAT groups was quite wrong - basically it is possible
to unify within groups but extra care needs to be taken to unify across
groups
10) There is no need to always give up when local alias of original can
not be created
11) ALIAS is interposable, it is still possible to turn it into a thunk
and keep interpositions correct
12) I merged diagnostics to be all "Not unifying;" or "Unified;"
13) sem_variable::merge diverged somewhat from function version and
had strange code preventing alias cycles that should not fire because
we should not be merging aliases, only their targets.
Code also disabled itself with -fvariable-sections for no reason.
this simplifies grepping.
14) As noticed by Jakub, it does not make that much sense to do
just partial redirection when orignal function is going to stay
for other reason.
I tested the patch on x86_64-linux and on LTO firefox.
I also tested LTO firefox with symbol aliases disabled (to immitate darwin)
and plan to regtest with this flag.
Finally I tried to reorganize the code so it is easier to follow. First the
function decides whether alias is possible and if not it check conditions that
makes wrapper/redirection possible. If everything fails it reports reason and
terminates. After all checking it finally performs changes.
I plan to commit after some further testing tomorrow and having chance
Martin to look across the changes and discuss 5).
Honza
PR ipa/65150
* ipa-icf.c (redirect_all_callers): New function.
(sem_function::merge): Reorganize and fix merging issues.
(sem_variable::merge): Likewise.
(sem_variable::compare_sections): Remove.
* symtab.c (symtab_node::resolve_alias): When alias has aliases,
redirect them.
(address_matters_1): New function.
(symtab_node::address_taken_from_non_vtable_p): Move here from
ipa-visibility.
(symtab_node::address_matters_p): New function.
* cgraph.c (cgraph_edge::verify_corresponds_to_fndecl): Fix
check for merged flag.
* cgraph.h (address_matters_p): Declare.
* ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p):
Remove.
(update_vtable_references): Fix formating.
Index: symtab.c
===================================================================
--- symtab.c (revision 220956)
+++ symtab.c (working copy)
@@ -1512,6 +1512,19 @@ symtab_node::resolve_alias (symtab_node
/* If alias has address taken, so does the target. */
if (address_taken)
target->ultimate_alias_target ()->address_taken = true;
+
+ /* All non-weakref aliases of THIS are now in fact aliases of TARGET. */
+ ipa_ref *ref;
+ for (unsigned i = 0; iterate_direct_aliases (i, ref);)
+ {
+ struct symtab_node *alias_alias = ref->referring;
+ if (!alias_alias->weakref)
+ {
+ alias_alias->remove_all_references ();
+ alias_alias->create_reference (target, IPA_REF_ALIAS, NULL);
+ }
+ else i++;
+ }
return true;
}
@@ -1862,3 +1875,51 @@ symtab_node::call_for_symbol_and_aliases
}
return false;
}
+
+/* Return ture if address of N is possibly compared. */
+
+static bool
+address_matters_1 (symtab_node *n, void *)
+{
+ if (DECL_VIRTUAL_P (n->decl))
+ return false;
+ if (is_a <cgraph_node *> (n)
+ && (DECL_CXX_CONSTRUCTOR_P (n->decl)
+ || DECL_CXX_DESTRUCTOR_P (n->decl)))
+ return false;
+ if (n->externally_visible
+ || n->symtab_node::address_taken_from_non_vtable_p ())
+ return true;
+ return false;
+}
+
+/* Return true when there is a reference to node and it is not vtable. */
+
+bool
+symtab_node::address_taken_from_non_vtable_p (void)
+{
+ int i;
+ struct ipa_ref *ref = NULL;
+
+ for (i = 0; iterate_referring (i, ref); i++)
+ if (ref->use == IPA_REF_ADDR)
+ {
+ varpool_node *node;
+ if (is_a <cgraph_node *> (ref->referring))
+ return true;
+ node = dyn_cast <varpool_node *> (ref->referring);
+ if (!DECL_VIRTUAL_P (node->decl))
+ return true;
+ }
+ return false;
+}
+
+/* Return true if symbol's address may possibly be compared to other
+ symbol's address. */
+
+bool
+symtab_node::address_matters_p ()
+{
+ gcc_assert (!alias);
+ return call_for_symbol_and_aliases (address_matters_1, NULL, true);
+}
Index: cgraph.c
===================================================================
--- cgraph.c (revision 220956)
+++ cgraph.c (working copy)
@@ -2630,7 +2630,7 @@ cgraph_edge::verify_corresponds_to_fndec
if (!node
|| node->body_removed
|| node->in_other_partition
- || node->icf_merged
+ || callee->icf_merged
|| callee->in_other_partition)
return false;
Index: cgraph.h
===================================================================
--- cgraph.h (revision 220956)
+++ cgraph.h (working copy)
@@ -337,6 +337,10 @@ public:
return 2 otherwise. */
int equal_address_to (symtab_node *s2);
+ /* Return true if symbol's address may possibly be compared to other
+ symbol's address. */
+ bool address_matters_p ();
+
/* Return symbol table node associated with DECL, if any,
and NULL otherwise. */
static inline symtab_node *get (const_tree decl)
Index: ipa-icf.c
===================================================================
--- ipa-icf.c (revision 220956)
+++ ipa-icf.c (working copy)
@@ -594,8 +594,46 @@ set_local (cgraph_node *node, void *data
return false;
}
+/* Redirect all callers of N and its aliases to TO. Remove aliases if
+ possible. Return number of redirections made. */
+
+static int
+redirect_all_callers (cgraph_node *n, cgraph_node *to)
+{
+ int nredirected = 0;
+ ipa_ref *ref;
+
+ while (n->callers)
+ {
+ cgraph_edge *e = n->callers;
+ e->redirect_callee (to);
+ nredirected++;
+ }
+ for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
+ {
+ bool removed = false;
+ cgraph_node *n_alias = dyn_cast <cgraph_node *> (ref->referring);
+
+ if ((DECL_COMDAT_GROUP (n->decl)
+ && (DECL_COMDAT_GROUP (n->decl)
+ == DECL_COMDAT_GROUP (n_alias->decl)))
+ || (n_alias->get_availability () > AVAIL_INTERPOSABLE
+ && n->get_availability () > AVAIL_INTERPOSABLE))
+ {
+ nredirected += redirect_all_callers (n_alias, to);
+ if (n_alias->can_remove_if_no_direct_calls_p ()
+ && !n_alias->has_aliases_p ())
+ n_alias->remove ();
+ }
+ if (!removed)
+ i++;
+ }
+ return nredirected;
+}
+
/* Merges instance with an ALIAS_ITEM, where alias, thunk or redirection can
be applied. */
+
bool
sem_function::merge (sem_item *alias_item)
{
@@ -604,16 +642,19 @@ sem_function::merge (sem_item *alias_ite
sem_function *alias_func = static_cast<sem_function *> (alias_item);
cgraph_node *original = get_node ();
- cgraph_node *local_original = original;
+ cgraph_node *local_original = NULL;
cgraph_node *alias = alias_func->get_node ();
- bool original_address_matters;
- bool alias_address_matters;
- bool create_thunk = false;
+ bool create_wrapper = false;
bool create_alias = false;
bool redirect_callers = false;
+ bool remove = false;
+
bool original_discardable = false;
+ bool original_address_matters = original->address_matters_p ();
+ bool alias_address_matters = original->address_matters_p ();
+
/* Do not attempt to mix functions from different user sections;
we do not know what user intends with those. */
if (((DECL_SECTION_NAME (original->decl) && !original->implicit_section)
@@ -622,123 +663,165 @@ sem_function::merge (sem_item *alias_ite
{
if (dump_file)
fprintf (dump_file,
- "Not unifying; original and alias are in different sections.\n\n");
+ "Not unifying; "
+ "original and alias are in different sections.\n\n");
return false;
}
/* See if original is in a section that can be discarded if the main
- symbol is not used. */
- if (DECL_EXTERNAL (original->decl))
- original_discardable = true;
- if (original->resolution == LDPR_PREEMPTED_REG
- || original->resolution == LDPR_PREEMPTED_IR)
- original_discardable = true;
- if (original->can_be_discarded_p ())
+ symbol is not used.
+
+ Also consider case where we have resolution info and we know that
+ original's definition is not going to be used. In this case we can not
+ create alias to original. */
+ if (original->can_be_discarded_p ()
+ || (node->resolution != LDPR_UNKNOWN
+ && !decl_binds_to_current_def_p (node->decl)))
original_discardable = true;
- /* See if original and/or alias address can be compared for equality. */
- original_address_matters
- = (!DECL_VIRTUAL_P (original->decl)
- && (original->externally_visible
- || original->address_taken_from_non_vtable_p ()));
- alias_address_matters
- = (!DECL_VIRTUAL_P (alias->decl)
- && (alias->externally_visible
- || alias->address_taken_from_non_vtable_p ()));
-
- /* If alias and original can be compared for address equality, we need
- to create a thunk. Also we can not create extra aliases into discardable
- section (or we risk link failures when section is discarded). */
- if ((original_address_matters
- && alias_address_matters)
+ /* Creating a symtab alias is the optimal way to merge.
+ It however can not be used in the following cases:
+
+ 1) if ORIGINAL and ALIAS may be possibly compared for address equality.
+ 2) if ORIGINAL is in a section that may be discarded by linker or if
+ it is an external functions where we can not create an alias
+ (ORIGINAL_DISCARDABLE)
+ 3) if target do not support symbol aliases.
+
+ If we can not produce alias, we will turn ALIAS into WRAPPER of ORIGINAL
+ and/or redirect all callers from ALIAS to ORIGINAL. */
+ if ((original_address_matters && alias_address_matters)
|| original_discardable
- || DECL_COMDAT_GROUP (alias->decl)
|| !sem_item::target_supports_symbol_aliases_p ())
{
- create_thunk = !stdarg_p (TREE_TYPE (alias->decl));
- create_alias = false;
- /* When both alias and original are not overwritable, we can save
- the extra thunk wrapper for direct calls. */
+ /* First see if we can produce wrapper. */
+
+ /* Do not turn function in one comdat group into wrapper to another
+ comdat group; other compiler producing the body of the
+ another comdat group may make opossite decision and with unfortunate
+ linker choices this may close a loop. */
+ if (DECL_COMDAT_GROUP (alias->decl)
+ && (DECL_COMDAT_GROUP (alias->decl)
+ != DECL_COMDAT_GROUP (original->decl)))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Wrapper cannot be created because of COMDAT\n");
+ }
+ else if (DECL_STATIC_CHAIN (alias->decl))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Can not create wrapper of nested functions.\n");
+ }
+ /* TODO: We can also deal with variadic functions never calling
+ VA_START. */
+ else if (stdarg_p (TREE_TYPE (alias->decl)))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "can not create wrapper of stdarg function.\n");
+ }
+ else if (inline_summaries
+ && inline_summaries->get (alias)->self_size <= 2)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Wrapper creation is not "
+ "profitable (function is too small).\n");
+ }
+ else
+ create_wrapper = true;
+
+ /* We can redirect local calls in the case both alias and orignal
+ are not interposable. */
redirect_callers
- = (!original_discardable
- && !DECL_COMDAT_GROUP (alias->decl)
- && alias->get_availability () > AVAIL_INTERPOSABLE
- && original->get_availability () > AVAIL_INTERPOSABLE
- && !alias->instrumented_version);
- }
- else
- {
- create_alias = true;
- create_thunk = false;
- redirect_callers = false;
- }
+ = alias->get_availability () > AVAIL_INTERPOSABLE
+ && original->get_availability () > AVAIL_INTERPOSABLE
+ && !alias->instrumented_version;
- /* We want thunk to always jump to the local function body
- unless the body is comdat and may be optimized out. */
- if ((create_thunk || redirect_callers)
- && (!original_discardable
+ if (!redirect_callers && !create_wrapper)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; can not redirect callers nor "
+ "produce wrapper\n\n");
+ return false;
+ }
+
+ /* Work out the symbol the wrapper should call.
+ If ORIGINAL is interposable, we need to call a local alias.
+ Also produce local alias (if possible) as an optimization. */
+ if (!original_discardable
|| (DECL_COMDAT_GROUP (original->decl)
&& (DECL_COMDAT_GROUP (original->decl)
- == DECL_COMDAT_GROUP (alias->decl)))))
- local_original
- = dyn_cast <cgraph_node *> (original->noninterposable_alias ());
-
- if (!local_original)
- {
- if (dump_file)
- fprintf (dump_file, "Noninterposable alias cannot be created.\n\n");
-
- return false;
- }
+ == DECL_COMDAT_GROUP (alias->decl))))
+ {
+ local_original
+ = dyn_cast <cgraph_node *> (original->noninterposable_alias ());
+ if (!local_original
+ && original->get_availability () > AVAIL_INTERPOSABLE)
+ local_original = original;
+ /* If original is COMDAT local, we can not really redirect external
+ callers to it. */
+ if (original->comdat_local_p ())
+ redirect_callers = false;
+ }
+ /* If we can not use local alias, fallback to the original
+ when possible. */
+ else if (original->get_availability () > AVAIL_INTERPOSABLE)
+ local_original = original;
+ if (!local_original)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; "
+ "can not produce local alias.\n\n");
+ return false;
+ }
- if (!decl_binds_to_current_def_p (alias->decl))
- {
- if (dump_file)
- fprintf (dump_file, "Declaration does not bind to currect definition.\n\n");
- return false;
+ if (!redirect_callers && !create_wrapper)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; "
+ "can not redirect callers nor produce a wrapper\n\n");
+ return false;
+ }
+ if (!create_wrapper
+ && !alias->can_remove_if_no_direct_calls_p ())
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; can not make wrapper and "
+ "function has other uses than direct calls\n\n");
+ return false;
+ }
}
+ else
+ create_alias = true;
if (redirect_callers)
{
- /* If alias is non-overwritable then
- all direct calls are safe to be redirected to the original. */
- bool redirected = false;
- while (alias->callers)
- {
- cgraph_edge *e = alias->callers;
- e->redirect_callee (local_original);
- push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl));
+ int nredirected = redirect_all_callers (alias, local_original);
- if (e->call_stmt)
- e->redirect_call_stmt_to_callee ();
+ if (nredirected)
+ {
+ alias->icf_merged = true;
+ local_original->icf_merged = true;
- pop_cfun ();
- redirected = true;
+ if (dump_file && nredirected)
+ fprintf (dump_file, "%i local calls have been "
+ "redirected.\n", nredirected);
}
- alias->icf_merged = true;
- if (local_original->lto_file_data
- && alias->lto_file_data
- && local_original->lto_file_data != alias->lto_file_data)
- local_original->merged = true;
-
- /* The alias function is removed if symbol address
- does not matter. */
- if (!alias_address_matters)
- alias->remove ();
-
- if (dump_file && redirected)
- fprintf (dump_file, "Callgraph local calls have been redirected.\n\n");
+ /* If all callers was redirected, do not produce wrapper. */
+ if (alias->can_remove_if_no_direct_calls_p ()
+ && !alias->has_aliases_p ())
+ {
+ create_wrapper = false;
+ remove = true;
+ }
+ gcc_assert (!create_alias);
}
- /* If the condtion above is not met, we are lucky and can turn the
- function into real alias. */
else if (create_alias)
{
alias->icf_merged = true;
- if (local_original->lto_file_data
- && alias->lto_file_data
- && local_original->lto_file_data != alias->lto_file_data)
- local_original->merged = true;
/* Remove the function's body. */
ipa_merge_profiles (original, alias);
@@ -753,39 +836,38 @@ sem_function::merge (sem_item *alias_ite
(set_local, (void *)(size_t) original->local_p (), true);
if (dump_file)
- fprintf (dump_file, "Callgraph alias has been created.\n\n");
+ fprintf (dump_file, "Unified; Function alias has been created.\n\n");
}
- else if (create_thunk)
+ if (create_wrapper)
{
- if (DECL_COMDAT_GROUP (alias->decl))
- {
- if (dump_file)
- fprintf (dump_file, "Callgraph thunk cannot be created because of COMDAT\n");
+ gcc_assert (!create_alias);
+ alias->icf_merged = true;
+ local_original->icf_merged = true;
- return 0;
- }
+ ipa_merge_profiles (local_original, alias, true);
+ alias->create_wrapper (local_original);
- if (DECL_STATIC_CHAIN (alias->decl))
- {
- if (dump_file)
- fprintf (dump_file, "Thunk creation is risky for static-chain functions.\n\n");
+ if (dump_file)
+ fprintf (dump_file, "Unified; Wrapper has been created.\n\n");
+ }
+ gcc_assert (alias->icf_merged || remove);
+ original->icf_merged = true;
- return 0;
- }
+ /* Inform the inliner about cross-module merging. */
+ if ((original->lto_file_data || alias->lto_file_data)
+ && original->lto_file_data != alias->lto_file_data)
+ local_original->merged = original->merged = true;
+ if (remove)
+ {
+ ipa_merge_profiles (original, alias);
+ alias->release_body ();
+ alias->reset ();
+ alias->body_removed = true;
alias->icf_merged = true;
- if (local_original->lto_file_data
- && alias->lto_file_data
- && local_original->lto_file_data != alias->lto_file_data)
- local_original->merged = true;
- ipa_merge_profiles (local_original, alias, true);
- alias->create_wrapper (local_original);
-
if (dump_file)
- fprintf (dump_file, "Callgraph thunk has been created.\n\n");
+ fprintf (dump_file, "Unified; Function body was removed.\n");
}
- else if (dump_file)
- fprintf (dump_file, "Callgraph merge operation cannot be performed.\n\n");
return true;
}
@@ -1281,7 +1363,8 @@ sem_variable::merge (sem_item *alias_ite
if (!sem_item::target_supports_symbol_aliases_p ())
{
if (dump_file)
- fprintf (dump_file, "Symbol aliases are not supported by target\n\n");
+ fprintf (dump_file, "Not unifying; "
+ "Symbol aliases are not supported by target\n\n");
return false;
}
@@ -1291,42 +1374,60 @@ sem_variable::merge (sem_item *alias_ite
varpool_node *alias = alias_var->get_node ();
bool original_discardable = false;
+ bool original_address_matters = original->address_matters_p ();
+ bool alias_address_matters = original->address_matters_p ();
+
/* See if original is in a section that can be discarded if the main
- symbol is not used. */
- if (DECL_EXTERNAL (original->decl))
- original_discardable = true;
- if (original->resolution == LDPR_PREEMPTED_REG
- || original->resolution == LDPR_PREEMPTED_IR)
- original_discardable = true;
- if (original->can_be_discarded_p ())
+ symbol is not used.
+ Also consider case where we have resolution info and we know that
+ original's definition is not going to be used. In this case we can not
+ create alias to original. */
+ if (original->can_be_discarded_p ()
+ || (node->resolution != LDPR_UNKNOWN
+ && !decl_binds_to_current_def_p (node->decl)))
original_discardable = true;
gcc_assert (!TREE_ASM_WRITTEN (alias->decl));
- if (original_discardable || DECL_EXTERNAL (alias_var->decl) ||
- !compare_sections (alias_var))
+ /* Do not attempt to mix functions from different user sections;
+ we do not know what user intends with those. */
+ if (((DECL_SECTION_NAME (original->decl) && !original->implicit_section)
+ || (DECL_SECTION_NAME (alias->decl) && !alias->implicit_section))
+ && DECL_SECTION_NAME (original->decl) != DECL_SECTION_NAME (alias->decl))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Not unifying; "
+ "original and alias are in different sections.\n\n");
+ return false;
+ }
+
+ /* We can not merge if address comparsion metters. */
+ if (original_address_matters && alias_address_matters
+ && flag_merge_constants < 2)
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Not unifying; "
+ "adress of original and alias may be compared.\n\n");
+ return false;
+ }
+
+ if (original_discardable
+ && (!DECL_COMDAT_GROUP (original->decl)
+ || (DECL_COMDAT_GROUP (original->decl)
+ != DECL_COMDAT_GROUP (alias->decl))))
{
if (dump_file)
- fprintf (dump_file, "Varpool alias cannot be created\n\n");
+ fprintf (dump_file, "Not unifying; alias cannot be created; "
+ "target is discardable\n\n");
return false;
}
else
{
- // alias cycle creation check
- varpool_node *n = original;
-
- while (n->alias)
- {
- n = n->get_alias_target ();
- if (n == alias)
- {
- if (dump_file)
- fprintf (dump_file, "Varpool alias cannot be created (alias cycle).\n\n");
-
- return false;
- }
- }
+ gcc_assert (!original->alias);
+ gcc_assert (!alias->alias);
alias->analyzed = false;
@@ -1338,26 +1439,12 @@ sem_variable::merge (sem_item *alias_ite
alias->resolve_alias (original);
if (dump_file)
- fprintf (dump_file, "Varpool alias has been created.\n\n");
+ fprintf (dump_file, "Unified; Variable alias has been created.\n\n");
return true;
}
}
-bool
-sem_variable::compare_sections (sem_variable *alias)
-{
- const char *source = node->get_section ();
- const char *target = alias->node->get_section();
-
- if (source == NULL && target == NULL)
- return true;
- else if(!source || !target)
- return false;
- else
- return strcmp (source, target) == 0;
-}
-
/* Dump symbol to FILE. */
void
Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c (revision 220956)
+++ ipa-visibility.c (working copy)
@@ -129,27 +129,6 @@ cgraph_node::local_p (void)
}
-/* Return true when there is a reference to node and it is not vtable. */
-
-bool
-symtab_node::address_taken_from_non_vtable_p (void)
-{
- int i;
- struct ipa_ref *ref = NULL;
-
- for (i = 0; iterate_referring (i, ref); i++)
- if (ref->use == IPA_REF_ADDR)
- {
- varpool_node *node;
- if (is_a <cgraph_node *> (ref->referring))
- return true;
- node = dyn_cast <varpool_node *> (ref->referring);
- if (!DECL_VIRTUAL_P (node->decl))
- return true;
- }
- return false;
-}
-
/* A helper for comdat_can_be_unshared_p. */
static bool
@@ -387,7 +366,8 @@ can_replace_by_local_alias_in_vtable (sy
/* walk_tree callback that rewrites initializer references. */
static tree
-update_vtable_references (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
+update_vtable_references (tree *tp, int *walk_subtrees,
+ void *data ATTRIBUTE_UNUSED)
{
if (TREE_CODE (*tp) == VAR_DECL
|| TREE_CODE (*tp) == FUNCTION_DECL)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-25 9:03 ipa-icf::merge TLC Jan Hubicka
@ 2015-02-25 13:10 ` Markus Trippelsdorf
2015-02-25 17:31 ` Jan Hubicka
2015-02-26 16:46 ` Jack Howarth
1 sibling, 1 reply; 26+ messages in thread
From: Markus Trippelsdorf @ 2015-02-25 13:10 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches, mliska, jakub
On 2015.02.25 at 09:38 +0100, Jan Hubicka wrote:
> this patch reorganize sem_function::merge and sem_variable::merge.
> I read the code in detail and found several issues that are fixed in the
> following patch.
I gave your patch a quick spin. It breaks Chromium. Its protocol buffer
compiler gets miscompiled:
RULE Generating C++ and Python code from copresence/proto/enums.proto
FAILED: cd ../../components; python ../tools/protoc_wrapper/protoc_wrapper.py --include "" --protobuf "../out/Release/gen/protoc_out/components/copresence/proto/enums.pb.h" -
-proto-in-dir copresence/proto --proto-in-file "enums.proto" "--use-system-protobuf=0" -- ../out/Release/protoc --cpp_out ../out/Release/gen/protoc_out/components/copresence/
proto --python_out ../out/Release/pyproto/components/copresence/proto
*** Error in `../out/Release/protoc': double free or corruption (out): 0x00007ffd966468b0 ***
======= Backtrace: =========
/lib/libc.so.6(+0x7265e)[0x7fe09058065e]
/lib/libc.so.6(+0x77f1d)[0x7fe090585f1d]
/lib/libc.so.6(+0x7874b)[0x7fe09058674b]
../out/Release/protoc[0x45f5e1]
../out/Release/protoc[0x462caf]
../out/Release/protoc[0x462d1c]
../out/Release/protoc[0x46685c]
../out/Release/protoc[0x4677d5]
../out/Release/protoc[0x4679c3]
../out/Release/protoc[0x467b15]
../out/Release/protoc[0x479d89]
../out/Release/protoc[0x4aa058]
../out/Release/protoc[0x4aa0af]
../out/Release/protoc[0x46b217]
../out/Release/protoc[0x4a4832]
../out/Release/protoc[0x4a86cf]
../out/Release/protoc[0x4a890e]
../out/Release/protoc[0x4a0827]
../out/Release/protoc[0x4678fd]
../out/Release/protoc[0x467b15]
../out/Release/protoc[0x40c926]
../out/Release/protoc[0x403131]
/lib/libc.so.6(__libc_start_main+0xf0)[0x7fe09052e6d0]
../out/Release/protoc[0x4037f9]
======= Memory map: ========
00400000-004eb000 r-xp 00000000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
004ec000-004ef000 r--p 000eb000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
004ef000-004f0000 rw-p 000ee000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
01ab0000-01b03000 rw-p 00000000 00:00 0 [heap]
7fe09027f000-7fe09030d000 r-xp 00000000 00:0f 2540736 /lib64/libm-2.21.90.so
7fe09030d000-7fe09050c000 ---p 0008e000 00:0f 2540736 /lib64/libm-2.21.90.so
7fe09050c000-7fe09050d000 r--p 0008d000 00:0f 2540736 /lib64/libm-2.21.90.so
7fe09050d000-7fe09050e000 rw-p 0008e000 00:0f 2540736 /lib64/libm-2.21.90.so
7fe09050e000-7fe09066e000 r-xp 00000000 00:0f 2541268 /lib64/libc-2.21.90.so
7fe09066e000-7fe09086d000 ---p 00160000 00:0f 2541268 /lib64/libc-2.21.90.so
7fe09086d000-7fe090871000 r--p 0015f000 00:0f 2541268 /lib64/libc-2.21.90.so
7fe090871000-7fe090873000 rw-p 00163000 00:0f 2541268 /lib64/libc-2.21.90.so
7fe090873000-7fe090877000 rw-p 00000000 00:00 0
7fe090877000-7fe09088f000 r-xp 00000000 00:0f 2541131 /lib64/libpthread-2.21.90.so
7fe09088f000-7fe090a8e000 ---p 00018000 00:0f 2541131 /lib64/libpthread-2.21.90.so
7fe090a8e000-7fe090a8f000 r--p 00017000 00:0f 2541131 /lib64/libpthread-2.21.90.so
7fe090a8f000-7fe090a90000 rw-p 00018000 00:0f 2541131 /lib64/libpthread-2.21.90.so
7fe090a90000-7fe090a94000 rw-p 00000000 00:00 0
7fe090a94000-7fe090ab6000 r-xp 00000000 00:0f 2541267 /lib64/ld-2.21.90.so
7fe090ad2000-7fe090ad7000 rw-p 00000000 00:00 0
7fe090ad7000-7fe090aee000 r-xp 00000000 00:0f 72800 /lib64/libgcc_s.so.1
7fe090aee000-7fe090aef000 rw-p 00016000 00:0f 72800 /lib64/libgcc_s.so.1
7fe090aef000-7fe090af0000 rw-p 00000000 00:00 0
7fe090af0000-7fe090c7b000 r-xp 00000000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
7fe090c7b000-7fe090c7c000 ---p 0018b000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
7fe090c7c000-7fe090c86000 r--p 0018b000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
7fe090c86000-7fe090c8a000 rw-p 00195000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
7fe090c8a000-7fe090c8d000 rw-p 00000000 00:00 0
7fe090cb3000-7fe090cb5000 rw-p 00000000 00:00 0
7fe090cb5000-7fe090cb6000 r--p 00021000 00:0f 2541267 /lib64/ld-2.21.90.so
7fe090cb6000-7fe090cb7000 rw-p 00022000 00:0f 2541267 /lib64/ld-2.21.90.so
7fe090cb7000-7fe090cb8000 rw-p 00000000 00:00 0
7ffd96628000-7ffd96649000 rw-p 00000000 00:00 0 [stack]
7ffd967e4000-7ffd967e6000 r--p 00000000 00:00 0 [vvar]
7ffd967e6000-7ffd967e8000 r-xp 00000000 00:00 0 [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
--
Markus
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-25 13:10 ` Markus Trippelsdorf
@ 2015-02-25 17:31 ` Jan Hubicka
2015-02-25 18:41 ` Martin Liška
0 siblings, 1 reply; 26+ messages in thread
From: Jan Hubicka @ 2015-02-25 17:31 UTC (permalink / raw)
To: Markus Trippelsdorf; +Cc: Jan Hubicka, gcc-patches, mliska, jakub
> On 2015.02.25 at 09:38 +0100, Jan Hubicka wrote:
> > this patch reorganize sem_function::merge and sem_variable::merge.
> > I read the code in detail and found several issues that are fixed in the
> > following patch.
>
> I gave your patch a quick spin. It breaks Chromium. Its protocol buffer
> compiler gets miscompiled:
I see (I remember running into simiarly looking ICE last time I tried Chromium).
Is there any chance you can look into what gets wrong? I added enough of sanity checking
code so I do not see how merging itself can lead to wrong codes without ICEing,
but the patch makes considerably more merging to happen (old code had bug where it
tried to merge but didn't), so we may run into another previously latent issue.
Martin has 3 correctness ipa-icf patches in a way, so perhaps one of them ;)
Honza
>
> RULE Generating C++ and Python code from copresence/proto/enums.proto
> FAILED: cd ../../components; python ../tools/protoc_wrapper/protoc_wrapper.py --include "" --protobuf "../out/Release/gen/protoc_out/components/copresence/proto/enums.pb.h" -
> -proto-in-dir copresence/proto --proto-in-file "enums.proto" "--use-system-protobuf=0" -- ../out/Release/protoc --cpp_out ../out/Release/gen/protoc_out/components/copresence/
> proto --python_out ../out/Release/pyproto/components/copresence/proto
> *** Error in `../out/Release/protoc': double free or corruption (out): 0x00007ffd966468b0 ***
> ======= Backtrace: =========
> /lib/libc.so.6(+0x7265e)[0x7fe09058065e]
> /lib/libc.so.6(+0x77f1d)[0x7fe090585f1d]
> /lib/libc.so.6(+0x7874b)[0x7fe09058674b]
> ../out/Release/protoc[0x45f5e1]
> ../out/Release/protoc[0x462caf]
> ../out/Release/protoc[0x462d1c]
> ../out/Release/protoc[0x46685c]
> ../out/Release/protoc[0x4677d5]
> ../out/Release/protoc[0x4679c3]
> ../out/Release/protoc[0x467b15]
> ../out/Release/protoc[0x479d89]
> ../out/Release/protoc[0x4aa058]
> ../out/Release/protoc[0x4aa0af]
> ../out/Release/protoc[0x46b217]
> ../out/Release/protoc[0x4a4832]
> ../out/Release/protoc[0x4a86cf]
> ../out/Release/protoc[0x4a890e]
> ../out/Release/protoc[0x4a0827]
> ../out/Release/protoc[0x4678fd]
> ../out/Release/protoc[0x467b15]
> ../out/Release/protoc[0x40c926]
> ../out/Release/protoc[0x403131]
> /lib/libc.so.6(__libc_start_main+0xf0)[0x7fe09052e6d0]
> ../out/Release/protoc[0x4037f9]
> ======= Memory map: ========
> 00400000-004eb000 r-xp 00000000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
> 004ec000-004ef000 r--p 000eb000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
> 004ef000-004f0000 rw-p 000ee000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
> 01ab0000-01b03000 rw-p 00000000 00:00 0 [heap]
> 7fe09027f000-7fe09030d000 r-xp 00000000 00:0f 2540736 /lib64/libm-2.21.90.so
> 7fe09030d000-7fe09050c000 ---p 0008e000 00:0f 2540736 /lib64/libm-2.21.90.so
> 7fe09050c000-7fe09050d000 r--p 0008d000 00:0f 2540736 /lib64/libm-2.21.90.so
> 7fe09050d000-7fe09050e000 rw-p 0008e000 00:0f 2540736 /lib64/libm-2.21.90.so
> 7fe09050e000-7fe09066e000 r-xp 00000000 00:0f 2541268 /lib64/libc-2.21.90.so
> 7fe09066e000-7fe09086d000 ---p 00160000 00:0f 2541268 /lib64/libc-2.21.90.so
> 7fe09086d000-7fe090871000 r--p 0015f000 00:0f 2541268 /lib64/libc-2.21.90.so
> 7fe090871000-7fe090873000 rw-p 00163000 00:0f 2541268 /lib64/libc-2.21.90.so
> 7fe090873000-7fe090877000 rw-p 00000000 00:00 0
> 7fe090877000-7fe09088f000 r-xp 00000000 00:0f 2541131 /lib64/libpthread-2.21.90.so
> 7fe09088f000-7fe090a8e000 ---p 00018000 00:0f 2541131 /lib64/libpthread-2.21.90.so
> 7fe090a8e000-7fe090a8f000 r--p 00017000 00:0f 2541131 /lib64/libpthread-2.21.90.so
> 7fe090a8f000-7fe090a90000 rw-p 00018000 00:0f 2541131 /lib64/libpthread-2.21.90.so
> 7fe090a90000-7fe090a94000 rw-p 00000000 00:00 0
> 7fe090a94000-7fe090ab6000 r-xp 00000000 00:0f 2541267 /lib64/ld-2.21.90.so
> 7fe090ad2000-7fe090ad7000 rw-p 00000000 00:00 0
> 7fe090ad7000-7fe090aee000 r-xp 00000000 00:0f 72800 /lib64/libgcc_s.so.1
> 7fe090aee000-7fe090aef000 rw-p 00016000 00:0f 72800 /lib64/libgcc_s.so.1
> 7fe090aef000-7fe090af0000 rw-p 00000000 00:00 0
> 7fe090af0000-7fe090c7b000 r-xp 00000000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
> 7fe090c7b000-7fe090c7c000 ---p 0018b000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
> 7fe090c7c000-7fe090c86000 r--p 0018b000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
> 7fe090c86000-7fe090c8a000 rw-p 00195000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
> 7fe090c8a000-7fe090c8d000 rw-p 00000000 00:00 0
> 7fe090cb3000-7fe090cb5000 rw-p 00000000 00:00 0
> 7fe090cb5000-7fe090cb6000 r--p 00021000 00:0f 2541267 /lib64/ld-2.21.90.so
> 7fe090cb6000-7fe090cb7000 rw-p 00022000 00:0f 2541267 /lib64/ld-2.21.90.so
> 7fe090cb7000-7fe090cb8000 rw-p 00000000 00:00 0
> 7ffd96628000-7ffd96649000 rw-p 00000000 00:00 0 [stack]
> 7ffd967e4000-7ffd967e6000 r--p 00000000 00:00 0 [vvar]
> 7ffd967e6000-7ffd967e8000 r-xp 00000000 00:00 0 [vdso]
> ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
>
> --
> Markus
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-25 17:31 ` Jan Hubicka
@ 2015-02-25 18:41 ` Martin Liška
2015-02-25 19:02 ` Markus Trippelsdorf
0 siblings, 1 reply; 26+ messages in thread
From: Martin Liška @ 2015-02-25 18:41 UTC (permalink / raw)
To: Jan Hubicka, Markus Trippelsdorf; +Cc: gcc-patches, jakub
On 02/25/2015 06:15 PM, Jan Hubicka wrote:
>> On 2015.02.25 at 09:38 +0100, Jan Hubicka wrote:
>>> this patch reorganize sem_function::merge and sem_variable::merge.
>>> I read the code in detail and found several issues that are fixed in the
>>> following patch.
>>
>> I gave your patch a quick spin. It breaks Chromium. Its protocol buffer
>> compiler gets miscompiled:
>
> I see (I remember running into simiarly looking ICE last time I tried Chromium).
> Is there any chance you can look into what gets wrong? I added enough of sanity checking
> code so I do not see how merging itself can lead to wrong codes without ICEing,
> but the patch makes considerably more merging to happen (old code had bug where it
> tried to merge but didn't), so we may run into another previously latent issue.
> Martin has 3 correctness ipa-icf patches in a way, so perhaps one of them ;)
>
> Honza
Hello.
I've just updated chromium to latest version, but unfortunately I cannot reproduce the memory corruption.
Which build flags do you use Markus for Chromium? Can you please run valgrind to spot the problematic function?
Moreover, I would appreciate if you will be able to find corresponding merge operation (-fdump-ipa-icf)
Thank you,
Martin
>>
>> RULE Generating C++ and Python code from copresence/proto/enums.proto
>> FAILED: cd ../../components; python ../tools/protoc_wrapper/protoc_wrapper.py --include "" --protobuf "../out/Release/gen/protoc_out/components/copresence/proto/enums.pb.h" -
>> -proto-in-dir copresence/proto --proto-in-file "enums.proto" "--use-system-protobuf=0" -- ../out/Release/protoc --cpp_out ../out/Release/gen/protoc_out/components/copresence/
>> proto --python_out ../out/Release/pyproto/components/copresence/proto
>> *** Error in `../out/Release/protoc': double free or corruption (out): 0x00007ffd966468b0 ***
>> ======= Backtrace: =========
>> /lib/libc.so.6(+0x7265e)[0x7fe09058065e]
>> /lib/libc.so.6(+0x77f1d)[0x7fe090585f1d]
>> /lib/libc.so.6(+0x7874b)[0x7fe09058674b]
>> ../out/Release/protoc[0x45f5e1]
>> ../out/Release/protoc[0x462caf]
>> ../out/Release/protoc[0x462d1c]
>> ../out/Release/protoc[0x46685c]
>> ../out/Release/protoc[0x4677d5]
>> ../out/Release/protoc[0x4679c3]
>> ../out/Release/protoc[0x467b15]
>> ../out/Release/protoc[0x479d89]
>> ../out/Release/protoc[0x4aa058]
>> ../out/Release/protoc[0x4aa0af]
>> ../out/Release/protoc[0x46b217]
>> ../out/Release/protoc[0x4a4832]
>> ../out/Release/protoc[0x4a86cf]
>> ../out/Release/protoc[0x4a890e]
>> ../out/Release/protoc[0x4a0827]
>> ../out/Release/protoc[0x4678fd]
>> ../out/Release/protoc[0x467b15]
>> ../out/Release/protoc[0x40c926]
>> ../out/Release/protoc[0x403131]
>> /lib/libc.so.6(__libc_start_main+0xf0)[0x7fe09052e6d0]
>> ../out/Release/protoc[0x4037f9]
>> ======= Memory map: ========
>> 00400000-004eb000 r-xp 00000000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
>> 004ec000-004ef000 r--p 000eb000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
>> 004ef000-004f0000 rw-p 000ee000 00:13 36244132 /var/tmp/chromium/src/out/Release/protoc
>> 01ab0000-01b03000 rw-p 00000000 00:00 0 [heap]
>> 7fe09027f000-7fe09030d000 r-xp 00000000 00:0f 2540736 /lib64/libm-2.21.90.so
>> 7fe09030d000-7fe09050c000 ---p 0008e000 00:0f 2540736 /lib64/libm-2.21.90.so
>> 7fe09050c000-7fe09050d000 r--p 0008d000 00:0f 2540736 /lib64/libm-2.21.90.so
>> 7fe09050d000-7fe09050e000 rw-p 0008e000 00:0f 2540736 /lib64/libm-2.21.90.so
>> 7fe09050e000-7fe09066e000 r-xp 00000000 00:0f 2541268 /lib64/libc-2.21.90.so
>> 7fe09066e000-7fe09086d000 ---p 00160000 00:0f 2541268 /lib64/libc-2.21.90.so
>> 7fe09086d000-7fe090871000 r--p 0015f000 00:0f 2541268 /lib64/libc-2.21.90.so
>> 7fe090871000-7fe090873000 rw-p 00163000 00:0f 2541268 /lib64/libc-2.21.90.so
>> 7fe090873000-7fe090877000 rw-p 00000000 00:00 0
>> 7fe090877000-7fe09088f000 r-xp 00000000 00:0f 2541131 /lib64/libpthread-2.21.90.so
>> 7fe09088f000-7fe090a8e000 ---p 00018000 00:0f 2541131 /lib64/libpthread-2.21.90.so
>> 7fe090a8e000-7fe090a8f000 r--p 00017000 00:0f 2541131 /lib64/libpthread-2.21.90.so
>> 7fe090a8f000-7fe090a90000 rw-p 00018000 00:0f 2541131 /lib64/libpthread-2.21.90.so
>> 7fe090a90000-7fe090a94000 rw-p 00000000 00:00 0
>> 7fe090a94000-7fe090ab6000 r-xp 00000000 00:0f 2541267 /lib64/ld-2.21.90.so
>> 7fe090ad2000-7fe090ad7000 rw-p 00000000 00:00 0
>> 7fe090ad7000-7fe090aee000 r-xp 00000000 00:0f 72800 /lib64/libgcc_s.so.1
>> 7fe090aee000-7fe090aef000 rw-p 00016000 00:0f 72800 /lib64/libgcc_s.so.1
>> 7fe090aef000-7fe090af0000 rw-p 00000000 00:00 0
>> 7fe090af0000-7fe090c7b000 r-xp 00000000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
>> 7fe090c7b000-7fe090c7c000 ---p 0018b000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
>> 7fe090c7c000-7fe090c86000 r--p 0018b000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
>> 7fe090c86000-7fe090c8a000 rw-p 00195000 00:0f 3018239 /usr/lib64/gcc/x86_64-pc-linux-gnu/5.0.0/libstdc++.so.6.0.21
>> 7fe090c8a000-7fe090c8d000 rw-p 00000000 00:00 0
>> 7fe090cb3000-7fe090cb5000 rw-p 00000000 00:00 0
>> 7fe090cb5000-7fe090cb6000 r--p 00021000 00:0f 2541267 /lib64/ld-2.21.90.so
>> 7fe090cb6000-7fe090cb7000 rw-p 00022000 00:0f 2541267 /lib64/ld-2.21.90.so
>> 7fe090cb7000-7fe090cb8000 rw-p 00000000 00:00 0
>> 7ffd96628000-7ffd96649000 rw-p 00000000 00:00 0 [stack]
>> 7ffd967e4000-7ffd967e6000 r--p 00000000 00:00 0 [vvar]
>> 7ffd967e6000-7ffd967e8000 r-xp 00000000 00:00 0 [vdso]
>> ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vsyscall]
>>
>> --
>> Markus
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-25 18:41 ` Martin Liška
@ 2015-02-25 19:02 ` Markus Trippelsdorf
0 siblings, 0 replies; 26+ messages in thread
From: Markus Trippelsdorf @ 2015-02-25 19:02 UTC (permalink / raw)
To: Martin Liška; +Cc: Jan Hubicka, gcc-patches, jakub
On 2015.02.25 at 19:32 +0100, Martin Liška wrote:
> On 02/25/2015 06:15 PM, Jan Hubicka wrote:
> >> On 2015.02.25 at 09:38 +0100, Jan Hubicka wrote:
> >>> this patch reorganize sem_function::merge and sem_variable::merge.
> >>> I read the code in detail and found several issues that are fixed in the
> >>> following patch.
> >>
> >> I gave your patch a quick spin. It breaks Chromium. Its protocol buffer
> >> compiler gets miscompiled:
> >
> > I see (I remember running into simiarly looking ICE last time I tried Chromium).
> > Is there any chance you can look into what gets wrong? I added enough of sanity checking
> > code so I do not see how merging itself can lead to wrong codes without ICEing,
> > but the patch makes considerably more merging to happen (old code had bug where it
> > tried to merge but didn't), so we may run into another previously latent issue.
> > Martin has 3 correctness ipa-icf patches in a way, so perhaps one of them ;)
> >
> > Honza
>
> Hello.
>
> I've just updated chromium to latest version, but unfortunately I cannot reproduce the memory corruption.
> Which build flags do you use Markus for Chromium? Can you please run valgrind to spot the problematic function?
> Moreover, I would appreciate if you will be able to find corresponding merge operation (-fdump-ipa-icf)
I'm just using the defaults:
GYP_DEFINES="ffmpeg_branding=Chrome use_kerberos=0 fastbuild=1 remove_webcore_debug_symbols=1 use_pulseaudio=0 use_gnome_keyring=0 use_linux_link_gnome_keyring=0 disable_nacl=1 clang=0 host_clang=0 linux_use_bundled_binutils=0 linux_use_bundled_gold=0 google_api_key=AIzaSyDEAOvatFo0eTgsV_ZlEzx0ObmepsMzfAc google_default_client_id=329227923882.apps.googleusercontent.com google_default_client_secret=vgKG0NNv7GoDpbtoFNLxCUXu werror=" gclient sync
and then "ninja chrome" to build.
Program received signal SIGABRT, Aborted.
0x00007ffff78886f8 in raise () from /lib/libc.so.6
(gdb) bt
#0 0x00007ffff78886f8 in raise () from /lib/libc.so.6
#1 0x00007ffff7889bbd in abort () from /lib/libc.so.6
#2 0x00007ffff78c7663 in __libc_message () from /lib/libc.so.6
#3 0x00007ffff78ccf1d in malloc_printerr () from /lib/libc.so.6
#4 0x00007ffff78cd74b in _int_free () from /lib/libc.so.6
#5 0x000000000045f5c1 in google::protobuf::DescriptorBuilder::BuildFieldOrExtension(google::protobuf::FieldDescriptorProto const&, google::protobuf::Descriptor const*, googl
e::protobuf::FieldDescriptor*, bool) ()
#6 0x0000000000462c8f in google::protobuf::DescriptorBuilder::BuildMessage(google::protobuf::DescriptorProto const&, google::protobuf::Descriptor const*, google::protobuf::D
escriptor*) ()
#7 0x0000000000462cfc in google::protobuf::DescriptorBuilder::BuildMessage(google::protobuf::DescriptorProto const&, google::protobuf::Descriptor const*, google::protobuf::D
escriptor*) ()
#8 0x000000000046683c in google::protobuf::DescriptorBuilder::BuildFile(google::protobuf::FileDescriptorProto const&) ()
#9 0x00000000004677b5 in google::protobuf::DescriptorPool::BuildFileFromDatabase(google::protobuf::FileDescriptorProto const&) const ()
#10 0x00000000004679a3 in google::protobuf::DescriptorPool::TryFindFileInFallbackDatabase(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cons
t&) const ()
#11 0x0000000000467af5 in google::protobuf::DescriptorPool::FindFileByName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const ()
#12 0x0000000000479d69 in google::protobuf::protobuf_AssignDesc_google_2fprotobuf_2fdescriptor_2eproto() ()
#13 0x00000000004aa038 in google::protobuf::GoogleOnceInitImpl(long*, google::protobuf::Closure*) ()
#14 0x00000000004aa08f in google::protobuf::GoogleOnceInit(long*, void (*)()) ()
#15 0x000000000046b1f7 in google::protobuf::FileOptions::GetMetadata() const ()
#16 0x00000000004a4812 in google::protobuf::compiler::Parser::ParseOption(google::protobuf::Message*, google::protobuf::compiler::Parser::LocationRecorder const&, google::pro
tobuf::compiler::Parser::OptionStyle) ()
#17 0x00000000004a86af in google::protobuf::compiler::Parser::ParseTopLevelStatement(google::protobuf::FileDescriptorProto*, google::protobuf::compiler::Parser::LocationRecor
der const&) ()
#18 0x00000000004a88ee in google::protobuf::compiler::Parser::Parse(google::protobuf::io::Tokenizer*, google::protobuf::FileDescriptorProto*) ()
#19 0x00000000004a0807 in google::protobuf::compiler::SourceTreeDescriptorDatabase::FindFileByName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<cha
r> > const&, google::protobuf::FileDescriptorProto*) ()
#20 0x00000000004678dd in google::protobuf::DescriptorPool::TryFindFileInFallbackDatabase(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > cons
t&) const ()
#21 0x0000000000467af5 in google::protobuf::DescriptorPool::FindFileByName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const ()
#22 0x000000000040c906 in google::protobuf::compiler::CommandLineInterface::Run(int, char const* const*) ()
#23 0x0000000000403131 in main ()
I have tried to add -fno-ipa-icf when compiling various object files, whose
symbols are in the backtrace, but without success thus far.
--
Markus
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-25 9:03 ipa-icf::merge TLC Jan Hubicka
2015-02-25 13:10 ` Markus Trippelsdorf
@ 2015-02-26 16:46 ` Jack Howarth
2015-02-27 5:55 ` Jan Hubicka
1 sibling, 1 reply; 26+ messages in thread
From: Jack Howarth @ 2015-02-26 16:46 UTC (permalink / raw)
To: Jan Hubicka; +Cc: GCC Patches, mliska, Jakub Jelinek
On Wed, Feb 25, 2015 at 3:38 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>
> I plan to commit after some further testing tomorrow and having chance
> Martin to look across the changes and discuss 5).
>
> Honza
>
Is this still going in today or is there a newer patch to test?
Jack
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-26 16:46 ` Jack Howarth
@ 2015-02-27 5:55 ` Jan Hubicka
2015-02-27 13:48 ` H.J. Lu
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Jan Hubicka @ 2015-02-27 5:55 UTC (permalink / raw)
To: Jack Howarth; +Cc: Jan Hubicka, GCC Patches, mliska, Jakub Jelinek
Hi,
this is the final version of patch I comitted. It has new fix to make_decl_local
to set TREE_ADDRESSABLE becuase we leave the flag undefined for non-local decls.
I also dropped Optimization from fmerge-all-constants, fmerge-constants
those can not be done in function speicfic way, I made ipa_ref::address_matters_p
to use fmerge-constants, added code to drop UNINLINABLE flag when function is turned
into a wrapper, added check to require DECL_NO_INLINE_WARNING_P match
and added code to set TREE_ADDRESSABLE when non-addressable and addressable vars are merged.
I also disabled merging for DECL_CONSTANT_POOL because it does not work (symtab does not
expect aliases here)
Bootstrapped/regtested x86_64-linux, comitted.
Honza
* ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton):
Use address_matters_p.
(redirect_all_callers, set_addressable): New functions.
(sem_function::merge): Reorganize and fix merging issues.
(sem_variable::merge): Likewise.
(sem_variable::compare_sections): Remove.
* common.opt (fmerge-all-constants, fmerge-constants): Remove
Optimization flag.
* symtab.c (symtab_node::resolve_alias): When alias has aliases,
redirect them.
(symtab_node::make_decl_local): Set ADDRESSABLE bit when
decl is used.
(address_matters_1): New function.
(symtab_node::address_matters_p): New function.
* cgraph.c (cgraph_edge::verify_corresponds_to_fndecl): Fix
check for merged flag.
* cgraph.h (address_matters_p): Declare.
(symtab_node::address_taken_from_non_vtable_p): Remove.
(symtab_node::address_can_be_compared_p): New method.
(ipa_ref::address_matters_p): Move here from ipa-ref.c; simplify.
* ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p):
Remove.
(comdat_can_be_unshared_p_1) Use address_matters_p.
(update_vtable_references): Fix formating.
* ipa-ref.c (ipa_ref::address_matters_p): Move inline.
* cgraphunit.c (cgraph_node::create_wrapper): Drop UNINLINABLE flag.
* cgraphclones.c: Preserve merged and icf_merged flags.
* gcc.dg/pr64454.c: Disable ICF.
* gcc.dg/pr28685-1.c: Disable ICF
* gcc.dg/ipa/iinline-5.c: Disable ICF.
* g++.dg/warn/Wsuggest-final.C: Force methods to be different.
* g++.dg/ipa/ipa-icf-4.C: Update template.
Index: symtab.c
===================================================================
--- symtab.c (revision 221034)
+++ symtab.c (working copy)
@@ -1156,7 +1156,11 @@ symtab_node::make_decl_local (void)
return;
if (TREE_CODE (decl) == VAR_DECL)
- DECL_COMMON (decl) = 0;
+ {
+ DECL_COMMON (decl) = 0;
+ /* ADDRESSABLE flag is not defined for public symbols. */
+ TREE_ADDRESSABLE (decl) = 1;
+ }
else gcc_assert (TREE_CODE (decl) == FUNCTION_DECL);
DECL_COMDAT (decl) = 0;
@@ -1513,6 +1517,19 @@ symtab_node::resolve_alias (symtab_node
/* If alias has address taken, so does the target. */
if (address_taken)
target->ultimate_alias_target ()->address_taken = true;
+
+ /* All non-weakref aliases of THIS are now in fact aliases of TARGET. */
+ ipa_ref *ref;
+ for (unsigned i = 0; iterate_direct_aliases (i, ref);)
+ {
+ struct symtab_node *alias_alias = ref->referring;
+ if (!alias_alias->weakref)
+ {
+ alias_alias->remove_all_references ();
+ alias_alias->create_reference (target, IPA_REF_ALIAS, NULL);
+ }
+ else i++;
+ }
return true;
}
@@ -1863,3 +1880,31 @@ symtab_node::call_for_symbol_and_aliases
}
return false;
}
+
+/* Return ture if address of N is possibly compared. */
+
+static bool
+address_matters_1 (symtab_node *n, void *)
+{
+ struct ipa_ref *ref;
+
+ if (!n->address_can_be_compared_p ())
+ return false;
+ if (n->externally_visible || n->force_output)
+ return true;
+
+ for (unsigned int i = 0; n->iterate_referring (i, ref); i++)
+ if (ref->address_matters_p ())
+ return true;
+ return false;
+}
+
+/* Return true if symbol's address may possibly be compared to other
+ symbol's address. */
+
+bool
+symtab_node::address_matters_p ()
+{
+ gcc_assert (!alias);
+ return call_for_symbol_and_aliases (address_matters_1, NULL, true);
+}
Index: common.opt
===================================================================
--- common.opt (revision 221034)
+++ common.opt (working copy)
@@ -1644,11 +1644,11 @@ Report on permanent memory allocation in
; string constants and constants from constant pool, if 2 also constant
; variables.
fmerge-all-constants
-Common Report Var(flag_merge_constants,2) Init(1) Optimization
+Common Report Var(flag_merge_constants,2) Init(1)
Attempt to merge identical constants and constant variables
fmerge-constants
-Common Report Var(flag_merge_constants,1) Optimization
+Common Report Var(flag_merge_constants,1)
Attempt to merge identical constants across compilation units
fmerge-debug-strings
Index: testsuite/gcc.dg/pr64454.c
===================================================================
--- testsuite/gcc.dg/pr64454.c (revision 221034)
+++ testsuite/gcc.dg/pr64454.c (working copy)
@@ -1,6 +1,6 @@
/* PR tree-optimization/64454 */
/* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1" } */
+/* { dg-options "-O2 -fdump-tree-vrp1 -fno-ipa-icf" } */
unsigned
f1 (unsigned x)
Index: testsuite/gcc.dg/pr28685-1.c
===================================================================
--- testsuite/gcc.dg/pr28685-1.c (revision 221034)
+++ testsuite/gcc.dg/pr28685-1.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-optimized" } */
+/* { dg-options "-O2 -fdump-tree-optimized -fno-ipa-icf" } */
/* Should produce <=. */
int test1 (int a, int b)
Index: testsuite/gcc.dg/ipa/iinline-5.c
===================================================================
--- testsuite/gcc.dg/ipa/iinline-5.c (revision 221034)
+++ testsuite/gcc.dg/ipa/iinline-5.c (working copy)
@@ -1,7 +1,7 @@
/* Verify that simple indirect calls are inlined even without early
inlining.. */
/* { dg-do run } */
-/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */
+/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-icf" } */
extern void abort (void);
Index: testsuite/g++.dg/warn/Wsuggest-final.C
===================================================================
--- testsuite/g++.dg/warn/Wsuggest-final.C (revision 221034)
+++ testsuite/g++.dg/warn/Wsuggest-final.C (working copy)
@@ -1,8 +1,9 @@
// { dg-do compile }
// { dg-options "-O2 -Wsuggest-final-types -Wsuggest-final-methods" }
+int c;
struct A { // { dg-warning "final would enable devirtualization of 4 calls" }
virtual void a() {} // { dg-warning "final would enable devirtualization of 2 calls" }
- virtual void b() {} // { dg-warning "final would enable devirtualization of 2 calls" }
+ virtual void b() {c++;} // { dg-warning "final would enable devirtualization of 2 calls" }
};
void
t(struct A *a)
Index: testsuite/g++.dg/ipa/ipa-icf-4.C
===================================================================
--- testsuite/g++.dg/ipa/ipa-icf-4.C (revision 221034)
+++ testsuite/g++.dg/ipa/ipa-icf-4.C (working copy)
@@ -43,6 +43,6 @@ int main()
return 123;
}
-/* { dg-final { scan-ipa-dump "\(Varpool alias has been created\)|\(Symbol aliases are not supported by target\)" "icf" } } */
+/* { dg-final { scan-ipa-dump "\(Unified; Variable alias has been created\)|\(Symbol aliases are not supported by target\)" "icf" } } */
/* { dg-final { scan-ipa-dump "Equal symbols: 6" "icf" } } */
/* { dg-final { cleanup-ipa-dump "icf" } } */
Index: ipa-ref.c
===================================================================
--- ipa-ref.c (revision 221034)
+++ ipa-ref.c (working copy)
@@ -124,23 +124,3 @@ ipa_ref::referred_ref_list (void)
{
return &referred->ref_list;
}
-
-/* Return true if refernece may be used in address compare. */
-bool
-ipa_ref::address_matters_p ()
-{
- if (use != IPA_REF_ADDR)
- return false;
- /* Addresses taken from virtual tables are never compared. */
- if (is_a <varpool_node *> (referring)
- && DECL_VIRTUAL_P (referring->decl))
- return false;
- /* Address of virtual tables and functions is never compared. */
- if (DECL_VIRTUAL_P (referred->decl))
- return false;
- /* Address of C++ cdtors is never compared. */
- if (is_a <cgraph_node *> (referred)
- && (DECL_CXX_CONSTRUCTOR_P (referred->decl) || DECL_CXX_DESTRUCTOR_P (referred->decl)))
- return false;
- return true;
-}
Index: cgraph.c
===================================================================
--- cgraph.c (revision 221034)
+++ cgraph.c (working copy)
@@ -2630,7 +2630,7 @@ cgraph_edge::verify_corresponds_to_fndec
if (!node
|| node->body_removed
|| node->in_other_partition
- || node->icf_merged
+ || callee->icf_merged
|| callee->in_other_partition)
return false;
Index: cgraph.h
===================================================================
--- cgraph.h (revision 221034)
+++ cgraph.h (working copy)
@@ -326,9 +326,6 @@ public:
/* Return true if ONE and TWO are part of the same COMDAT group. */
inline bool in_same_comdat_group_p (symtab_node *target);
- /* Return true when there is a reference to node and it is not vtable. */
- bool address_taken_from_non_vtable_p (void);
-
/* Return true if symbol is known to be nonzero. */
bool nonzero_address ();
@@ -337,6 +334,15 @@ public:
return 2 otherwise. */
int equal_address_to (symtab_node *s2);
+ /* Return true if symbol's address may possibly be compared to other
+ symbol's address. */
+ bool address_matters_p ();
+
+ /* Return true if NODE's address can be compared. This use properties
+ of NODE only and does not look if the address is actually taken in
+ interesting way. For that use ADDRESS_MATTERS_P instead. */
+ bool address_can_be_compared_p (void);
+
/* Return symbol table node associated with DECL, if any,
and NULL otherwise. */
static inline symtab_node *get (const_tree decl)
@@ -3022,6 +3028,43 @@ varpool_node::call_for_symbol_and_aliase
return false;
}
+/* Return true if NODE's address can be compared. */
+
+inline bool
+symtab_node::address_can_be_compared_p ()
+{
+ /* Address of virtual tables and functions is never compared. */
+ if (DECL_VIRTUAL_P (decl))
+ return false;
+ /* Address of C++ cdtors is never compared. */
+ if (is_a <cgraph_node *> (this)
+ && (DECL_CXX_CONSTRUCTOR_P (decl)
+ || DECL_CXX_DESTRUCTOR_P (decl)))
+ return false;
+ /* Constant pool symbols addresses are never compared.
+ flag_merge_constants permits us to assume the same on readonly vars. */
+ if (is_a <varpool_node *> (this)
+ && (DECL_IN_CONSTANT_POOL (decl)
+ || (flag_merge_constants >= 2
+ && TREE_READONLY (decl) && !TREE_THIS_VOLATILE (decl))))
+ return false;
+ return true;
+}
+
+/* Return true if refernece may be used in address compare. */
+
+inline bool
+ipa_ref::address_matters_p ()
+{
+ if (use != IPA_REF_ADDR)
+ return false;
+ /* Addresses taken from virtual tables are never compared. */
+ if (is_a <varpool_node *> (referring)
+ && DECL_VIRTUAL_P (referring->decl))
+ return false;
+ return referred->address_can_be_compared_p ();
+}
+
/* Build polymorphic call context for indirect call E. */
inline
Index: ipa-visibility.c
===================================================================
--- ipa-visibility.c (revision 221034)
+++ ipa-visibility.c (working copy)
@@ -129,27 +129,6 @@ cgraph_node::local_p (void)
}
-/* Return true when there is a reference to node and it is not vtable. */
-
-bool
-symtab_node::address_taken_from_non_vtable_p (void)
-{
- int i;
- struct ipa_ref *ref = NULL;
-
- for (i = 0; iterate_referring (i, ref); i++)
- if (ref->use == IPA_REF_ADDR)
- {
- varpool_node *node;
- if (is_a <cgraph_node *> (ref->referring))
- return true;
- node = dyn_cast <varpool_node *> (ref->referring);
- if (!DECL_VIRTUAL_P (node->decl))
- return true;
- }
- return false;
-}
-
/* A helper for comdat_can_be_unshared_p. */
static bool
@@ -157,16 +136,14 @@ comdat_can_be_unshared_p_1 (symtab_node
{
if (!node->externally_visible)
return true;
- /* When address is taken, we don't know if equality comparison won't
- break eventually. Exception are virutal functions, C++
- constructors/destructors and vtables, where this is not possible by
- language standard. */
- if (!DECL_VIRTUAL_P (node->decl)
- && (TREE_CODE (node->decl) != FUNCTION_DECL
- || (!DECL_CXX_CONSTRUCTOR_P (node->decl)
- && !DECL_CXX_DESTRUCTOR_P (node->decl)))
- && node->address_taken_from_non_vtable_p ())
- return false;
+ if (node->address_can_be_compared_p ())
+ {
+ struct ipa_ref *ref;
+
+ for (unsigned int i = 0; node->iterate_referring (i, ref); i++)
+ if (ref->address_matters_p ())
+ return false;
+ }
/* If the symbol is used in some weird way, better to not touch it. */
if (node->force_output)
@@ -387,7 +364,8 @@ can_replace_by_local_alias_in_vtable (sy
/* walk_tree callback that rewrites initializer references. */
static tree
-update_vtable_references (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
+update_vtable_references (tree *tp, int *walk_subtrees,
+ void *data ATTRIBUTE_UNUSED)
{
if (TREE_CODE (*tp) == VAR_DECL
|| TREE_CODE (*tp) == FUNCTION_DECL)
Index: cgraphunit.c
===================================================================
--- cgraphunit.c (revision 221034)
+++ cgraphunit.c (working copy)
@@ -2468,6 +2468,7 @@ cgraph_node::create_wrapper (cgraph_node
release_body (true);
reset ();
+ DECL_UNINLINABLE (decl) = false;
DECL_RESULT (decl) = decl_result;
DECL_INITIAL (decl) = NULL;
allocate_struct_function (decl, false);
Index: ipa-icf.c
===================================================================
--- ipa-icf.c (revision 221034)
+++ ipa-icf.c (working copy)
@@ -147,7 +147,7 @@ symbol_compare_collection::symbol_compar
if (ref->referred->get_availability () <= AVAIL_INTERPOSABLE)
{
- if (ref->use == IPA_REF_ADDR)
+ if (ref->address_matters_p ())
m_references.safe_push (ref->referred);
else
m_interposables.safe_push (ref->referred);
@@ -632,8 +633,56 @@ set_local (cgraph_node *node, void *data
return false;
}
+/* TREE_ADDRESSABLE of NODE to true if DATA is non-NULL.
+ Helper for call_for_symbol_thunks_and_aliases. */
+
+static bool
+set_addressable (varpool_node *node, void *)
+{
+ TREE_ADDRESSABLE (node->decl) = 1;
+ return false;
+}
+
+/* Redirect all callers of N and its aliases to TO. Remove aliases if
+ possible. Return number of redirections made. */
+
+static int
+redirect_all_callers (cgraph_node *n, cgraph_node *to)
+{
+ int nredirected = 0;
+ ipa_ref *ref;
+
+ while (n->callers)
+ {
+ cgraph_edge *e = n->callers;
+ e->redirect_callee (to);
+ nredirected++;
+ }
+ for (unsigned i = 0; n->iterate_direct_aliases (i, ref);)
+ {
+ bool removed = false;
+ cgraph_node *n_alias = dyn_cast <cgraph_node *> (ref->referring);
+
+ if ((DECL_COMDAT_GROUP (n->decl)
+ && (DECL_COMDAT_GROUP (n->decl)
+ == DECL_COMDAT_GROUP (n_alias->decl)))
+ || (n_alias->get_availability () > AVAIL_INTERPOSABLE
+ && n->get_availability () > AVAIL_INTERPOSABLE))
+ {
+ nredirected += redirect_all_callers (n_alias, to);
+ if (n_alias->can_remove_if_no_direct_calls_p ()
+ && !n_alias->has_aliases_p ())
+ n_alias->remove ();
+ }
+ if (!removed)
+ i++;
+ }
+ return nredirected;
+}
+
/* Merges instance with an ALIAS_ITEM, where alias, thunk or redirection can
be applied. */
+
bool
sem_function::merge (sem_item *alias_item)
{
@@ -642,16 +691,29 @@ sem_function::merge (sem_item *alias_ite
sem_function *alias_func = static_cast<sem_function *> (alias_item);
cgraph_node *original = get_node ();
- cgraph_node *local_original = original;
+ cgraph_node *local_original = NULL;
cgraph_node *alias = alias_func->get_node ();
- bool original_address_matters;
- bool alias_address_matters;
- bool create_thunk = false;
+ bool create_wrapper = false;
bool create_alias = false;
bool redirect_callers = false;
+ bool remove = false;
+
bool original_discardable = false;
+ bool original_address_matters = original->address_matters_p ();
+ bool alias_address_matters = alias->address_matters_p ();
+
+ if (DECL_NO_INLINE_WARNING_P (original->decl)
+ != DECL_NO_INLINE_WARNING_P (alias->decl))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Not unifying; "
+ "DECL_NO_INLINE_WARNING mismatch.\n\n");
+ return false;
+ }
+
/* Do not attempt to mix functions from different user sections;
we do not know what user intends with those. */
if (((DECL_SECTION_NAME (original->decl) && !original->implicit_section)
@@ -660,123 +722,173 @@ sem_function::merge (sem_item *alias_ite
{
if (dump_file)
fprintf (dump_file,
- "Not unifying; original and alias are in different sections.\n\n");
+ "Not unifying; "
+ "original and alias are in different sections.\n\n");
return false;
}
/* See if original is in a section that can be discarded if the main
- symbol is not used. */
- if (DECL_EXTERNAL (original->decl))
- original_discardable = true;
- if (original->resolution == LDPR_PREEMPTED_REG
- || original->resolution == LDPR_PREEMPTED_IR)
- original_discardable = true;
- if (original->can_be_discarded_p ())
+ symbol is not used.
+
+ Also consider case where we have resolution info and we know that
+ original's definition is not going to be used. In this case we can not
+ create alias to original. */
+ if (original->can_be_discarded_p ()
+ || (node->resolution != LDPR_UNKNOWN
+ && !decl_binds_to_current_def_p (node->decl)))
original_discardable = true;
- /* See if original and/or alias address can be compared for equality. */
- original_address_matters
- = (!DECL_VIRTUAL_P (original->decl)
- && (original->externally_visible
- || original->address_taken_from_non_vtable_p ()));
- alias_address_matters
- = (!DECL_VIRTUAL_P (alias->decl)
- && (alias->externally_visible
- || alias->address_taken_from_non_vtable_p ()));
-
- /* If alias and original can be compared for address equality, we need
- to create a thunk. Also we can not create extra aliases into discardable
- section (or we risk link failures when section is discarded). */
- if ((original_address_matters
- && alias_address_matters)
+ /* Creating a symtab alias is the optimal way to merge.
+ It however can not be used in the following cases:
+
+ 1) if ORIGINAL and ALIAS may be possibly compared for address equality.
+ 2) if ORIGINAL is in a section that may be discarded by linker or if
+ it is an external functions where we can not create an alias
+ (ORIGINAL_DISCARDABLE)
+ 3) if target do not support symbol aliases.
+
+ If we can not produce alias, we will turn ALIAS into WRAPPER of ORIGINAL
+ and/or redirect all callers from ALIAS to ORIGINAL. */
+ if ((original_address_matters && alias_address_matters)
|| original_discardable
- || DECL_COMDAT_GROUP (alias->decl)
|| !sem_item::target_supports_symbol_aliases_p ())
{
- create_thunk = !stdarg_p (TREE_TYPE (alias->decl));
- create_alias = false;
- /* When both alias and original are not overwritable, we can save
- the extra thunk wrapper for direct calls. */
+ /* First see if we can produce wrapper. */
+
+ /* Do not turn function in one comdat group into wrapper to another
+ comdat group. Other compiler producing the body of the
+ another comdat group may make opossite decision and with unfortunate
+ linker choices this may close a loop. */
+ if (DECL_COMDAT_GROUP (alias->decl)
+ && (DECL_COMDAT_GROUP (alias->decl)
+ != DECL_COMDAT_GROUP (original->decl)))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Wrapper cannot be created because of COMDAT\n");
+ }
+ else if (DECL_STATIC_CHAIN (alias->decl))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Can not create wrapper of nested functions.\n");
+ }
+ /* TODO: We can also deal with variadic functions never calling
+ VA_START. */
+ else if (stdarg_p (TREE_TYPE (alias->decl)))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "can not create wrapper of stdarg function.\n");
+ }
+ else if (inline_summaries
+ && inline_summaries->get (alias)->self_size <= 2)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Wrapper creation is not "
+ "profitable (function is too small).\n");
+ }
+ /* If user paid attention to mark function noinline, assume it is
+ somewhat special and do not try to turn it into a wrapper that can
+ not be undone by inliner. */
+ else if (lookup_attribute ("noinline", DECL_ATTRIBUTES (alias->decl)))
+ {
+ if (dump_file)
+ fprintf (dump_file, "Wrappers are not created for noinline.\n");
+ }
+ else
+ create_wrapper = true;
+
+ /* We can redirect local calls in the case both alias and orignal
+ are not interposable. */
redirect_callers
- = (!original_discardable
- && !DECL_COMDAT_GROUP (alias->decl)
- && alias->get_availability () > AVAIL_INTERPOSABLE
- && original->get_availability () > AVAIL_INTERPOSABLE
- && !alias->instrumented_version);
- }
- else
- {
- create_alias = true;
- create_thunk = false;
- redirect_callers = false;
- }
+ = alias->get_availability () > AVAIL_INTERPOSABLE
+ && original->get_availability () > AVAIL_INTERPOSABLE
+ && !alias->instrumented_version;
- /* We want thunk to always jump to the local function body
- unless the body is comdat and may be optimized out. */
- if ((create_thunk || redirect_callers)
- && (!original_discardable
+ if (!redirect_callers && !create_wrapper)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; can not redirect callers nor "
+ "produce wrapper\n\n");
+ return false;
+ }
+
+ /* Work out the symbol the wrapper should call.
+ If ORIGINAL is interposable, we need to call a local alias.
+ Also produce local alias (if possible) as an optimization. */
+ if (!original_discardable
|| (DECL_COMDAT_GROUP (original->decl)
&& (DECL_COMDAT_GROUP (original->decl)
- == DECL_COMDAT_GROUP (alias->decl)))))
- local_original
- = dyn_cast <cgraph_node *> (original->noninterposable_alias ());
-
- if (!local_original)
- {
- if (dump_file)
- fprintf (dump_file, "Noninterposable alias cannot be created.\n\n");
-
- return false;
- }
+ == DECL_COMDAT_GROUP (alias->decl))))
+ {
+ local_original
+ = dyn_cast <cgraph_node *> (original->noninterposable_alias ());
+ if (!local_original
+ && original->get_availability () > AVAIL_INTERPOSABLE)
+ local_original = original;
+ /* If original is COMDAT local, we can not really redirect external
+ callers to it. */
+ if (original->comdat_local_p ())
+ redirect_callers = false;
+ }
+ /* If we can not use local alias, fallback to the original
+ when possible. */
+ else if (original->get_availability () > AVAIL_INTERPOSABLE)
+ local_original = original;
+ if (!local_original)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; "
+ "can not produce local alias.\n\n");
+ return false;
+ }
- if (!decl_binds_to_current_def_p (alias->decl))
- {
- if (dump_file)
- fprintf (dump_file, "Declaration does not bind to currect definition.\n\n");
- return false;
+ if (!redirect_callers && !create_wrapper)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; "
+ "can not redirect callers nor produce a wrapper\n\n");
+ return false;
+ }
+ if (!create_wrapper
+ && !alias->can_remove_if_no_direct_calls_p ())
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; can not make wrapper and "
+ "function has other uses than direct calls\n\n");
+ return false;
+ }
}
+ else
+ create_alias = true;
if (redirect_callers)
{
- /* If alias is non-overwritable then
- all direct calls are safe to be redirected to the original. */
- bool redirected = false;
- while (alias->callers)
- {
- cgraph_edge *e = alias->callers;
- e->redirect_callee (local_original);
- push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl));
+ int nredirected = redirect_all_callers (alias, local_original);
- if (e->call_stmt)
- e->redirect_call_stmt_to_callee ();
+ if (nredirected)
+ {
+ alias->icf_merged = true;
+ local_original->icf_merged = true;
- pop_cfun ();
- redirected = true;
+ if (dump_file && nredirected)
+ fprintf (dump_file, "%i local calls have been "
+ "redirected.\n", nredirected);
}
- alias->icf_merged = true;
- if (local_original->lto_file_data
- && alias->lto_file_data
- && local_original->lto_file_data != alias->lto_file_data)
- local_original->merged = true;
-
- /* The alias function is removed if symbol address
- does not matter. */
- if (!alias_address_matters)
- alias->remove ();
-
- if (dump_file && redirected)
- fprintf (dump_file, "Callgraph local calls have been redirected.\n\n");
+ /* If all callers was redirected, do not produce wrapper. */
+ if (alias->can_remove_if_no_direct_calls_p ()
+ && !alias->has_aliases_p ())
+ {
+ create_wrapper = false;
+ remove = true;
+ }
+ gcc_assert (!create_alias);
}
- /* If the condtion above is not met, we are lucky and can turn the
- function into real alias. */
else if (create_alias)
{
alias->icf_merged = true;
- if (local_original->lto_file_data
- && alias->lto_file_data
- && local_original->lto_file_data != alias->lto_file_data)
- local_original->merged = true;
/* Remove the function's body. */
ipa_merge_profiles (original, alias);
@@ -791,39 +903,38 @@ sem_function::merge (sem_item *alias_ite
(set_local, (void *)(size_t) original->local_p (), true);
if (dump_file)
- fprintf (dump_file, "Callgraph alias has been created.\n\n");
+ fprintf (dump_file, "Unified; Function alias has been created.\n\n");
}
- else if (create_thunk)
+ if (create_wrapper)
{
- if (DECL_COMDAT_GROUP (alias->decl))
- {
- if (dump_file)
- fprintf (dump_file, "Callgraph thunk cannot be created because of COMDAT\n");
+ gcc_assert (!create_alias);
+ alias->icf_merged = true;
+ local_original->icf_merged = true;
- return 0;
- }
+ ipa_merge_profiles (local_original, alias, true);
+ alias->create_wrapper (local_original);
- if (DECL_STATIC_CHAIN (alias->decl))
- {
- if (dump_file)
- fprintf (dump_file, "Thunk creation is risky for static-chain functions.\n\n");
+ if (dump_file)
+ fprintf (dump_file, "Unified; Wrapper has been created.\n\n");
+ }
+ gcc_assert (alias->icf_merged || remove);
+ original->icf_merged = true;
- return 0;
- }
+ /* Inform the inliner about cross-module merging. */
+ if ((original->lto_file_data || alias->lto_file_data)
+ && original->lto_file_data != alias->lto_file_data)
+ local_original->merged = original->merged = true;
+ if (remove)
+ {
+ ipa_merge_profiles (original, alias);
+ alias->release_body ();
+ alias->reset ();
+ alias->body_removed = true;
alias->icf_merged = true;
- if (local_original->lto_file_data
- && alias->lto_file_data
- && local_original->lto_file_data != alias->lto_file_data)
- local_original->merged = true;
- ipa_merge_profiles (local_original, alias, true);
- alias->create_wrapper (local_original);
-
if (dump_file)
- fprintf (dump_file, "Callgraph thunk has been created.\n\n");
+ fprintf (dump_file, "Unified; Function body was removed.\n");
}
- else if (dump_file)
- fprintf (dump_file, "Callgraph merge operation cannot be performed.\n\n");
return true;
}
@@ -1319,7 +1430,8 @@ sem_variable::merge (sem_item *alias_ite
if (!sem_item::target_supports_symbol_aliases_p ())
{
if (dump_file)
- fprintf (dump_file, "Symbol aliases are not supported by target\n\n");
+ fprintf (dump_file, "Not unifying; "
+ "Symbol aliases are not supported by target\n\n");
return false;
}
@@ -1329,73 +1441,94 @@ sem_variable::merge (sem_item *alias_ite
varpool_node *alias = alias_var->get_node ();
bool original_discardable = false;
+ bool original_address_matters = original->address_matters_p ();
+ bool alias_address_matters = alias->address_matters_p ();
+
/* See if original is in a section that can be discarded if the main
- symbol is not used. */
- if (DECL_EXTERNAL (original->decl))
- original_discardable = true;
- if (original->resolution == LDPR_PREEMPTED_REG
- || original->resolution == LDPR_PREEMPTED_IR)
- original_discardable = true;
- if (original->can_be_discarded_p ())
+ symbol is not used.
+ Also consider case where we have resolution info and we know that
+ original's definition is not going to be used. In this case we can not
+ create alias to original. */
+ if (original->can_be_discarded_p ()
+ || (node->resolution != LDPR_UNKNOWN
+ && !decl_binds_to_current_def_p (node->decl)))
original_discardable = true;
gcc_assert (!TREE_ASM_WRITTEN (alias->decl));
- if (original_discardable || DECL_EXTERNAL (alias_var->decl) ||
- !compare_sections (alias_var))
+ /* Constant pool machinery is not quite ready for aliases.
+ TODO: varasm code contains logic for merging DECL_IN_CONSTANT_POOL.
+ For LTO merging does not happen that is an important missing feature.
+ We can enable merging with LTO if the DECL_IN_CONSTANT_POOL
+ flag is dropped and non-local symbol name is assigned. */
+ if (DECL_IN_CONSTANT_POOL (alias->decl)
+ || DECL_IN_CONSTANT_POOL (original->decl))
{
if (dump_file)
- fprintf (dump_file, "Varpool alias cannot be created\n\n");
+ fprintf (dump_file,
+ "Not unifying; constant pool variables.\n\n");
+ return false;
+ }
+ /* Do not attempt to mix functions from different user sections;
+ we do not know what user intends with those. */
+ if (((DECL_SECTION_NAME (original->decl) && !original->implicit_section)
+ || (DECL_SECTION_NAME (alias->decl) && !alias->implicit_section))
+ && DECL_SECTION_NAME (original->decl) != DECL_SECTION_NAME (alias->decl))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ "Not unifying; "
+ "original and alias are in different sections.\n\n");
return false;
}
- else
+
+ /* We can not merge if address comparsion metters. */
+ if (original_address_matters && alias_address_matters
+ && flag_merge_constants < 2)
{
- // alias cycle creation check
- varpool_node *n = original;
+ if (dump_file)
+ fprintf (dump_file,
+ "Not unifying; "
+ "adress of original and alias may be compared.\n\n");
+ return false;
+ }
- while (n->alias)
- {
- n = n->get_alias_target ();
- if (n == alias)
- {
- if (dump_file)
- fprintf (dump_file, "Varpool alias cannot be created (alias cycle).\n\n");
+ if (original_discardable
+ && (!DECL_COMDAT_GROUP (original->decl)
+ || (DECL_COMDAT_GROUP (original->decl)
+ != DECL_COMDAT_GROUP (alias->decl))))
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not unifying; alias cannot be created; "
+ "target is discardable\n\n");
- return false;
- }
- }
+ return false;
+ }
+ else
+ {
+ gcc_assert (!original->alias);
+ gcc_assert (!alias->alias);
alias->analyzed = false;
DECL_INITIAL (alias->decl) = NULL;
alias->need_bounds_init = false;
alias->remove_all_references ();
+ if (TREE_ADDRESSABLE (alias->decl))
+ original->call_for_symbol_thunks_and_aliases
+ (set_addressable, NULL, true);
varpool_node::create_alias (alias_var->decl, decl);
alias->resolve_alias (original);
if (dump_file)
- fprintf (dump_file, "Varpool alias has been created.\n\n");
+ fprintf (dump_file, "Unified; Variable alias has been created.\n\n");
return true;
}
}
-bool
-sem_variable::compare_sections (sem_variable *alias)
-{
- const char *source = node->get_section ();
- const char *target = alias->node->get_section();
-
- if (source == NULL && target == NULL)
- return true;
- else if(!source || !target)
- return false;
- else
- return strcmp (source, target) == 0;
-}
-
/* Dump symbol to FILE. */
void
Index: cgraphclones.c
===================================================================
--- cgraphclones.c (revision 221034)
+++ cgraphclones.c (working copy)
@@ -471,6 +471,8 @@ cgraph_node::create_clone (tree decl, gc
new_node->frequency = frequency;
new_node->tp_first_run = tp_first_run;
new_node->tm_clone = tm_clone;
+ new_node->icf_merged = icf_merged;
+ new_node->merged = merged;
new_node->clone.tree_map = NULL;
new_node->clone.args_to_skip = args_to_skip;
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 5:55 ` Jan Hubicka
@ 2015-02-27 13:48 ` H.J. Lu
2015-02-27 18:04 ` Jan Hubicka
2015-02-27 18:39 ` Steve Ellcey
2015-02-28 16:38 ` James Greenhalgh
2 siblings, 1 reply; 26+ messages in thread
From: H.J. Lu @ 2015-02-27 13:48 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jack Howarth, GCC Patches, Martin Liška, Jakub Jelinek
On Thu, Feb 26, 2015 at 6:10 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this is the final version of patch I comitted. It has new fix to make_decl_local
> to set TREE_ADDRESSABLE becuase we leave the flag undefined for non-local decls.
> I also dropped Optimization from fmerge-all-constants, fmerge-constants
> those can not be done in function speicfic way, I made ipa_ref::address_matters_p
> to use fmerge-constants, added code to drop UNINLINABLE flag when function is turned
> into a wrapper, added check to require DECL_NO_INLINE_WARNING_P match
> and added code to set TREE_ADDRESSABLE when non-addressable and addressable vars are merged.
> I also disabled merging for DECL_CONSTANT_POOL because it does not work (symtab does not
> expect aliases here)
>
> Bootstrapped/regtested x86_64-linux, comitted.
>
> Honza
> * ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton):
> Use address_matters_p.
> (redirect_all_callers, set_addressable): New functions.
> (sem_function::merge): Reorganize and fix merging issues.
> (sem_variable::merge): Likewise.
> (sem_variable::compare_sections): Remove.
> * common.opt (fmerge-all-constants, fmerge-constants): Remove
> Optimization flag.
> * symtab.c (symtab_node::resolve_alias): When alias has aliases,
> redirect them.
> (symtab_node::make_decl_local): Set ADDRESSABLE bit when
> decl is used.
> (address_matters_1): New function.
> (symtab_node::address_matters_p): New function.
> * cgraph.c (cgraph_edge::verify_corresponds_to_fndecl): Fix
> check for merged flag.
> * cgraph.h (address_matters_p): Declare.
> (symtab_node::address_taken_from_non_vtable_p): Remove.
> (symtab_node::address_can_be_compared_p): New method.
> (ipa_ref::address_matters_p): Move here from ipa-ref.c; simplify.
> * ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p):
> Remove.
> (comdat_can_be_unshared_p_1) Use address_matters_p.
> (update_vtable_references): Fix formating.
> * ipa-ref.c (ipa_ref::address_matters_p): Move inline.
> * cgraphunit.c (cgraph_node::create_wrapper): Drop UNINLINABLE flag.
> * cgraphclones.c: Preserve merged and icf_merged flags.
>
This caused:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65237
--
H.J.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 13:48 ` H.J. Lu
@ 2015-02-27 18:04 ` Jan Hubicka
0 siblings, 0 replies; 26+ messages in thread
From: Jan Hubicka @ 2015-02-27 18:04 UTC (permalink / raw)
To: H.J. Lu
Cc: Jan Hubicka, Jack Howarth, GCC Patches, Martin Liška, Jakub Jelinek
>
> This caused:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65237
Hi,
this is patch I commited. gcc.dg/attr-noinline.c has template that counts number of calls
in optimized assembler. Those do not match if one function is turned into another's wrapper.
gcc.dg/noreturn-7.c misses one warning because we unify the functions before outputting it.
I think that is OK given that the warning will come out if user fix the first instance.
gcc.dg/ipa/ipa-cp-1.c, gcc.dg/ipa/ipa-cp-2.c was accidental commits from my work with
Martin Jambor, sorry for that.
There is still gcc.target/i386/stackalign/longlong-2.c that is real bug of alignments not
being compared. I noticed that independnetly yesterday and asked Martin to add patch
(among with several other details)
Honza
PR ipa/65237
* gcc.dg/attr-noinline.c: Add -fno-ipa-icf
* gcc.dg/noreturn-7.c: Add -fno-ipa-icf.
* gcc.dg/ipa/ipa-cp-1.c: Revert accidental commit.
* gcc.dg/ipa/ipa-cp-2.c: Revert accidental commit.
Index: gcc.dg/attr-noinline.c
===================================================================
--- gcc.dg/attr-noinline.c (revision 221034)
+++ gcc.dg/attr-noinline.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O2 -finline-functions" } */
+/* { dg-options "-O2 -finline-functions -fno-ipa-icf" } */
extern int t();
Index: gcc.dg/noreturn-7.c
===================================================================
--- gcc.dg/noreturn-7.c (revision 221034)
+++ gcc.dg/noreturn-7.c (working copy)
@@ -5,7 +5,7 @@
in presence of tail recursion within a noreturn function. */
/* { dg-do compile } */
-/* { dg-options "-O2 -Wreturn-type -Wmissing-noreturn" } */
+/* { dg-options "-O2 -Wreturn-type -Wmissing-noreturn -fno-ipa-icf" } */
void f(void) __attribute__ ((__noreturn__));
Index: gcc.dg/ipa/ipa-cp-1.c
===================================================================
--- gcc.dg/ipa/ipa-cp-1.c (revision 221034)
+++ gcc.dg/ipa/ipa-cp-1.c (working copy)
@@ -1,22 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -fdump-ipa-cp" } */
-int n;
-
-static void
-__attribute__ ((noinline))
-test(void *a)
-{
- __builtin_memset (a,0,n);
-}
-
-int
-main()
-{
- int aa;
- short bb;
- test (&aa);
- test (&bb);
- return 0;
-}
-/* { dg-final { scan-ipa-dump "Alignment 2" "cp" } } */
-/* { dg-final { cleanup-ipa-dump "cp" } } */
Index: gcc.dg/ipa/ipa-cp-2.c
===================================================================
--- gcc.dg/ipa/ipa-cp-2.c (revision 221034)
+++ gcc.dg/ipa/ipa-cp-2.c (working copy)
@@ -1,22 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-O2 -fdump-ipa-cp" } */
-int n;
-
-static void
-__attribute__ ((noinline))
-test(void *a)
-{
- __builtin_memset (a,0,n);
-}
-
-static __attribute__ ((aligned(16))) int aa[10];
-
-int
-main()
-{
- test (&aa[1]);
- test (&aa[3]);
- return 0;
-}
-/* { dg-final { scan-ipa-dump "Alignment 8, misalignment 4" "cp" } } */
-/* { dg-final { cleanup-ipa-dump "cp" } } */
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 5:55 ` Jan Hubicka
2015-02-27 13:48 ` H.J. Lu
@ 2015-02-27 18:39 ` Steve Ellcey
2015-02-27 18:56 ` Steve Ellcey
2015-02-27 21:16 ` Jan Hubicka
2015-02-28 16:38 ` James Greenhalgh
2 siblings, 2 replies; 26+ messages in thread
From: Steve Ellcey @ 2015-02-27 18:39 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jack Howarth, GCC Patches, mliska, Jakub Jelinek
On Fri, 2015-02-27 at 03:10 +0100, Jan Hubicka wrote:
> Bootstrapped/regtested x86_64-linux, comitted.
>
> Honza
> * ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton):
> Use address_matters_p.
I think this patch is causing an ICE while building glibc on MIPS. I am
building a toolchain for mips-mti-linux-gnu and when compiling
sysdeps/gnu/siglist.c from glibc for mips64r2 (N32 ABI) I get the
following ICE.
I will try to create a preprocessed source file for this but I wanted
to report it first to see if anyone else is seeing it on other
platforms.
Steve Ellcey
sellcey@imgtec.com
../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
^
0x66a080 symtab_node::address_matters_p()
/scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
/scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
0xe81ff9 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
/scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:2659
0xe86491 ipa_icf::sem_item_optimizer::execute()
/scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1923
0xe885a1 ipa_icf_driver
/scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:2738
0xe885a1 ipa_icf::pass_ipa_icf::execute(function*)
/scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:2785
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[2]: *** [/scratch/sellcey/repos/bootstrap/obj-mips-mti-linux-gnu/glibc/obj_mips64r2/stdio-common/siglist.os] Error 1
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 18:39 ` Steve Ellcey
@ 2015-02-27 18:56 ` Steve Ellcey
2015-02-27 20:00 ` Martin Liška
2015-02-27 21:16 ` Jan Hubicka
1 sibling, 1 reply; 26+ messages in thread
From: Steve Ellcey @ 2015-02-27 18:56 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jack Howarth, GCC Patches, mliska, Jakub Jelinek
On Fri, 2015-02-27 at 09:33 -0800, Steve Ellcey wrote:
> On Fri, 2015-02-27 at 03:10 +0100, Jan Hubicka wrote:
>
> > Bootstrapped/regtested x86_64-linux, comitted.
> >
> > Honza
> > * ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton):
> > Use address_matters_p.
>
> I think this patch is causing an ICE while building glibc on MIPS. I am
> building a toolchain for mips-mti-linux-gnu and when compiling
> sysdeps/gnu/siglist.c from glibc for mips64r2 (N32 ABI) I get the
> following ICE.
>
> I will try to create a preprocessed source file for this but I wanted
> to report it first to see if anyone else is seeing it on other
> platforms.
>
> Steve Ellcey
> sellcey@imgtec.com
Following up to my own email. I can reproduce this with the following
cut down test case if I compile with '-O2 -fmerge-all-constants' on
MIPS.
extern const char *const _sys_siglist[128];
const char *const __new_sys_siglist[128] = { };
extern __typeof (_sys_siglist) __EI__sys_siglist __attribute__((alias ("" "__new_sys_siglist")));
extern __typeof (__new_sys_siglist) _new_sys_siglist __attribute__ ((alias ("__new_sys_siglist")));
Steve Ellcey
sellcey@imgtec.com
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 18:56 ` Steve Ellcey
@ 2015-02-27 20:00 ` Martin Liška
0 siblings, 0 replies; 26+ messages in thread
From: Martin Liška @ 2015-02-27 20:00 UTC (permalink / raw)
To: gcc-patches
On 02/27/2015 07:04 PM, Steve Ellcey wrote:
> Following up to my own email. I can reproduce this with the following
> cut down test case if I compile with '-O2 -fmerge-all-constants' on
> MIPS.
>
> extern const char *const _sys_siglist[128];
> const char *const __new_sys_siglist[128] = { };
> extern __typeof (_sys_siglist) __EI__sys_siglist __attribute__((alias ("" "__new_sys_siglist")));
> extern __typeof (__new_sys_siglist) _new_sys_siglist __attribute__ ((alias ("__new_sys_siglist")));
>
> Steve Ellcey
> sellcey@imgtec.com
Hello.
I've just created PR65245, where I've attached suggested patch I'm testing.
Thanks,
Martin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 18:39 ` Steve Ellcey
2015-02-27 18:56 ` Steve Ellcey
@ 2015-02-27 21:16 ` Jan Hubicka
2015-03-01 16:47 ` Christophe Lyon
1 sibling, 1 reply; 26+ messages in thread
From: Jan Hubicka @ 2015-02-27 21:16 UTC (permalink / raw)
To: Steve Ellcey
Cc: Jan Hubicka, Jack Howarth, GCC Patches, mliska, Jakub Jelinek
>
> ../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
> ^
> 0x66a080 symtab_node::address_matters_p()
> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
> 0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
Indeed, the ipa-icf should not try to analyze aliases - just prove ekvialence of
definitions they are attached to. It already does that for functions (bit by accident;
it gives up when there is no gimple body), but it does not do that for variables because
it gets into ctor_for_folding. For that reason it sometimes decides to try to make two
variable aliases alias of each other that is not a good idea, because of possible creation
of loops.
I am just discussing with Martin the fix.
Honza
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 5:55 ` Jan Hubicka
2015-02-27 13:48 ` H.J. Lu
2015-02-27 18:39 ` Steve Ellcey
@ 2015-02-28 16:38 ` James Greenhalgh
2015-02-28 18:15 ` Jan Hubicka
2 siblings, 1 reply; 26+ messages in thread
From: James Greenhalgh @ 2015-02-28 16:38 UTC (permalink / raw)
To: Jan Hubicka
Cc: Jack Howarth, GCC Patches, mliska, Jakub Jelinek, Alan Lawrence,
Alex Velenko
On Fri, Feb 27, 2015 at 02:10:47AM +0000, Jan Hubicka wrote:
> * ipa-icf.c (symbol_compare_collection::symbol_compare_colleciton):
> Use address_matters_p.
> (redirect_all_callers, set_addressable): New functions.
> (sem_function::merge): Reorganize and fix merging issues.
> (sem_variable::merge): Likewise.
> (sem_variable::compare_sections): Remove.
> * common.opt (fmerge-all-constants, fmerge-constants): Remove
> Optimization flag.
> * symtab.c (symtab_node::resolve_alias): When alias has aliases,
> redirect them.
> (symtab_node::make_decl_local): Set ADDRESSABLE bit when
> decl is used.
> (address_matters_1): New function.
> (symtab_node::address_matters_p): New function.
> * cgraph.c (cgraph_edge::verify_corresponds_to_fndecl): Fix
> check for merged flag.
> * cgraph.h (address_matters_p): Declare.
> (symtab_node::address_taken_from_non_vtable_p): Remove.
> (symtab_node::address_can_be_compared_p): New method.
> (ipa_ref::address_matters_p): Move here from ipa-ref.c; simplify.
> * ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p):
> Remove.
> (comdat_can_be_unshared_p_1) Use address_matters_p.
> (update_vtable_references): Fix formating.
> * ipa-ref.c (ipa_ref::address_matters_p): Move inline.
> * cgraphunit.c (cgraph_node::create_wrapper): Drop UNINLINABLE flag.
> * cgraphclones.c: Preserve merged and icf_merged flags.
>
> * gcc.dg/pr64454.c: Disable ICF.
> * gcc.dg/pr28685-1.c: Disable ICF
> * gcc.dg/ipa/iinline-5.c: Disable ICF.
> * g++.dg/warn/Wsuggest-final.C: Force methods to be different.
> * g++.dg/ipa/ipa-icf-4.C: Update template.
Hi Honza,
This is more a note for other interested AArch64 testers, but this patch
breaks some tests on aarch64-none-elf. Looking at the test output, this
is a problem with the tests than with your patch. We now eliminate some
function bodies which are identical across test functions, causing us to
fail some scan-assembler directives.
The fix is straightforward, we just need to add -fno-ipa-icf to get things
working again. This keeps the spirit of the original tests. I'll propose
a patch along those lines on Monday.
Thanks,
James
---
PASS->FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times ldaxr\tw[0-9]+, \\[x[0-9]+\\] 4
PASS->FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times stlxr\tw[0-9]+, w[0-9]+, \\[x[0-9]+\\] 4
PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 2
PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 2
PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 5
PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 5
PASS->FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl2\tv[0-9]+.8h 5
PASS->FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl\tv[0-9]+.8h 5
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-28 16:38 ` James Greenhalgh
@ 2015-02-28 18:15 ` Jan Hubicka
2015-03-04 9:38 ` James Greenhalgh
0 siblings, 1 reply; 26+ messages in thread
From: Jan Hubicka @ 2015-02-28 18:15 UTC (permalink / raw)
To: James Greenhalgh
Cc: Jan Hubicka, Jack Howarth, GCC Patches, mliska, Jakub Jelinek,
Alan Lawrence, Alex Velenko
>
> Hi Honza,
>
> This is more a note for other interested AArch64 testers, but this patch
> breaks some tests on aarch64-none-elf. Looking at the test output, this
> is a problem with the tests than with your patch. We now eliminate some
> function bodies which are identical across test functions, causing us to
> fail some scan-assembler directives.
>
> The fix is straightforward, we just need to add -fno-ipa-icf to get things
> working again. This keeps the spirit of the original tests. I'll propose
> a patch along those lines on Monday.
>
> Thanks,
> James
>
> ---
>
> PASS->FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times ldaxr\tw[0-9]+, \\[x[0-9]+\\] 4
> PASS->FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times stlxr\tw[0-9]+, w[0-9]+, \\[x[0-9]+\\] 4
> PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 2
> PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 2
> PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 5
> PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 5
> PASS->FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl2\tv[0-9]+.8h 5
> PASS->FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl\tv[0-9]+.8h 5
Thanks, yes adding -fno-ipa-icf is a proper fix for tests like this.
What happened was a logic bug in old implementation of merge routine. When
symbol is externally visible it decided to create a wrapper (to preserve
potential address compares) and to avoid wrapper cost redirect all direct
uses.
There was extra else between redirection and thunk creation, so often it
redirected calls but it kept old function body in code. We now see a lot more
DCE tham before.
i386 tests also needed a compensation at two places.
Honza
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-27 21:16 ` Jan Hubicka
@ 2015-03-01 16:47 ` Christophe Lyon
2015-03-02 19:01 ` Alex Velenko
0 siblings, 1 reply; 26+ messages in thread
From: Christophe Lyon @ 2015-03-01 16:47 UTC (permalink / raw)
To: Jan Hubicka
Cc: Steve Ellcey, Jack Howarth, GCC Patches, Martin Liška,
Jakub Jelinek
On 27 February 2015 at 21:49, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>> ../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
>> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
>> ^
>> 0x66a080 symtab_node::address_matters_p()
>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
>> 0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
>
> Indeed, the ipa-icf should not try to analyze aliases - just prove ekvialence of
> definitions they are attached to. It already does that for functions (bit by accident;
> it gives up when there is no gimple body), but it does not do that for variables because
> it gets into ctor_for_folding. For that reason it sometimes decides to try to make two
> variable aliases alias of each other that is not a good idea, because of possible creation
> of loops.
>
> I am just discussing with Martin the fix.
>
> Honza
For the record, I have noticed similar errors on ARM and AArch64
targets, when building glibc.
Christophe.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-01 16:47 ` Christophe Lyon
@ 2015-03-02 19:01 ` Alex Velenko
2015-03-02 20:21 ` Jan Hubicka
0 siblings, 1 reply; 26+ messages in thread
From: Alex Velenko @ 2015-03-02 19:01 UTC (permalink / raw)
To: Christophe Lyon, Jan Hubicka
Cc: Steve Ellcey, Jack Howarth, GCC Patches, Martin Liška,
Jakub Jelinek, James Greenhalgh
On 01/03/15 16:47, Christophe Lyon wrote:
> On 27 February 2015 at 21:49, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>
>>> ../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
>>> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
>>> ^
>>> 0x66a080 symtab_node::address_matters_p()
>>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
>>> 0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
>>
>> Indeed, the ipa-icf should not try to analyze aliases - just prove ekvialence of
>> definitions they are attached to. It already does that for functions (bit by accident;
>> it gives up when there is no gimple body), but it does not do that for variables because
>> it gets into ctor_for_folding. For that reason it sometimes decides to try to make two
>> variable aliases alias of each other that is not a good idea, because of possible creation
>> of loops.
>>
>> I am just discussing with Martin the fix.
>>
>> Honza
>
> For the record, I have noticed similar errors on ARM and AArch64
> targets, when building glibc.
>
> Christophe.
>
I confirm ARM and AArch64 failing to build with this patch:
/work/build-aarch64-none-linux-gnu/install//bin/aarch64-none-linux-gnu-gcc
../sysdeps/posix/cuserid.c -c -std=gnu99 -fgnu89-inline -O2 -Wall
-Winline -Wundef -Wwrite-strings -fmerge-all-constants -frounding-math
-g -Wstrict-prototypes -fPIC -fexceptions -I../include
-I/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common
-I/work/build-aarch64-none-linux-gnu/obj/glibc
-I../sysdeps/unix/sysv/linux/aarch64 -I../sysdeps/aarch64/nptl
-I../sysdeps/unix/sysv/linux/generic
-I../sysdeps/unix/sysv/linux/wordsize-64
-I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux
-I../sysdeps/nptl -I../sysdeps/pthread -I../sysdeps/gnu
-I../sysdeps/unix/inet -I../sysdeps/unix/sysv -I../sysdeps/unix
-I../sysdeps/posix -I../sysdeps/aarch64/fpu -I../sysdeps/aarch64
-I../sysdeps/wordsize-64 -I../sysdeps/ieee754/ldbl-128
-I../sysdeps/ieee754/dbl-64/wordsize-64 -I../sysdeps/ieee754/dbl-64
-I../sysdeps/ieee754/flt-32 -I../sysdeps/aarch64/soft-fp
-I../sysdeps/ieee754 -I../sysdeps/generic -I.. -I../libio -I.
-nostdinc -isystem
/work/build-aarch64-none-linux-gnu/install/bin/../lib/gcc/aarch64-none-linux-gnu/5.0.0/include
-isystem
/work/build-aarch64-none-linux-gnu/install/bin/../lib/gcc/aarch64-none-linux-gnu/5.0.0/include-fixed
-isystem
/work/build-aarch64-none-linux-gnu/install//aarch64-none-linux-gnu/libc/usr/include
-D_LIBC_REENTRANT -include
/work/build-aarch64-none-linux-gnu/obj/glibc/libc-modules.h
-DMODULE_NAME=libc -include ../include/libc-symbols.h -DPIC -DSHARED
-D_IO_MTSAFE_IO -o
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/cuserid.os -MD
-MP -MF
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/cuserid.os.dt
-MT /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/cuserid.os
../sysdeps/gnu/siglist.c:77:1: internal compiler error: in
address_matters_p, at symtab.c:1908
versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_1);
^
*** errlist.c count 134 inflated to GLIBC_2.12 count 135 (old errno.h?)
chmod a-w
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
0x6b9100 symtab_node::address_matters_p()
/work/src/gcc/gcc/symtab.c:1908
0xedb4e5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
/work/src/gcc/gcc/ipa-icf.c:1723
0xee03f9 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
/work/src/gcc/gcc/ipa-icf.c:2955
0xee6d31 ipa_icf::sem_item_optimizer::execute()
/work/src/gcc/gcc/ipa-icf.c:2217
0xee8df1 ipa_icf_driver
/work/src/gcc/gcc/ipa-icf.c:3034
0xee8df1 ipa_icf::pass_ipa_icf::execute(function*)
/work/src/gcc/gcc/ipa-icf.c:3081
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[2]: ***
[/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o]
Error 1
make[2]: *** Waiting for unfinished jobs....
mv -f
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.c
Regards,
Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-02 19:01 ` Alex Velenko
@ 2015-03-02 20:21 ` Jan Hubicka
2015-03-02 22:04 ` Christophe Lyon
0 siblings, 1 reply; 26+ messages in thread
From: Jan Hubicka @ 2015-03-02 20:21 UTC (permalink / raw)
To: Alex Velenko
Cc: Christophe Lyon, Jan Hubicka, Steve Ellcey, Jack Howarth,
GCC Patches, Martin Liška, Jakub Jelinek, James Greenhalgh
>
>
> On 01/03/15 16:47, Christophe Lyon wrote:
> >On 27 February 2015 at 21:49, Jan Hubicka <hubicka@ucw.cz> wrote:
> >>>
> >>>../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
> >>> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
> >>> ^
> >>>0x66a080 symtab_node::address_matters_p()
> >>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
> >>>0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
> >>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
> >>
> >>Indeed, the ipa-icf should not try to analyze aliases - just prove ekvialence of
> >>definitions they are attached to. It already does that for functions (bit by accident;
> >>it gives up when there is no gimple body), but it does not do that for variables because
> >>it gets into ctor_for_folding. For that reason it sometimes decides to try to make two
> >>variable aliases alias of each other that is not a good idea, because of possible creation
> >>of loops.
> >>
> >>I am just discussing with Martin the fix.
> >>
> >>Honza
> >
> >For the record, I have noticed similar errors on ARM and AArch64
> >targets, when building glibc.
> >
> >Christophe.
> >
>
> I confirm ARM and AArch64 failing to build with this patch:
> chmod a-w /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
> 0x6b9100 symtab_node::address_matters_p()
> /work/src/gcc/gcc/symtab.c:1908
> 0xedb4e5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
> /work/src/gcc/gcc/ipa-icf.c:1723
> 0xee03f9 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
> /work/src/gcc/gcc/ipa-icf.c:2955
> 0xee6d31 ipa_icf::sem_item_optimizer::execute()
> /work/src/gcc/gcc/ipa-icf.c:2217
> 0xee8df1 ipa_icf_driver
> /work/src/gcc/gcc/ipa-icf.c:3034
> 0xee8df1 ipa_icf::pass_ipa_icf::execute(function*)
> /work/src/gcc/gcc/ipa-icf.c:3081
I commited patch for the alias merging yesterda night, so it should be fixed
now. If it still fails, please fill in a PR with preprocessed testcase so I
can reproduce it in a cross.
Honza
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> make[2]: ***
> [/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o]
> Error 1
> make[2]: *** Waiting for unfinished jobs....
> mv -f /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.c
>
> Regards,
> Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-02 20:21 ` Jan Hubicka
@ 2015-03-02 22:04 ` Christophe Lyon
2015-03-03 12:44 ` Alex Velenko
0 siblings, 1 reply; 26+ messages in thread
From: Christophe Lyon @ 2015-03-02 22:04 UTC (permalink / raw)
To: Jan Hubicka
Cc: Alex Velenko, Steve Ellcey, Jack Howarth, GCC Patches,
Martin Liška, Jakub Jelinek, James Greenhalgh
On 2 March 2015 at 21:21, Jan Hubicka <hubicka@ucw.cz> wrote:
>>
>>
>> On 01/03/15 16:47, Christophe Lyon wrote:
>> >On 27 February 2015 at 21:49, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >>>
>> >>>../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
>> >>> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
>> >>> ^
>> >>>0x66a080 symtab_node::address_matters_p()
>> >>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
>> >>>0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>> >>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
>> >>
>> >>Indeed, the ipa-icf should not try to analyze aliases - just prove ekvialence of
>> >>definitions they are attached to. It already does that for functions (bit by accident;
>> >>it gives up when there is no gimple body), but it does not do that for variables because
>> >>it gets into ctor_for_folding. For that reason it sometimes decides to try to make two
>> >>variable aliases alias of each other that is not a good idea, because of possible creation
>> >>of loops.
>> >>
>> >>I am just discussing with Martin the fix.
>> >>
>> >>Honza
>> >
>> >For the record, I have noticed similar errors on ARM and AArch64
>> >targets, when building glibc.
>> >
>> >Christophe.
>> >
>>
>> I confirm ARM and AArch64 failing to build with this patch:
>> chmod a-w /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
>> 0x6b9100 symtab_node::address_matters_p()
>> /work/src/gcc/gcc/symtab.c:1908
>> 0xedb4e5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>> /work/src/gcc/gcc/ipa-icf.c:1723
>> 0xee03f9 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
>> /work/src/gcc/gcc/ipa-icf.c:2955
>> 0xee6d31 ipa_icf::sem_item_optimizer::execute()
>> /work/src/gcc/gcc/ipa-icf.c:2217
>> 0xee8df1 ipa_icf_driver
>> /work/src/gcc/gcc/ipa-icf.c:3034
>> 0xee8df1 ipa_icf::pass_ipa_icf::execute(function*)
>> /work/src/gcc/gcc/ipa-icf.c:3081
>
> I commited patch for the alias merging yesterda night, so it should be fixed
> now. If it still fails, please fill in a PR with preprocessed testcase so I
> can reproduce it in a cross.
>
On my side, I saw builds complete again starting with r221090, I guess
that's the commit you are referring to?
Thanks,
Christophe.
> Honza
>> Please submit a full bug report,
>> with preprocessed source if appropriate.
>> Please include the complete backtrace with any bug report.
>> See <http://gcc.gnu.org/bugs.html> for instructions.
>> make[2]: ***
>> [/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o]
>> Error 1
>> make[2]: *** Waiting for unfinished jobs....
>> mv -f /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.c
>>
>> Regards,
>> Alex
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-02 22:04 ` Christophe Lyon
@ 2015-03-03 12:44 ` Alex Velenko
2015-03-03 15:06 ` Christophe Lyon
0 siblings, 1 reply; 26+ messages in thread
From: Alex Velenko @ 2015-03-03 12:44 UTC (permalink / raw)
To: Christophe Lyon, Jan Hubicka
Cc: Steve Ellcey, Jack Howarth, GCC Patches, Martin Liška,
Jakub Jelinek, James Greenhalgh
On 02/03/15 22:04, Christophe Lyon wrote:
> On 2 March 2015 at 21:21, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>
>>>
>>> On 01/03/15 16:47, Christophe Lyon wrote:
>>>> On 27 February 2015 at 21:49, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>>
>>>>>> ../sysdeps/gnu/siglist.c:72:1: internal compiler error: in address_matters_p, at symtab.c:1908
>>>>>> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_3_3);
>>>>>> ^
>>>>>> 0x66a080 symtab_node::address_matters_p()
>>>>>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
>>>>>> 0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>>>>>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
>>>>>
>>>>> Indeed, the ipa-icf should not try to analyze aliases - just prove ekvialence of
>>>>> definitions they are attached to. It already does that for functions (bit by accident;
>>>>> it gives up when there is no gimple body), but it does not do that for variables because
>>>>> it gets into ctor_for_folding. For that reason it sometimes decides to try to make two
>>>>> variable aliases alias of each other that is not a good idea, because of possible creation
>>>>> of loops.
>>>>>
>>>>> I am just discussing with Martin the fix.
>>>>>
>>>>> Honza
>>>>
>>>> For the record, I have noticed similar errors on ARM and AArch64
>>>> targets, when building glibc.
>>>>
>>>> Christophe.
>>>>
>>>
>>> I confirm ARM and AArch64 failing to build with this patch:
>>> chmod a-w /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
>>> 0x6b9100 symtab_node::address_matters_p()
>>> /work/src/gcc/gcc/symtab.c:1908
>>> 0xedb4e5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>>> /work/src/gcc/gcc/ipa-icf.c:1723
>>> 0xee03f9 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
>>> /work/src/gcc/gcc/ipa-icf.c:2955
>>> 0xee6d31 ipa_icf::sem_item_optimizer::execute()
>>> /work/src/gcc/gcc/ipa-icf.c:2217
>>> 0xee8df1 ipa_icf_driver
>>> /work/src/gcc/gcc/ipa-icf.c:3034
>>> 0xee8df1 ipa_icf::pass_ipa_icf::execute(function*)
>>> /work/src/gcc/gcc/ipa-icf.c:3081
>>
>> I commited patch for the alias merging yesterda night, so it should be fixed
>> now. If it still fails, please fill in a PR with preprocessed testcase so I
>> can reproduce it in a cross.
>>
>
> On my side, I saw builds complete again starting with r221090, I guess
> that's the commit you are referring to?
Hi,
I built with r221117. I see errors while building following targets:
aarch64_be-none-linux-gnu, aarch64_be-none-linux-gnu,
arm-none-linux-gnueabihf, arm-none-linux-gnueabi.
For aarch64_be-none-linux-gnu I reproduce the error like this:
/work/build-aarch64-none-linux-gnu/install//bin/aarch64-none-linux-gnu-gcc
/work/src/glibc/sysdeps/gnu/siglist.c -c -std=gnu99 -fgnu89-inline -O2
-Wall -Winline -Wundef -Wwrite-strings -fmerge-all-constants
-frounding-math -g -Wstrict-prototypes -fno-toplevel-reorder
-fno-section-anchors -I/work/src/glibc/include
-I/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common
-I/work/build-aarch64-none-linux-gnu/obj/glibc
-I/work/src/glibc/sysdeps/unix/sysv/linux/aarch64
-I/work/src/glibc/sysdeps/aarch64/nptl
-I/work/src/glibc/sysdeps/unix/sysv/linux/generic
-I/work/src/glibc/sysdeps/unix/sysv/linux/wordsize-64
-I/work/src/glibc/sysdeps/unix/sysv/linux/include
-I/work/src/glibc/sysdeps/unix/sysv/linux
-I/work/src/glibc/sysdeps/nptl -I/work/src/glibc/sysdeps/pthread
-I/work/src/glibc/sysdeps/gnu -I/work/src/glibc/sysdeps/unix/inet
-I/work/src/glibc/sysdeps/unix/sysv -I/work/src/glibc/sysdeps/unix
-I/work/src/glibc/sysdeps/posix -I/work/src/glibc/sysdeps/aarch64/fpu
-I/work/src/glibc/sysdeps/aarch64 -I/work/src/glibc/sysdeps/wordsize-64
-I/work/src/glibc/sysdeps/ieee754/ldbl-128
-I/work/src/glibc/sysdeps/ieee754/dbl-64/wordsize-64
-I/work/src/glibc/sysdeps/ieee754/dbl-64
-I/work/src/glibc/sysdeps/ieee754/flt-32
-I/work/src/glibc/sysdeps/aarch64/soft-fp
-I/work/src/glibc/sysdeps/ieee754 -I/work/src/glibc/sysdeps/generic
-I/work/src/glibc -I/work/src/glibc/libio -I. -nostdinc -isystem
/work/build-aarch64-none-linux-gnu/install/bin/../lib/gcc/aarch64-none-linux-gnu/5.0.0/include
-isystem
/work/build-aarch64-none-linux-gnu/install/bin/../lib/gcc/aarch64-none-linux-gnu/5.0.0/include-fixed
-isystem
/work/build-aarch64-none-linux-gnu/install//aarch64-none-linux-gnu/libc/usr/include
-D_LIBC_REENTRANT -include
/work/build-aarch64-none-linux-gnu/obj/glibc/libc-modules.h
-DMODULE_NAME=libc -include /work/src/glibc/include/libc-symbols.h
-D_IO_MTSAFE_IO -o
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o -MD
-MP -MF
/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o.dt
-MT /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o
/work/src/glibc/sysdeps/gnu/siglist.c:77:1: internal compiler error: in
address_matters_p, at symtab.c:1908
versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_1);
^
0x6b9140 symtab_node::address_matters_p()
/work/src/gcc/gcc/symtab.c:1908
0xedb685 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
/work/src/gcc/gcc/ipa-icf.c:1740
0xee05b1 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
/work/src/gcc/gcc/ipa-icf.c:2979
0xee6f11 ipa_icf::sem_item_optimizer::execute()
/work/src/gcc/gcc/ipa-icf.c:2236
0xee902e ipa_icf_driver
/work/src/gcc/gcc/ipa-icf.c:3061
0xee902e ipa_icf::pass_ipa_icf::execute(function*)
/work/src/gcc/gcc/ipa-icf.c:3108
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
I see a related ticket opened recently:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65287
Kind regards,
Alex
>
> Thanks,
>
> Christophe.
>
>> Honza
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> Please include the complete backtrace with any bug report.
>>> See <http://gcc.gnu.org/bugs.html> for instructions.
>>> make[2]: ***
>>> [/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o]
>>> Error 1
>>> make[2]: *** Waiting for unfinished jobs....
>>> mv -f /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.c
>>>
>>> Regards,
>>> Alex
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-03 12:44 ` Alex Velenko
@ 2015-03-03 15:06 ` Christophe Lyon
2015-03-03 20:01 ` Jan Hubicka
0 siblings, 1 reply; 26+ messages in thread
From: Christophe Lyon @ 2015-03-03 15:06 UTC (permalink / raw)
To: Alex Velenko
Cc: Jan Hubicka, Steve Ellcey, Jack Howarth, GCC Patches,
Martin Liška, Jakub Jelinek, James Greenhalgh
On 3 March 2015 at 13:44, Alex Velenko <Alex.Velenko@arm.com> wrote:
> On 02/03/15 22:04, Christophe Lyon wrote:
>>
>> On 2 March 2015 at 21:21, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>
>>>>
>>>>
>>>> On 01/03/15 16:47, Christophe Lyon wrote:
>>>>>
>>>>> On 27 February 2015 at 21:49, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>>>>>
>>>>>>>
>>>>>>> ../sysdeps/gnu/siglist.c:72:1: internal compiler error: in
>>>>>>> address_matters_p, at symtab.c:1908
>>>>>>> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev,
>>>>>>> GLIBC_2_3_3);
>>>>>>> ^
>>>>>>> 0x66a080 symtab_node::address_matters_p()
>>>>>>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/symtab.c:1908
>>>>>>> 0xe7cbe5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>>>>>>> /scratch/sellcey/repos/bootstrap/src/gcc/gcc/ipa-icf.c:1443
>>>>>>
>>>>>>
>>>>>> Indeed, the ipa-icf should not try to analyze aliases - just prove
>>>>>> ekvialence of
>>>>>> definitions they are attached to. It already does that for functions
>>>>>> (bit by accident;
>>>>>> it gives up when there is no gimple body), but it does not do that for
>>>>>> variables because
>>>>>> it gets into ctor_for_folding. For that reason it sometimes decides to
>>>>>> try to make two
>>>>>> variable aliases alias of each other that is not a good idea, because
>>>>>> of possible creation
>>>>>> of loops.
>>>>>>
>>>>>> I am just discussing with Martin the fix.
>>>>>>
>>>>>> Honza
>>>>>
>>>>>
>>>>> For the record, I have noticed similar errors on ARM and AArch64
>>>>> targets, when building glibc.
>>>>>
>>>>> Christophe.
>>>>>
>>>>
>>>> I confirm ARM and AArch64 failing to build with this patch:
>>>> chmod a-w
>>>> /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
>>>> 0x6b9100 symtab_node::address_matters_p()
>>>> /work/src/gcc/gcc/symtab.c:1908
>>>> 0xedb4e5 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
>>>> /work/src/gcc/gcc/ipa-icf.c:1723
>>>> 0xee03f9 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
>>>> /work/src/gcc/gcc/ipa-icf.c:2955
>>>> 0xee6d31 ipa_icf::sem_item_optimizer::execute()
>>>> /work/src/gcc/gcc/ipa-icf.c:2217
>>>> 0xee8df1 ipa_icf_driver
>>>> /work/src/gcc/gcc/ipa-icf.c:3034
>>>> 0xee8df1 ipa_icf::pass_ipa_icf::execute(function*)
>>>> /work/src/gcc/gcc/ipa-icf.c:3081
>>>
>>>
>>> I commited patch for the alias merging yesterda night, so it should be
>>> fixed
>>> now. If it still fails, please fill in a PR with preprocessed testcase
>>> so I
>>> can reproduce it in a cross.
>>>
>>
>> On my side, I saw builds complete again starting with r221090, I guess
>> that's the commit you are referring to?
>
> Hi,
>
> I built with r221117. I see errors while building following targets:
> aarch64_be-none-linux-gnu, aarch64_be-none-linux-gnu,
> arm-none-linux-gnueabihf, arm-none-linux-gnueabi.
Indeed, it's broken again since r221099.
>
> For aarch64_be-none-linux-gnu I reproduce the error like this:
>
> /work/build-aarch64-none-linux-gnu/install//bin/aarch64-none-linux-gnu-gcc
> /work/src/glibc/sysdeps/gnu/siglist.c -c -std=gnu99 -fgnu89-inline -O2
> -Wall -Winline -Wundef -Wwrite-strings -fmerge-all-constants -frounding-math
> -g -Wstrict-prototypes -fno-toplevel-reorder -fno-section-anchors
> -I/work/src/glibc/include
> -I/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common
> -I/work/build-aarch64-none-linux-gnu/obj/glibc
> -I/work/src/glibc/sysdeps/unix/sysv/linux/aarch64
> -I/work/src/glibc/sysdeps/aarch64/nptl
> -I/work/src/glibc/sysdeps/unix/sysv/linux/generic
> -I/work/src/glibc/sysdeps/unix/sysv/linux/wordsize-64
> -I/work/src/glibc/sysdeps/unix/sysv/linux/include
> -I/work/src/glibc/sysdeps/unix/sysv/linux -I/work/src/glibc/sysdeps/nptl
> -I/work/src/glibc/sysdeps/pthread -I/work/src/glibc/sysdeps/gnu
> -I/work/src/glibc/sysdeps/unix/inet -I/work/src/glibc/sysdeps/unix/sysv
> -I/work/src/glibc/sysdeps/unix -I/work/src/glibc/sysdeps/posix
> -I/work/src/glibc/sysdeps/aarch64/fpu -I/work/src/glibc/sysdeps/aarch64
> -I/work/src/glibc/sysdeps/wordsize-64
> -I/work/src/glibc/sysdeps/ieee754/ldbl-128
> -I/work/src/glibc/sysdeps/ieee754/dbl-64/wordsize-64
> -I/work/src/glibc/sysdeps/ieee754/dbl-64
> -I/work/src/glibc/sysdeps/ieee754/flt-32
> -I/work/src/glibc/sysdeps/aarch64/soft-fp -I/work/src/glibc/sysdeps/ieee754
> -I/work/src/glibc/sysdeps/generic -I/work/src/glibc -I/work/src/glibc/libio
> -I. -nostdinc -isystem
> /work/build-aarch64-none-linux-gnu/install/bin/../lib/gcc/aarch64-none-linux-gnu/5.0.0/include
> -isystem
> /work/build-aarch64-none-linux-gnu/install/bin/../lib/gcc/aarch64-none-linux-gnu/5.0.0/include-fixed
> -isystem
> /work/build-aarch64-none-linux-gnu/install//aarch64-none-linux-gnu/libc/usr/include
> -D_LIBC_REENTRANT -include
> /work/build-aarch64-none-linux-gnu/obj/glibc/libc-modules.h
> -DMODULE_NAME=libc -include /work/src/glibc/include/libc-symbols.h
> -D_IO_MTSAFE_IO -o
> /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o -MD -MP
> -MF /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o.dt
> -MT /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o
> /work/src/glibc/sysdeps/gnu/siglist.c:77:1: internal compiler error: in
> address_matters_p, at symtab.c:1908
> versioned_symbol (libc, __new_sys_sigabbrev, sys_sigabbrev, GLIBC_2_1);
> ^
> 0x6b9140 symtab_node::address_matters_p()
> /work/src/gcc/gcc/symtab.c:1908
> 0xedb685 ipa_icf::sem_variable::merge(ipa_icf::sem_item*)
> /work/src/gcc/gcc/ipa-icf.c:1740
> 0xee05b1 ipa_icf::sem_item_optimizer::merge_classes(unsigned int)
> /work/src/gcc/gcc/ipa-icf.c:2979
> 0xee6f11 ipa_icf::sem_item_optimizer::execute()
> /work/src/gcc/gcc/ipa-icf.c:2236
> 0xee902e ipa_icf_driver
> /work/src/gcc/gcc/ipa-icf.c:3061
> 0xee902e ipa_icf::pass_ipa_icf::execute(function*)
> /work/src/gcc/gcc/ipa-icf.c:3108
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
>
>
> I see a related ticket opened recently:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65287
>
> Kind regards,
> Alex
>
>>
>> Thanks,
>>
>> Christophe.
>>
>>> Honza
>>>>
>>>> Please submit a full bug report,
>>>> with preprocessed source if appropriate.
>>>> Please include the complete backtrace with any bug report.
>>>> See <http://gcc.gnu.org/bugs.html> for instructions.
>>>> make[2]: ***
>>>> [/work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/siglist.o]
>>>> Error 1
>>>> make[2]: *** Waiting for unfinished jobs....
>>>> mv -f
>>>> /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.cT
>>>> /work/build-aarch64-none-linux-gnu/obj/glibc/stdio-common/errlist-compat.c
>>>>
>>>> Regards,
>>>> Alex
>>
>>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-03 15:06 ` Christophe Lyon
@ 2015-03-03 20:01 ` Jan Hubicka
2015-03-04 9:11 ` Christophe Lyon
0 siblings, 1 reply; 26+ messages in thread
From: Jan Hubicka @ 2015-03-03 20:01 UTC (permalink / raw)
To: Christophe Lyon
Cc: Alex Velenko, Jan Hubicka, Steve Ellcey, Jack Howarth,
GCC Patches, Martin Liška, Jakub Jelinek, James Greenhalgh
> > Hi,
> >
> > I built with r221117. I see errors while building following targets:
> > aarch64_be-none-linux-gnu, aarch64_be-none-linux-gnu,
> > arm-none-linux-gnueabihf, arm-none-linux-gnueabi.
>
> Indeed, it's broken again since r221099.
Accidentally I reverted the var->alias check (probably while merging patches from mainline).
It should be fixed again now by:
2015-03-03 Martin Liska <mliska@suse.cz>
PR ipa/65287
* ipa-icf.c (sem_variable::parse): Skip all alias variables.
Sorry for that.
Honza
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-03-03 20:01 ` Jan Hubicka
@ 2015-03-04 9:11 ` Christophe Lyon
0 siblings, 0 replies; 26+ messages in thread
From: Christophe Lyon @ 2015-03-04 9:11 UTC (permalink / raw)
To: Jan Hubicka
Cc: Alex Velenko, Steve Ellcey, Jack Howarth, GCC Patches,
Martin Liška, Jakub Jelinek, James Greenhalgh
On 3 March 2015 at 21:01, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > Hi,
>> >
>> > I built with r221117. I see errors while building following targets:
>> > aarch64_be-none-linux-gnu, aarch64_be-none-linux-gnu,
>> > arm-none-linux-gnueabihf, arm-none-linux-gnueabi.
>>
>> Indeed, it's broken again since r221099.
>
> Accidentally I reverted the var->alias check (probably while merging patches from mainline).
> It should be fixed again now by:
>
> 2015-03-03 Martin Liska <mliska@suse.cz>
>
> PR ipa/65287
> * ipa-icf.c (sem_variable::parse): Skip all alias variables.
>
> Sorry for that.
>
Indeed, it's OK now.
Thanks.
> Honza
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: ipa-icf::merge TLC
2015-02-28 18:15 ` Jan Hubicka
@ 2015-03-04 9:38 ` James Greenhalgh
2015-03-06 16:09 ` [ARM testsuite obvious] Fixup atomic-comp-swap-release-acquire.c to not use ICF James Greenhalgh
0 siblings, 1 reply; 26+ messages in thread
From: James Greenhalgh @ 2015-03-04 9:38 UTC (permalink / raw)
To: Jan Hubicka
Cc: Jack Howarth, GCC Patches, mliska, Jakub Jelinek, Alan Lawrence,
Alex Velenko
[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]
On Sat, Feb 28, 2015 at 06:08:33PM +0000, Jan Hubicka wrote:
> >
> > Hi Honza,
> >
> > This is more a note for other interested AArch64 testers, but this patch
> > breaks some tests on aarch64-none-elf. Looking at the test output, this
> > is a problem with the tests than with your patch. We now eliminate some
> > function bodies which are identical across test functions, causing us to
> > fail some scan-assembler directives.
> >
> > The fix is straightforward, we just need to add -fno-ipa-icf to get things
> > working again. This keeps the spirit of the original tests. I'll propose
> > a patch along those lines on Monday.
> >
> > Thanks,
> > James
> >
> > ---
> >
> > PASS->FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times ldaxr\tw[0-9]+, \\[x[0-9]+\\] 4
> > PASS->FAIL: gcc.target/aarch64/atomic-comp-swap-release-acquire.c scan-assembler-times stlxr\tw[0-9]+, w[0-9]+, \\[x[0-9]+\\] 4
> > PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 2
> > PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times saddl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 2
> > PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl2\tv[0-9]+.2d, v[0-9]+.4s, v[0-9]+.4s 5
> > PASS->FAIL: gcc.target/aarch64/vect_saddl_1.c scan-assembler-times ssubl\tv[0-9]+.2d, v[0-9]+.2s, v[0-9]+.2s 5
> > PASS->FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl2\tv[0-9]+.8h 5
> > PASS->FAIL: gcc.target/aarch64/vect_smlal_1.c scan-assembler-times smlsl\tv[0-9]+.8h 5
>
> Thanks, yes adding -fno-ipa-icf is a proper fix for tests like this.
> What happened was a logic bug in old implementation of merge routine. When
> symbol is externally visible it decided to create a wrapper (to preserve
> potential address compares) and to avoid wrapper cost redirect all direct
> uses.
Hi Honza,
Thanks for the confirmation.
It took me a while longer than expected to get round to it, but
I've committed the attached (revision 221175) as the obvious fix, after
checking that it worked on aarch64-none-elf.
Thanks,
James
---
gcc/testsuite/
2015-03-04 James Greenhalgh <james.greenhalgh@arm.com>
* gcc.target/aarch64/atomic-comp-swap-release-acquire.c: Add
-fno-ipa-icf to dg-options
* gcc.target/aarch64/vect_saddl_1.c: Likewise.
* gcc.target/aarch64/vect_smlal_1.c: Likewise.
[-- Attachment #2: aarch64-testsuite-disable-ipa-icf.patch --]
[-- Type: text/x-diff, Size: 2038 bytes --]
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog (revision 221174)
+++ gcc/testsuite/ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2015-03-04 James Greenhalgh <james.greenhalgh@arm.com>
+
+ * gcc.target/aarch64/atomic-comp-swap-release-acquire.c: Add
+ -fno-ipa-icf to dg-options
+ * gcc.target/aarch64/vect_saddl_1.c: Likewise.
+ * gcc.target/aarch64/vect_smlal_1.c: Likewise.
+
2015-03-04 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/64398
Index: gcc/testsuite/gcc.target/aarch64/atomic-comp-swap-release-acquire.c
===================================================================
--- gcc/testsuite/gcc.target/aarch64/atomic-comp-swap-release-acquire.c (revision 221174)
+++ gcc/testsuite/gcc.target/aarch64/atomic-comp-swap-release-acquire.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -fno-ipa-icf" } */
#include "atomic-comp-swap-release-acquire.x"
Index: gcc/testsuite/gcc.target/aarch64/vect_saddl_1.c
===================================================================
--- gcc/testsuite/gcc.target/aarch64/vect_saddl_1.c (revision 221174)
+++ gcc/testsuite/gcc.target/aarch64/vect_saddl_1.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O3 -fno-inline -save-temps -fno-vect-cost-model" } */
+/* { dg-options "-O3 -fno-inline -save-temps -fno-vect-cost-model -fno-ipa-icf" } */
typedef signed char S8_t;
typedef signed short S16_t;
Index: gcc/testsuite/gcc.target/aarch64/vect_smlal_1.c
===================================================================
--- gcc/testsuite/gcc.target/aarch64/vect_smlal_1.c (revision 221174)
+++ gcc/testsuite/gcc.target/aarch64/vect_smlal_1.c (working copy)
@@ -1,5 +1,5 @@
/* { dg-do run } */
-/* { dg-options "-O3 -fno-inline -save-temps -fno-vect-cost-model" } */
+/* { dg-options "-O3 -fno-inline -save-temps -fno-vect-cost-model -fno-ipa-icf" } */
typedef signed char S8_t;
typedef signed short S16_t;
^ permalink raw reply [flat|nested] 26+ messages in thread
* [ARM testsuite obvious] Fixup atomic-comp-swap-release-acquire.c to not use ICF
2015-03-04 9:38 ` James Greenhalgh
@ 2015-03-06 16:09 ` James Greenhalgh
2015-03-06 16:13 ` James Greenhalgh
0 siblings, 1 reply; 26+ messages in thread
From: James Greenhalgh @ 2015-03-06 16:09 UTC (permalink / raw)
To: gcc-patches
Cc: GCC Patches, Alex Velenko, Ramana Radhakrishnan, Richard Earnshaw
On Wed, Mar 04, 2015 at 09:38:51AM +0000, James Greenhalgh wrote:
> It took me a while longer than expected to get round to it, but
> I've committed the attached (revision 221175) as the obvious fix, after
> checking that it worked on aarch64-none-elf.
>
> Thanks,
> James
>
> ---
> gcc/testsuite/
>
> 2015-03-04 James Greenhalgh <james.greenhalgh@arm.com>
>
> * gcc.target/aarch64/atomic-comp-swap-release-acquire.c: Add
> -fno-ipa-icf to dg-options
> * gcc.target/aarch64/vect_saddl_1.c: Likewise.
> * gcc.target/aarch64/vect_smlal_1.c: Likewise.
Hi,
ARM will want the same fix for the copy of
atomic-comp-swap-release-acquire.c that lives in gcc.target/arm/ .
I've committed the attached as obvious as revision 221243 after testing
on arm-none-eabi with no issues.
Thanks,
James
---
gcc/testsuite/
2015-03-06 James Greenhalgh <james.greenhalgh@arm.com>
* gcc.target/arm/atomic-comp-swap-release-acquire.c: Add
-fno-ipa-icf to dg-options.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [ARM testsuite obvious] Fixup atomic-comp-swap-release-acquire.c to not use ICF
2015-03-06 16:09 ` [ARM testsuite obvious] Fixup atomic-comp-swap-release-acquire.c to not use ICF James Greenhalgh
@ 2015-03-06 16:13 ` James Greenhalgh
0 siblings, 0 replies; 26+ messages in thread
From: James Greenhalgh @ 2015-03-06 16:13 UTC (permalink / raw)
To: gcc-patches; +Cc: Alex Velenko, Ramana Radhakrishnan, Richard Earnshaw
[-- Attachment #1: Type: text/plain, Size: 968 bytes --]
On Fri, Mar 06, 2015 at 04:09:40PM +0000, James Greenhalgh wrote:
> On Wed, Mar 04, 2015 at 09:38:51AM +0000, James Greenhalgh wrote:
> > It took me a while longer than expected to get round to it, but
> > I've committed the attached (revision 221175) as the obvious fix, after
> > checking that it worked on aarch64-none-elf.
> >
> > Thanks,
> > James
> >
> > ---
> > gcc/testsuite/
> >
> > 2015-03-04 James Greenhalgh <james.greenhalgh@arm.com>
> >
> > * gcc.target/aarch64/atomic-comp-swap-release-acquire.c: Add
> > -fno-ipa-icf to dg-options
> > * gcc.target/aarch64/vect_saddl_1.c: Likewise.
> > * gcc.target/aarch64/vect_smlal_1.c: Likewise.
>
> Hi,
>
> ARM will want the same fix for the copy of
> atomic-comp-swap-release-acquire.c that lives in gcc.target/arm/ .
>
> I've committed the attached as obvious as revision 221243 after testing
> on arm-none-eabi with no issues.
ENOPATCH, I'll try again!
r221243 looks like this...
Cheers,
James
[-- Attachment #2: arm-fixup-atomic-comp-swap-release.patch --]
[-- Type: text/x-diff, Size: 1094 bytes --]
Index: gcc/testsuite/gcc.target/arm/atomic-comp-swap-release-acquire.c
===================================================================
--- gcc/testsuite/gcc.target/arm/atomic-comp-swap-release-acquire.c (revision 221242)
+++ gcc/testsuite/gcc.target/arm/atomic-comp-swap-release-acquire.c (revision 221243)
@@ -1,6 +1,6 @@
/* { dg-do compile } */
/* { dg-require-effective-target arm_arch_v8a_ok } */
-/* { dg-options "-O2" } */
+/* { dg-options "-O2 -fno-ipa-icf" } */
/* { dg-add-options arm_arch_v8a } */
#include "../aarch64/atomic-comp-swap-release-acquire.x"
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog (revision 221242)
+++ gcc/testsuite/ChangeLog (revision 221243)
@@ -1,5 +1,10 @@
2015-03-06 James Greenhalgh <james.greenhalgh@arm.com>
+ * gcc.target/arm/atomic-comp-swap-release-acquire.c: Add
+ -fno-ipa-icf to dg-options.
+
+2015-03-06 James Greenhalgh <james.greenhalgh@arm.com>
+
* c-c++-common/torture/aarch64-vect-lane-2.c: XFAIL for LTO
compiles using the linker plugin.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2015-03-06 16:13 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-25 9:03 ipa-icf::merge TLC Jan Hubicka
2015-02-25 13:10 ` Markus Trippelsdorf
2015-02-25 17:31 ` Jan Hubicka
2015-02-25 18:41 ` Martin Liška
2015-02-25 19:02 ` Markus Trippelsdorf
2015-02-26 16:46 ` Jack Howarth
2015-02-27 5:55 ` Jan Hubicka
2015-02-27 13:48 ` H.J. Lu
2015-02-27 18:04 ` Jan Hubicka
2015-02-27 18:39 ` Steve Ellcey
2015-02-27 18:56 ` Steve Ellcey
2015-02-27 20:00 ` Martin Liška
2015-02-27 21:16 ` Jan Hubicka
2015-03-01 16:47 ` Christophe Lyon
2015-03-02 19:01 ` Alex Velenko
2015-03-02 20:21 ` Jan Hubicka
2015-03-02 22:04 ` Christophe Lyon
2015-03-03 12:44 ` Alex Velenko
2015-03-03 15:06 ` Christophe Lyon
2015-03-03 20:01 ` Jan Hubicka
2015-03-04 9:11 ` Christophe Lyon
2015-02-28 16:38 ` James Greenhalgh
2015-02-28 18:15 ` Jan Hubicka
2015-03-04 9:38 ` James Greenhalgh
2015-03-06 16:09 ` [ARM testsuite obvious] Fixup atomic-comp-swap-release-acquire.c to not use ICF James Greenhalgh
2015-03-06 16:13 ` James Greenhalgh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).