diff mbox

Fix for PR ipa/65245

Message ID 54F32435.9000104@suse.cz
State New
Headers show

Commit Message

Martin Liška March 1, 2015, 2:37 p.m. UTC
On 02/28/2015 11:41 PM, Jan Hubicka wrote:
>> Hello.
>>
>> Following patch skips all variable aliases as potential merge candidates. Tested on x86_64-linux.
>>
>> Ready for trunk?
>> Thanks,
>> Martin
> 
>> >From fbde2e98f98a71105d18cf3e91e8032d0c657139 Mon Sep 17 00:00:00 2001
>> From: mliska <mliska@suse.cz>
>> Date: Fri, 27 Feb 2015 22:42:49 +0100
>> Subject: [PATCH 2/4] ICF: Do not consider variable aliases for merge
>>  operation.
>>
>> gcc/ChangeLog:
>>
>> 2015-02-28  Martin Liska  <mliska@suse.cz>
>> 	    Jan Hubicka   <hubicka@ucw.cz>
>>
>> 	PR ipa/65245
>> 	* ipa-icf.c (sem_function::parse): Do not consider aliases.
>> 	(sem_variable::parse):  Likewise.
>> 	(sem_item_optimizer::build_graph): Consider ultimate aliases
>> 	for references.
> 
> OK, can you please add a testcase checking that two functions get merged even
> if each of them referrs different alias of an equvalent symbols?

Hi.

Updated version of patch adds new testcase.

Ready for trunk?
Thanks,
Martin

