On 02/12/2015 05:57 PM, Jan Hubicka wrote:
From: mliska <mli...@suse.cz>V
Date: Fri, 23 Jan 2015 14:58:36 +0100
Subject: [PATCH] Fix PR ipa/64693.
gcc/ChangeLog:
2015-02-12 Martin Liska <mli...@suse.cz>
PR ipa/64693
* ipa-icf.c (sem_item_optimizer::execute): Call
identify_address_sensitive_classes.
(sem_item_optimizer::identify_address_sensitive_classes): New function.
(sem_item_optimizer::merge_classes): Handle cases where we merge a class
with a sensitive reference.
(congruence_class::dump): Add support for a new sensitive_reference
flag.
* ipa-icf.h: Add new function.
+
+/* Identify congruence classes which have an address taken. These
+ classes can be potentially been merged just as thunks. */
Hmm, I would expect someting like this
1) break out code in sem_function::merge which decides if function A and B
can be identified or if thunk is needed
2) when comparing references to verify congruence, actually check if merging
would result in thunk and subdivide.
In the following:
void f1()
{
}
void f2()
{
}
void *a=&f1;
void *b=&f1;
void *c=&f2;
void *d=&f2;
The code bellow seems to disable all merging, while we ought to be able to mere
A,B
and C,D into two congruences.
Similarly we ought to be able to merge in a case F2 is a virtual
function/ctor/dtor
where address can not be compared for equality.
+
+void
+sem_item_optimizer::identify_address_sensitive_classes (void)
+{
+ for (hash_table<congruence_class_group_hash>::iterator it = m_classes.begin
();
+ it != m_classes.end (); ++it)
+ for (unsigned i = 0; i < (*it)->classes.length (); i++)
+ {
+ congruence_class *cls = (*it)->classes[i];
+
+ if (cls->members.length () == 1)
+ continue;
+
+ auto_vec <sem_item *> references;
+ sem_item *source_node = cls->members[0];
+ ipa_ref *r;
+
+ for (unsigned j = 0; j < source_node->node->num_references (); j++)
+ {
+ symtab_node *ref = source_node->node->iterate_reference (j,
r)->referred;
+ sem_item **symbol = m_symtab_node_map.get (ref);
+ if (symbol)
+ references.safe_push (*symbol);
+ }
+
+ for (unsigned j = 1; j < cls->members.length (); j++)
+ {
+ source_node = cls->members[j];
+
+ unsigned l = 0;
+ for (unsigned k = 0; k < source_node->node->num_references (); k++)
+ {
+ symtab_node *ref = source_node->node->iterate_reference (k,
r)->referred;
+ sem_item **symbol = m_symtab_node_map.get (ref);
+ if (symbol)
+ {
+ if (l >= references.length () || references[l] != *symbol)
+ {
+ cls->sensitive_reference = true;
+ break;
+ }
+
+ l++;
+ }
+ }
+
+ if (cls->sensitive_reference)
+ break;
+ }
+ }
+}
+
/* Debug function prints all informations about congruence classes. */
void
@@ -2374,6 +2430,15 @@ sem_item_optimizer::merge_classes (unsigned int
prev_class_count)
continue;
}
+ if (c->sensitive_reference)
+ {
+ if (dump_file)
+ fprintf (dump_file, "A function from the congruence class "
+ "uses a sensitive reference.\n");
+
+ continue;
+ }
+
if (dump_file && (dump_flags & TDF_DETAILS))
{
source->dump_to_file (dump_file);
@@ -2390,8 +2455,10 @@ sem_item_optimizer::merge_classes (unsigned int
prev_class_count)
void
congruence_class::dump (FILE *file, unsigned int indent) const
{
- FPRINTF_SPACES (file, indent, "class with id: %u, hash: %u, items: %u\n",
- id, members[0]->get_hash (), members.length ());
+ FPRINTF_SPACES (file, indent,
+ "class with id: %u, hash: %u, items: %u %s\n",
+ id, members[0]->get_hash (), members.length (),
+ sensitive_reference ? "sensitive_reference" : "");
FPUTS_SPACES (file, indent + 2, "");
for (unsigned i = 0; i < members.length (); i++)
diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h
index adbedd6..db0bc54 100644
--- a/gcc/ipa-icf.h
+++ b/gcc/ipa-icf.h
@@ -29,7 +29,8 @@ class congruence_class
{
public:
/* Congruence class constructor for a new class with _ID. */
- congruence_class (unsigned int _id): in_worklist (false), id(_id)
+ congruence_class (unsigned int _id): in_worklist (false), id(_id),
+ sensitive_reference (false)
{
}
@@ -54,6 +55,9 @@ public:
/* Global unique class identifier. */
unsigned int id;
+
+ /* A function from the class contains a reference to another class. */
+ bool sensitive_reference;
};
/* Semantic item type enum. */
@@ -476,6 +480,10 @@ private:
/* Iterative congruence reduction function. */
void process_cong_reduction (void);
+ /* Identify congruence classes which have an address taken. These
+ classes can be potentially been merged just as thunks. */
+ void identify_address_sensitive_classes (void);
+
/* After reduction is done, we can declare all items in a group
to be equal. PREV_CLASS_COUNT is start number of classes
before reduction. */
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
index 0c5a5a6..f25a251 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-26.c
@@ -38,7 +38,7 @@ 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 "A function from the congruence class uses a sensitive
reference." "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..66bcac5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c
@@ -0,0 +1,44 @@
+/* { 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: 2" "icf" } } */
+/* { dg-final { scan-ipa-dump "A function from the congruence class uses a sensitive
reference." "icf" } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
--
2.1.2
Hi.
This is second part which introduces better variable handling. Since readonly
variable flag
identification can identify new candidates, ICF should filter out non-readonly
variables in
execute phase.
Ready for trunk?
Thanks,
Martin
>From a18a4840d14b1c0d35a9e4387daae29f5e8c906c Mon Sep 17 00:00:00 2001
From: mliska <mli...@suse.cz>
Date: Fri, 20 Feb 2015 11:15:37 +0100
Subject: [PATCH 2/2] Fix missed optimization for vars not marked by const.
gcc/testsuite/ChangeLog:
2015-02-20 Martin Liska <mli...@suse.cz>
* gcc.dg/ipa/ipa-icf-35.c: New test.
gcc/ChangeLog:
2015-02-20 Martin Liska <mli...@suse.cz>
* ipa-icf.c (sem_variable::parse): Ignore readonly flag that
should be evaluated in driver.
(sem_item_optimizer::filter_removed_items): Filter out
non-readonly variables.
---
gcc/ipa-icf.c | 13 ++++++++-----
gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 859b9d1..5973b2f 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -1228,10 +1228,6 @@ sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
{
tree decl = node->decl;
- bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
- if (!readonly)
- return NULL;
-
bool can_handle = DECL_VIRTUAL_P (decl)
|| flag_merge_constants >= 2
|| (!TREE_ADDRESSABLE (decl) && !node->externally_visible);
@@ -1697,7 +1693,14 @@ sem_item_optimizer::filter_removed_items (void)
if (!flag_ipa_icf_variables)
remove_item (item);
else
- filtered.safe_push (item);
+ {
+ /* Filter out non-readonly variables. */
+ tree decl = item->decl;
+ if (TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl))
+ filtered.safe_push (item);
+ else
+ remove_item (item);
+ }
}
}
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c
new file mode 100644
index 0000000..95d247e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-35.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf" } */
+
+#include <stdlib.h>
+#include <assert.h>
+
+void f1()
+{
+}
+
+void f2()
+{
+}
+
+static void (*a)(void)=&f1;
+static void (*b)(void)=&f1;
+static void (*c)(void)=&f2;
+static void (*d)(void)=&f2;
+
+int main()
+{
+ a();
+ b();
+ c();
+ d();
+
+ return 0;
+}
+
+/* { dg-final { scan-ipa-dump "Equal symbols: 3" "icf" } } */
+/* { dg-final { scan-ipa-dump "Semantic equality hit:f2->f1" "icf" } } */
+/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */
+/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
--
2.1.2
