On 02/25/2015 07:46 PM, Jan Hubicka wrote:
>From dd240028726cb7fdc777acd0b6d14c4f89aed714 Mon Sep 17 00:00:00 2001
From: mliska <mli...@suse.cz>
Date: Thu, 19 Feb 2015 16:08:09 +0100
Subject: [PATCH 1/3] Fix PR ipa/64693
2015-02-25 Martin Liska <mli...@suse.cz>
Jan Hubicka <hubi...@ucw.cz>
* gcc.dg/ipa/ipa-icf-26.c: Update test.
* gcc.dg/ipa/ipa-icf-33.c: Remove redundant line.
* gcc.dg/ipa/ipa-icf-34.c: New test.
gcc/ChangeLog:
2015-02-25 Martin Liska <mli...@suse.cz>
Jan Hubicka <hubi...@ucw.cz>
* cgraph.h (address_matters_p): New function.
* ipa-icf.c (symbol_compare_collection::symbol_compare_collection): New.
(sem_item_optimizer::subdivide_classes_by_sensitive_refs): New function.
(sem_item_optimizer::process_cong_reduction): Include division by
sensitive references.
* ipa-icf.h (struct symbol_compare_hashmap_traits): New class.
* ipa-visibility.c (symtab_node::address_taken_from_non_vtable_p):
Removed.
* symtab.c (address_matters_1): New function.
(symtab_node::address_matters_p): Moved from ipa-visibility.c.
+ if (is_a <varpool_node *> (node) && DECL_VIRTUAL_P (node->decl))
+ return;
+
+ for (unsigned i = 0; i < node->num_references (); i++)
+ {
+ ref = node->iterate_reference (i, ref);
+ if (ref->use == IPA_REF_ADDR && ref->referred->address_matters_p ()
+ && !DECL_VIRTUAL_P (ref->referring->decl))
!address_matters_p should be implied by !DECL_VIRTUAL_P (ref->referring->decl).
+ m_references.safe_push (ref->referred);
+
+ if (ref->referred->get_availability () <= AVAIL_INTERPOSABLE)
+ m_interposables.safe_push (ref->referred);
Push into m_references if ref->use is IPA_REF_ADDR. We care about address and
not value then.
+ }
+
+ if (is_a <cgraph_node *> (node))
+ {
+ cgraph_node *cnode = dyn_cast <cgraph_node *> (node);
+
+ for (cgraph_edge *e = cnode->callees; e; e = e->next_callee)
+ if (e->callee->get_availability () <= AVAIL_INTERPOSABLE)
+ m_interposables.safe_push (e->callee);
+ }
@@ -140,6 +204,15 @@ public:
contains_polymorphic_type_p comparison. */
static bool get_base_types (tree *t1, tree *t2);
+ /* Return true if given DECL is neither virtual nor cdtor. */
+ static bool is_nonvirtual_or_cdtor (tree decl)
You should be able to drop this one.
+/* 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;
+}
Aha, I meant adding address_matters_p predicate into ipa-ref that will test
whether given refernece may lead
to address being used for comparsion.
Something like
/* 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;
}
Honza
Hi.
There's fixed version of the patch, patch can LTO-boostrap and no regression
has been seen.
Martin
>From d92ae52411e37c472eea04bc27ebd70db92745bc Mon Sep 17 00:00:00 2001
From: mliska <mli...@suse.cz>
Date: Thu, 19 Feb 2015 16:08:09 +0100
Subject: [PATCH 1/4] Fix PR ipa/64693
gcc/ChangeLog:
2015-02-25 Martin Liska <mli...@suse.cz>
Jan Hubicka <hubi...@ucw.cz>
PR ipa/64693
* ipa-icf.c (symbol_compare_collection::symbol_compare_collection): New.
(sem_item_optimizer::subdivide_classes_by_sensitive_refs): New function.
(sem_item_optimizer::process_cong_reduction): Include division by
sensitive references.
* ipa-icf.h (struct symbol_compare_hashmap_traits): New class.
* ipa-ref.c (ipa_ref::address_matters_p): New function.
* ipa-ref.h (ipa_ref::address_matters_p): Likewise.
gcc/testsuite/ChangeLog:
2015-02-25 Martin Liska <mli...@suse.cz>
Jan Hubicka <hubi...@ucw.cz>
* g++.dg/ipa/pr64146.C: Update expected results.
* gcc.dg/ipa/ipa-icf-26.c: Update test.
* gcc.dg/ipa/ipa-icf-33.c: Remove redundant line.
* gcc.dg/ipa/ipa-icf-34.c: New test.
---
gcc/ipa-icf.c | 125 ++++++++++++++++++++++++++++++++++
gcc/ipa-icf.h | 71 +++++++++++++++++++
gcc/ipa-ref.c | 20 ++++++
gcc/ipa-ref.h | 3 +
gcc/testsuite/g++.dg/ipa/pr64146.C | 3 +-
gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c | 3 +-
gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c | 1 -
gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c | 43 ++++++++++++
8 files changed, 264 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index e1af8bf..f34407c 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -126,6 +126,44 @@ along with GCC; see the file COPYING3. If not see
using namespace ipa_icf_gimple;
namespace ipa_icf {
+
+/* Constructor. */
+
+symbol_compare_collection::symbol_compare_collection (symtab_node *node)
+{
+ m_references.create (0);
+ m_interposables.create (0);
+
+ ipa_ref *ref;
+
+ if (is_a <varpool_node *> (node) && DECL_VIRTUAL_P (node->decl))
+ return;
+
+ for (unsigned i = 0; i < node->num_references (); i++)
+ {
+ ref = node->iterate_reference (i, ref);
+ if (ref->address_matters_p ())
+ m_references.safe_push (ref->referred);
+
+ if (ref->referred->get_availability () <= AVAIL_INTERPOSABLE)
+ {
+ if (ref->use == IPA_REF_ADDR)
+ m_references.safe_push (ref->referred);
+ else
+ m_interposables.safe_push (ref->referred);
+ }
+ }
+
+ if (is_a <cgraph_node *> (node))
+ {
+ cgraph_node *cnode = dyn_cast <cgraph_node *> (node);
+
+ for (cgraph_edge *e = cnode->callees; e; e = e->next_callee)
+ if (e->callee->get_availability () <= AVAIL_INTERPOSABLE)
+ m_interposables.safe_push (e->callee);
+ }
+}
+
/* Constructor for key value pair, where _ITEM is key and _INDEX is a target. */
sem_usage_pair::sem_usage_pair (sem_item *_item, unsigned int _index):
@@ -1967,6 +2005,84 @@ sem_item_optimizer::subdivide_classes_by_equality (bool in_wpa)
verify_classes ();
}
+/* Subdivide classes by address references that members of the class
+ reference. Example can be a pair of functions that have an address
+ taken from a function. If these addresses are different the class
+ is split. */
+
+unsigned
+sem_item_optimizer::subdivide_classes_by_sensitive_refs ()
+{
+ unsigned newly_created_classes = 0;
+
+ for (hash_table <congruence_class_group_hash>::iterator it = m_classes.begin ();
+ it != m_classes.end (); ++it)
+ {
+ unsigned int class_count = (*it)->classes.length ();
+ auto_vec<congruence_class *> new_classes;
+
+ for (unsigned i = 0; i < class_count; i++)
+ {
+ congruence_class *c = (*it)->classes [i];
+
+ if (c->members.length() > 1)
+ {
+ hash_map <symbol_compare_collection *, vec <sem_item *>,
+ symbol_compare_hashmap_traits> split_map;
+
+ for (unsigned j = 0; j < c->members.length (); j++)
+ {
+ sem_item *source_node = c->members[j];
+
+ symbol_compare_collection *collection = new symbol_compare_collection (source_node->node);
+
+ vec <sem_item *> *slot = &split_map.get_or_insert (collection);
+ gcc_checking_assert (slot);
+
+ slot->safe_push (source_node);
+ }
+
+ /* If the map contains more than one key, we have to split the map
+ appropriately. */
+ if (split_map.elements () != 1)
+ {
+ bool first_class = true;
+
+ hash_map <symbol_compare_collection *, vec <sem_item *>,
+ symbol_compare_hashmap_traits>::iterator it2 = split_map.begin ();
+ for (; it2 != split_map.end (); ++it2)
+ {
+ congruence_class *new_cls;
+ new_cls = new congruence_class (class_id++);
+
+ for (unsigned k = 0; k < (*it2).second.length (); k++)
+ add_item_to_class (new_cls, (*it2).second[k]);
+
+ worklist_push (new_cls);
+ newly_created_classes++;
+
+ if (first_class)
+ {
+ (*it)->classes[i] = new_cls;
+ first_class = false;
+ }
+ else
+ {
+ new_classes.safe_push (new_cls);
+ m_classes_count++;
+ }
+ }
+ }
+ }
+ }
+
+ for (unsigned i = 0; i < new_classes.length (); i++)
+ (*it)->classes.safe_push (new_classes[i]);
+ }
+
+ return newly_created_classes;
+}
+
/* Verify congruence classes if checking is enabled. */
void
@@ -2256,8 +2372,17 @@ sem_item_optimizer::process_cong_reduction (void)
fprintf (dump_file, "Congruence class reduction\n");
congruence_class *cls;
+
+ /* Process complete congruence reduction. */
while ((cls = worklist_pop ()) != NULL)
do_congruence_step (cls);
+
+ /* Subdivide newly created classes according to references. */
+ unsigned new_classes = subdivide_classes_by_sensitive_refs ();
+
+ if (dump_file)
+ fprintf (dump_file, "Address reference subdivision created: %u "
+ "new classes.\n", new_classes);
}
/* Debug function prints all informations about congruence classes. */
diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h
index a55699b..9e76239 100644
--- a/gcc/ipa-icf.h
+++ b/gcc/ipa-icf.h
@@ -63,6 +63,70 @@ enum sem_item_type
VAR
};
+/* Class is container for address references for a symtab_node. */
+
+class symbol_compare_collection
+{
+public:
+ /* Constructor. */
+ symbol_compare_collection (symtab_node *node);
+
+ /* Destructor. */
+ ~symbol_compare_collection ()
+ {
+ m_references.release ();
+ m_interposables.release ();
+ }
+
+ /* Vector of address references. */
+ vec<symtab_node *> m_references;
+
+ /* Vector of interposable references. */
+ vec<symtab_node *> m_interposables;
+};
+
+/* Hash traits for symbol_compare_collection map. */
+
+struct symbol_compare_hashmap_traits: default_hashmap_traits
+{
+ static hashval_t
+ hash (const symbol_compare_collection *v)
+ {
+ inchash::hash hstate;
+ hstate.add_int (v->m_references.length ());
+
+ for (unsigned i = 0; i < v->m_references.length (); i++)
+ hstate.add_ptr (v->m_references[i]->ultimate_alias_target ());
+
+ hstate.add_int (v->m_interposables.length ());
+
+ for (unsigned i = 0; i < v->m_interposables.length (); i++)
+ hstate.add_ptr (v->m_interposables[i]->ultimate_alias_target ());
+
+ return hstate.end ();
+ }
+
+ static bool
+ equal_keys (const symbol_compare_collection *a,
+ const symbol_compare_collection *b)
+ {
+ if (a->m_references.length () != b->m_references.length ())
+ return false;
+
+ for (unsigned i = 0; i < a->m_references.length (); i++)
+ if (a->m_references[i]->equal_address_to (b->m_references[i]) != 1)
+ return false;
+
+ for (unsigned i = 0; i < a->m_interposables.length (); i++)
+ if (!a->m_interposables[i]->semantically_equivalent_p
+ (b->m_interposables[i]))
+ return false;
+
+ return true;
+ }
+};
+
+
/* Semantic item usage pair. */
class sem_usage_pair
{
@@ -467,6 +531,13 @@ private:
classes. If IN_WPA, fast equality function is invoked. */
void subdivide_classes_by_equality (bool in_wpa = false);
+ /* Subdivide classes by address and interposable references
+ that members of the class reference.
+ Example can be a pair of functions that have an address
+ taken from a function. If these addresses are different the class
+ is split. */
+ unsigned subdivide_classes_by_sensitive_refs();
+
/* Debug function prints all informations about congruence classes. */
void dump_cong_classes (void);
diff --git a/gcc/ipa-ref.c b/gcc/ipa-ref.c
index 91c2f89..f9af352 100644
--- a/gcc/ipa-ref.c
+++ b/gcc/ipa-ref.c
@@ -124,3 +124,23 @@ 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;
+}
diff --git a/gcc/ipa-ref.h b/gcc/ipa-ref.h
index aea7f4c..38df8c9 100644
--- a/gcc/ipa-ref.h
+++ b/gcc/ipa-ref.h
@@ -47,6 +47,9 @@ public:
function. */
bool cannot_lead_to_return ();
+ /* Return true if refernece may be used in address compare. */
+ bool address_matters_p ();
+
/* Return reference list this reference is in. */
struct ipa_ref_list * referring_ref_list (void);
diff --git a/gcc/testsuite/g++.dg/ipa/pr64146.C b/gcc/testsuite/g++.dg/ipa/pr64146.C
index bdadfbe..c9a2590 100644
--- a/gcc/testsuite/g++.dg/ipa/pr64146.C
+++ b/gcc/testsuite/g++.dg/ipa/pr64146.C
@@ -34,6 +34,5 @@ int main (int argc, char **argv)
return 0;
}
-/* { dg-final { scan-ipa-dump-times "Declaration does not bind to currect definition." 2 "icf" } } */
-/* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf" } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
index 0c5a5a6..f48c040 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
@@ -38,7 +38,6 @@ int main()
return 0;
}
-/* { dg-final { scan-ipa-dump "Semantic equality hit:bar->foo" "icf" } } */
/* { dg-final { scan-ipa-dump "Semantic equality hit:remove->destroy" "icf" } } */
-/* { dg-final { scan-ipa-dump "Equal symbols: 2" "icf" } } */
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c
index d7e814d..7b6a8ae 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-33.c
@@ -23,5 +23,4 @@ int main()
}
/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
-/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c
new file mode 100644
index 0000000..7772e49
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O0 -fipa-icf -fdump-ipa-icf-details" } */
+
+#include <stdlib.h>
+#include <assert.h>
+
+int callback1(int a)
+{
+ return a * a;
+}
+
+int callback2(int a)
+{
+ return a * a;
+}
+
+static int test(int (*callback) (int))
+{
+ if (callback == callback1)
+ return 1;
+
+ return 0;
+}
+
+int foo()
+{
+ return test(&callback1);
+}
+
+int bar()
+{
+ return test(&callback2);
+}
+
+int main()
+{
+ assert (foo() != bar());
+
+ return 0;
+}
+
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf" } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
--
2.1.2
