[PATCH] Fix for PR ipa/64693
Martin Liška
mliska@suse.cz
Fri Feb 13 13:37:00 GMT 2015
On 02/12/2015 05:57 PM, Jan Hubicka wrote:
>> From: mliska <mliska@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 <mliska@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.
Hello.
The code below works fine with this example:
Semantic equality hit:d->c
Semantic equality hit:b->a
Semantic equality hit:f2->f1
It just provides guard for following very special situation (cc1 LTO contains just 2 classes with this example),
which is in gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c:
Where we have functions in these classes:
class 1:
foo,bar
class 2:
callback1, callback2
Where foo references callback1 and bar points to callback2. As callback{1,2} are in the same congruence class
we can process merge operation of foo and bar. Unfortunately, address of callback{1,2} is taken and in this case
we cannot merge foo and bar.
As you said, to make things really correct, one should split class 1 according to these references.
Question is if we want to do it for GCC 5.0 or prepare if for next release?
I'm going to measure number of occurrences in Firefox and can make a decision.
I like the idea of separating function in a class to candidates that need thunk creation and the rest. That will
work seamlessly with your idea to include all functions that are just 'wrappers' for a function.
Thanks for ideas,
Martin
>> +
>> +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
>>
>
More information about the Gcc-patches
mailing list