[PATCH] Fix for PR ipa/64693

Martin Liška mliska@suse.cz
Fri Feb 20 13:15: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.
>> +
>> +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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Fix-missed-optimization-for-vars-not-marked-by-const.patch
Type: text/x-patch
Size: 2630 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20150220/f1d96f23/attachment.bin>


More information about the Gcc-patches mailing list