> 
> Honza
>> ---
>>  gcc/ipa-icf.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
>> index 92133fc..864a5d0 100644
>> --- a/gcc/ipa-icf.c
>> +++ b/gcc/ipa-icf.c
>> @@ -1095,7 +1095,7 @@ sem_function::parse (cgraph_node *node, bitmap_obstack *stack)
>>    tree fndecl = node->decl;
>>    function *func = DECL_STRUCT_FUNCTION (fndecl);
>>  
>> -  /* TODO: add support for thunks and aliases.  */
>> +  /* TODO: add support for thunks.  */
>>  
>>    if (!func || !node->has_gimple_body_p ())
>>      return NULL;
>> @@ -1407,6 +1407,9 @@ sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
>>  {
>>    tree decl = node->decl;
>>  
>> +  if (node->alias)
>> +    return NULL;
>> +
>>    bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
>>    if (!readonly)
>>      return NULL;
>> @@ -2057,7 +2060,8 @@ sem_item_optimizer::build_graph (void)
>>  	  cgraph_edge *e = cnode->callees;
>>  	  while (e)
>>  	    {
>> -	      sem_item **slot = m_symtab_node_map.get (e->callee);
>> +	      sem_item **slot = m_symtab_node_map.get
>> +		(e->callee->ultimate_alias_target ());
>>  	      if (slot)
>>  		item->add_reference (*slot);
>>  
>> @@ -2068,7 +2072,8 @@ sem_item_optimizer::build_graph (void)
>>        ipa_ref *ref = NULL;
>>        for (unsigned i = 0; item->node->iterate_reference (i, ref); i++)
>>  	{
>> -	  sem_item **slot = m_symtab_node_map.get (ref->referred);
>> +	  sem_item **slot = m_symtab_node_map.get
>> +	    (ref->referred->ultimate_alias_target ());
>>  	  if (slot)
>>  	    item->add_reference (*slot);
>>  	}
>> -- 
>> 2.1.2
>>
>

Comments

Jan Hubicka March 1, 2015, 5:57 p.m. UTC | #1
> >From 7cedbe5e3736b9eb7b2fab1b931a7bc9ed269f37 Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Fri, 27 Feb 2015 22:42:49 +0100
> Subject: [PATCH 2/4] ICF: Do not consider variable aliases for merge
>  operation.
> 
> gcc/ChangeLog:
> 
> 2015-02-28  Martin Liska  <mliska@suse.cz>
> 	    Jan Hubicka   <hubicka@ucw.cz>
> 
> 	* ipa-icf-gimple.c (func_checker::compare_function_decl):
> 	Consider ultimate alias as targets.
> 	* ipa-icf.c (sem_function::parse): Do not consider aliases.
> 	(sem_function::compare_cgraph_references): Consider ultimate
> 	alias as targets.
> 	(sem_variable::parse):  Likewise.
> 	(sem_item_optimizer::build_graph): Consider ultimate aliases
> 	for references.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-02-28  Martin Liska  <mliska@suse.cz>
> 	    Jan Hubicka   <hubicka@ucw.cz>
> 
> 	* gcc.dg/ipa/ipa-icf-34.c: New test.
> 	* gcc.target/i386/stackalign/longlong-2.c: Omit ICF.
> ---
>  gcc/ipa-icf-gimple.c                  |  4 ++--
>  gcc/ipa-icf.c                         | 14 +++++++++++---
>  gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c | 28 ++++++++++++++++++++++++++++
>  3 files changed, 41 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c
> 
> diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
> index cbeb795..8008e86 100644
> --- a/gcc/ipa-icf-gimple.c
> +++ b/gcc/ipa-icf-gimple.c
> @@ -543,8 +543,8 @@ func_checker::compare_function_decl (tree t1, tree t2)
>    if (t1 == t2)
>      return true;
>  
> -  symtab_node *n1 = symtab_node::get (t1);
> -  symtab_node *n2 = symtab_node::get (t2);
> +  symtab_node *n1 = symtab_node::get (t1)->ultimate_alias_target ();
> +  symtab_node *n2 = symtab_node::get (t2)->ultimate_alias_target ();

Do we really need to compare them here if they are already compared by equals_wpa?
The logic here is bit more difficult - it depends on alias if it is interposable
or not.  This is obtained by symtab_node::get (t1)->ultimate_alias_target (&avail);

if one of aliases is interposable, you want to use symtab_node::get
(t1)->equal_address_to (symtab_node::get (t2)) if address of given reference
matters and semantically_equivalent_p otherwise.

This is all done already in WPA checking, so I would suggest just assuming that
decl maps if both pass decl_in_symtab_p.
>  
>    if (m_ignored_source_nodes != NULL && m_ignored_target_nodes != NULL)
>      {
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index 92133fc..b95db0d 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -342,6 +342,9 @@ sem_function::compare_cgraph_references (hash_map <symtab_node *, sem_item *>
>      &ignored_nodes,
>      symtab_node *n1, symtab_node *n2)
>  {
> +  n1 = n1->ultimate_alias_target ();
> +  n2 = n2->ultimate_alias_target ();
> +
>    if (n1 == n2 || (ignored_nodes.get (n1) && ignored_nodes.get (n2)))
>      return true;

Here I sent you correct change privately yesterday - you want to use equal_address_to.
I would still suggest just putting all those into sensitive reference lists.

Rest of the patch looks OK.
Honza
diff mbox

Patch

From 7cedbe5e3736b9eb7b2fab1b931a7bc9ed269f37 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Fri, 27 Feb 2015 22:42:49 +0100
Subject: [PATCH 2/4] ICF: Do not consider variable aliases for merge
 operation.

gcc/ChangeLog:

2015-02-28  Martin Liska  <mliska@suse.cz>
	    Jan Hubicka   <hubicka@ucw.cz>

	* ipa-icf-gimple.c (func_checker::compare_function_decl):
	Consider ultimate alias as targets.
	* ipa-icf.c (sem_function::parse): Do not consider aliases.
	(sem_function::compare_cgraph_references): Consider ultimate
	alias as targets.
	(sem_variable::parse):  Likewise.
	(sem_item_optimizer::build_graph): Consider ultimate aliases
	for references.

gcc/testsuite/ChangeLog:

2015-02-28  Martin Liska  <mliska@suse.cz>
	    Jan Hubicka   <hubicka@ucw.cz>

	* gcc.dg/ipa/ipa-icf-34.c: New test.
	* gcc.target/i386/stackalign/longlong-2.c: Omit ICF.
---
 gcc/ipa-icf-gimple.c                  |  4 ++--
 gcc/ipa-icf.c                         | 14 +++++++++++---
 gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c | 28 ++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index cbeb795..8008e86 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -543,8 +543,8 @@  func_checker::compare_function_decl (tree t1, tree t2)
   if (t1 == t2)
     return true;
 
-  symtab_node *n1 = symtab_node::get (t1);
-  symtab_node *n2 = symtab_node::get (t2);
+  symtab_node *n1 = symtab_node::get (t1)->ultimate_alias_target ();
+  symtab_node *n2 = symtab_node::get (t2)->ultimate_alias_target ();
 
   if (m_ignored_source_nodes != NULL && m_ignored_target_nodes != NULL)
     {
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 92133fc..b95db0d 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -342,6 +342,9 @@  sem_function::compare_cgraph_references (hash_map <symtab_node *, sem_item *>
     &ignored_nodes,
     symtab_node *n1, symtab_node *n2)
 {
+  n1 = n1->ultimate_alias_target ();
+  n2 = n2->ultimate_alias_target ();
+
   if (n1 == n2 || (ignored_nodes.get (n1) && ignored_nodes.get (n2)))
     return true;
 
@@ -1095,7 +1098,7 @@  sem_function::parse (cgraph_node *node, bitmap_obstack *stack)
   tree fndecl = node->decl;
   function *func = DECL_STRUCT_FUNCTION (fndecl);
 
-  /* TODO: add support for thunks and aliases.  */
+  /* TODO: add support for thunks.  */
 
   if (!func || !node->has_gimple_body_p ())
     return NULL;
@@ -1407,6 +1410,9 @@  sem_variable::parse (varpool_node *node, bitmap_obstack *stack)
 {
   tree decl = node->decl;
 
+  if (node->alias)
+    return NULL;
+
   bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl);
   if (!readonly)
     return NULL;
@@ -2057,7 +2063,8 @@  sem_item_optimizer::build_graph (void)
 	  cgraph_edge *e = cnode->callees;
 	  while (e)
 	    {
-	      sem_item **slot = m_symtab_node_map.get (e->callee);
+	      sem_item **slot = m_symtab_node_map.get
+		(e->callee->ultimate_alias_target ());
 	      if (slot)
 		item->add_reference (*slot);
 
@@ -2068,7 +2075,8 @@  sem_item_optimizer::build_graph (void)
       ipa_ref *ref = NULL;
       for (unsigned i = 0; item->node->iterate_reference (i, ref); i++)
 	{
-	  sem_item **slot = m_symtab_node_map.get (ref->referred);
+	  sem_item **slot = m_symtab_node_map.get
+	    (ref->referred->ultimate_alias_target ());
 	  if (slot)
 	    item->add_reference (*slot);
 	}
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..698044a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-34.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O0 -fipa-icf -fdump-ipa-icf"  } */
+
+static int do_work(void)
+{
+  return 0;
+}
+
+static int foo() __attribute__((alias("do_work")));
+static int bar() __attribute__((alias("do_work")));
+
+static int a()
+{
+  return foo();
+}
+
+static int b()
+{
+  return bar();
+}
+
+int main()
+{
+  return a() + b();
+}
+
+/* { dg-final { scan-ipa-dump "Equal symbols: 1" "icf"  } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */
diff --git a/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c b/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c
index 6ea83f9..d52b9d1 100644
--- a/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c
+++ b/gcc/testsuite/gcc.target/i386/stackalign/longlong-2.c
@@ -1,6 +1,6 @@ 
 /* { dg-do compile  { target { ! *-*-darwin* } } } */
 /* { dg-require-effective-target ia32 } */
-/* { dg-options "-O2 -mpreferred-stack-boundary=2" } */
+/* { dg-options "-O2 -mpreferred-stack-boundary=2 -fno-ipa-icf" } */
 /* { dg-final { scan-assembler-times "and\[lq\]?\[^\\n\]*-8,\[^\\n\]*sp" 2 } } */ 
 /* { dg-final { scan-assembler-times "and\[lq\]?\[^\\n\]*-16,\[^\\n\]*sp" 2 } } */ 

-- 
2.1.2