[PATCH] Fix for PR ipa/64146

[PATCH] Fix for PR ipa/64146

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

Hello.

In PR64146, for position independent code IPA ICF should be more careful about thunk creation.
Patch can bootstrap on x86_64-linux-pc and no new regression was seen.

Ready for thunk?
Thank you,
Martin

[-- Attachment #2: 0001-IPA-ICF-Fix-for-PR-ipa-64146.patch --]
[-- Type: text/x-patch, Size: 2149 bytes --]

From e57dbf95cf27c2d5da2322ee75dca6361ab59c8a Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 10 Dec 2014 14:46:28 +0100
Subject: [PATCH] IPA ICF: Fix for PR ipa/64146

gcc/ChangeLog:

2014-12-10  Martin Liska  <mliska@suse.cz>

	PR ipa/64146
	* ipa-icf.c (sem_function::merge): Check for
	decl_binds_to_current_def_p is newly added to merge operation.

gcc/testsuite/ChangeLog:

2014-12-10  Martin Liska  <mliska@suse.cz>

	* g++.dg/ipa/pr64146.C: New test.
---
 gcc/ipa-icf.c                      |  8 ++++++++
 gcc/testsuite/g++.dg/ipa/pr64146.C | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr64146.C

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index b193200..91878b2 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -101,6 +101,7 @@ along with GCC; see the file COPYING3.  If not see
 #include <list>
 #include "ipa-icf-gimple.h"
 #include "ipa-icf.h"
+#include "varasm.h"
 
 using namespace ipa_icf_gimple;
 
@@ -624,6 +625,13 @@ sem_function::merge (sem_item *alias_item)
 	return false;
       }
 
+  if (!decl_binds_to_current_def_p (alias->decl))
+    {
+      if (dump_file)
+	fprintf (dump_file, "Declaration does not bind to currect definition.\n\n");
+      return false;
+    }
+
   if (redirect_callers)
     {
       /* If alias is non-overwritable then
diff --git a/gcc/testsuite/g++.dg/ipa/pr64146.C b/gcc/testsuite/g++.dg/ipa/pr64146.C
new file mode 100644
index 0000000..90c5093
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr64146.C
@@ -0,0 +1,37 @@
+/* { dg-do compile } */
+/* { dg-options "-fpic -fdump-ipa-icf-details -fipa-icf"  } */
+
+extern "C" const char*
+foo()
+{
+  return "original";
+}
+
+const char*
+test_foo()
+{
+  return foo();
+}
+
+extern "C" const char*
+bar()
+{
+  return "original";
+}
+
+const char*
+test_bar()
+{
+  return bar();
+}
+
+int main (int argc, char **argv)
+{
+  test_foo ();
+  test_bar ();
+
+  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"  } } */
-- 
2.1.2

Re: [PATCH] Fix for PR ipa/64146

[Richard Biener]

On Thu, Dec 11, 2014 at 2:49 PM, Martin Liška <mliska@suse.cz> wrote:
> Hello.
>
> In PR64146, for position independent code IPA ICF should be more careful
> about thunk creation.
> Patch can bootstrap on x86_64-linux-pc and no new regression was seen.
>
> Ready for thunk?

Hmm, does that merge the functions but
keep a call to the original alias which can be overridden at runtime?

If so, ok.

Thanks,
Richard.

> Thank you,
> Martin

Re: [PATCH] Fix for PR ipa/64146

[Martin Liška]

On 12/11/2014 03:03 PM, Richard Biener wrote:
> On Thu, Dec 11, 2014 at 2:49 PM, Martin Liška <mliska@suse.cz> wrote:
>> Hello.
>>
>> In PR64146, for position independent code IPA ICF should be more careful
>> about thunk creation.
>> Patch can bootstrap on x86_64-linux-pc and no new regression was seen.
>>
>> Ready for thunk?
>
> Hmm, does that merge the functions but
> keep a call to the original alias which can be overridden at runtime?
>
> If so, ok.

Hello.

No, in this case there's no merge operation processed.
Function are going to remain as they are.

I hope such behavior is the right one.

Martin

>
> Thanks,
> Richard.
>
>> Thank you,
>> Martin

Re: [PATCH] Fix for PR ipa/64146

[Richard Biener]

On Tue, Dec 16, 2014 at 5:05 PM, Martin Liška <mliska@suse.cz> wrote:
> On 12/11/2014 03:03 PM, Richard Biener wrote:
>>
>> On Thu, Dec 11, 2014 at 2:49 PM, Martin Liška <mliska@suse.cz> wrote:
>>>
>>> Hello.
>>>
>>> In PR64146, for position independent code IPA ICF should be more careful
>>> about thunk creation.
>>> Patch can bootstrap on x86_64-linux-pc and no new regression was seen.
>>>
>>> Ready for thunk?
>>
>>
>> Hmm, does that merge the functions but
>> keep a call to the original alias which can be overridden at runtime?
>>
>> If so, ok.
>
>
> Hello.
>
> No, in this case there's no merge operation processed.
> Function are going to remain as they are.
>
> I hope such behavior is the right one.

It's certainly conservative.

Ok.

Thanks,
Richard.

> Martin
>
>
>>
>> Thanks,
>> Richard.
>>
>>> Thank you,
>>> Martin
>
>

Re: [PATCH] Fix for PR ipa/64146

[Martin Liška]

On 12/12/2014 06:21 PM, Dominique Dhumieres wrote:
> Martin,
>
> Your test g++.dg/ipa/pr64146.C fails on darwin:
> grep bind pr64146.C.051i.icf
> returns nothing, so the first scan fails, while the second one succeeds.
>
> Dominique
>

Hello.

You are right, I forgot to decorate test case with:
+/* { dg-require-alias "" } */

Martin

Re: [PATCH] Fix for PR ipa/64146

[Dominique Dhumieres]

Martin,

Your test g++.dg/ipa/pr64146.C fails on darwin:
grep bind pr64146.C.051i.icf
returns nothing, so the first scan fails, while the second one succeeds.

Dominique