Re: [PATCH] Fix PR ipa/65557

Re: [PATCH] Fix PR ipa/65557

[Uros Bizjak]

Hello!

> 2015-03-29  Martin Liska  <mliska@suse.cz>
>
>    PR ipa/65557
>    * ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
>    has already filled up function summary.
>    (sem_item_optimizer::update_hash_by_addr_refs): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2015-03-29  Martin Liska <mliska@suse.cz>
>
>    * g++.dg/ipa/pr65557.C: New test.

--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr65557.C
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
+
+struct S0
+{
+  S0 ()
+  {
+  }
+};
+
+struct S1
+{
+  S1 ()
+  {
+  }
+};
+
+S0 s0;
+S1 s1;

The testcase doesn't clean its IPA dump. However, there is also no
scan dump function, so it is questionable, what the testcase tries to
do with the dump. Also, the flags that trigger the bug in the PR are
different: "-fdevirtualize -fipa-cp -fipa-icf-functions".

Can you please check the testcase?

Uros.

Re: [PATCH] Fix PR ipa/65557

[Martin Liška]

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

On 04/30/2015 11:11 PM, Uros Bizjak wrote:
> Hello!
> 
>> 2015-03-29  Martin Liska  <mliska@suse.cz>
>>
>>    PR ipa/65557
>>    * ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
>>    has already filled up function summary.
>>    (sem_item_optimizer::update_hash_by_addr_refs): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-03-29  Martin Liska <mliska@suse.cz>
>>
>>    * g++.dg/ipa/pr65557.C: New test.
> 
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr65557.C
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
> +
> +struct S0
> +{
> +  S0 ()
> +  {
> +  }
> +};
> +
> +struct S1
> +{
> +  S1 ()
> +  {
> +  }
> +};
> +
> +S0 s0;
> +S1 s1;
> 
> The testcase doesn't clean its IPA dump. However, there is also no
> scan dump function, so it is questionable, what the testcase tries to
> do with the dump. Also, the flags that trigger the bug in the PR are
> different: "-fdevirtualize -fipa-cp -fipa-icf-functions".
> 
> Can you please check the testcase?
> 
> Uros.
> 

Hi.

The test case caused ICE before the patch was applied. So removing -fdump*
is the right fix for the test.

I hope the patch is obvious. Should I also apply the patch for 5.1.0 branch?

Thanks,
Martin

[-- Attachment #2: 0001-Fix-test-case.patch --]
[-- Type: text/x-patch, Size: 738 bytes --]

From 9814faae5ed8a3ffc48932e514648aba559055ea Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Tue, 12 May 2015 16:46:08 +0200
Subject: [PATCH] Fix test case.

gcc/testsuite/ChangeLog:

2015-05-12  Martin Liska  <mliska@suse.cz>

	* g++.dg/ipa/pr65557.C: Remove unnecessary dump flag.
---
 gcc/testsuite/g++.dg/ipa/pr65557.C | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/g++.dg/ipa/pr65557.C b/gcc/testsuite/g++.dg/ipa/pr65557.C
index 2250bb0..98471a3 100644
--- a/gcc/testsuite/g++.dg/ipa/pr65557.C
+++ b/gcc/testsuite/g++.dg/ipa/pr65557.C
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
+/* { dg-options "-O2"  } */
 
 struct S0
 {
-- 
2.1.4

Re: [PATCH] Fix PR ipa/65557

[Uros Bizjak]

On Tue, May 12, 2015 at 4:54 PM, Martin Liška <mliska@suse.cz> wrote:
> On 04/30/2015 11:11 PM, Uros Bizjak wrote:
>> Hello!
>>
>>> 2015-03-29  Martin Liska  <mliska@suse.cz>
>>>
>>>    PR ipa/65557
>>>    * ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
>>>    has already filled up function summary.
>>>    (sem_item_optimizer::update_hash_by_addr_refs): Likewise.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2015-03-29  Martin Liska <mliska@suse.cz>
>>>
>>>    * g++.dg/ipa/pr65557.C: New test.
>>
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/ipa/pr65557.C
>> @@ -0,0 +1,19 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
>> +
>> +struct S0
>> +{
>> +  S0 ()
>> +  {
>> +  }
>> +};
>> +
>> +struct S1
>> +{
>> +  S1 ()
>> +  {
>> +  }
>> +};
>> +
>> +S0 s0;
>> +S1 s1;
>>
>> The testcase doesn't clean its IPA dump. However, there is also no
>> scan dump function, so it is questionable, what the testcase tries to
>> do with the dump. Also, the flags that trigger the bug in the PR are
>> different: "-fdevirtualize -fipa-cp -fipa-icf-functions".
>>
>> Can you please check the testcase?
>>
>> Uros.
>>
>
> Hi.
>
> The test case caused ICE before the patch was applied. So removing -fdump*
> is the right fix for the test.
>
> I hope the patch is obvious. Should I also apply the patch for 5.1.0 branch?

Yes, please. Plenty of efforts were put in the cleaning of various dumps.

Thanks,
Uros.

Re: [PATCH] Fix PR ipa/65557

[James Greenhalgh]

On Wed, Apr 01, 2015 at 11:42:28PM +0100, H.J. Lu wrote:
> On Mon, Mar 30, 2015 at 3:19 PM, Martin Liška <mliska@suse.cz> wrote:
> > You are right, there's one more occurrence of the usage.
> > I'm going to install the patch I've attached.
> >
> 
> This caused:
> 
> FAIL: g++.dg/torture/pr64378.C   -O2 -flto -fno-use-linker-plugin
> -flto-partition=none  (internal compiler error)
> FAIL: g++.dg/torture/pr64378.C   -O2 -flto -fno-use-linker-plugin
> -flto-partition=none  (test for excess errors)
> 
> on x86-64.

Likewise on aarch64-none-elf and aarch64-none-linux-gnu.

Thanks,
James

----

/build-gcc/obj/gcc2/gcc/testsuite/g++6/../../xg++ -B/build-gcc/obj/gcc2/gcc/testsuite/g++6/../../ /gcc-src/gcc/gcc/testsuite/g++.dg/torture/pr64378.C -fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++ -I/build-gcc/obj/gcc2/aarch64-none-elf/libstdc++-v3/include/aarch64-none-elf -I/build-gcc/obj/gcc2/aarch64-none-elf/libstdc++-v3/include -I/gcc-src/gcc/libstdc++-v3/libsupc++ -I/gcc-src/gcc/libstdc++-v3/include/backward -I/gcc-src/gcc/libstdc++-v3/testsuite/util -fmessage-length=0 -O2 -flto -fno-use-linker-plugin -flto-partition=none -fno-ipa-cp -S -specs=aem-ve.specs -mcmodel=small -o pr64378.s

/gcc-src/gcc/gcc/testsuite/g++.dg/torture/pr64378.C:24:1: internal compiler error: Segmentation fault
0xc2c81a crash_signal
	/gcc-src/gcc/gcc/toplev.c:383
0x10e434e vec<ipa_param_descriptor, va_heap, vl_embed>::operator[](unsigned int)
	/gcc-src/gcc/gcc/vec.h:736
0x10e434e vec<ipa_param_descriptor, va_heap, vl_ptr>::operator[](unsigned int)
	/gcc-src/gcc/gcc/vec.h:1202
0x10e434e ipa_is_param_used
	/gcc-src/gcc/gcc/ipa-prop.h:405
0x10e434e ipa_icf::sem_item_optimizer::update_hash_by_addr_refs()
	/gcc-src/gcc/gcc/ipa-icf.c:2511
0x10e45c5 ipa_icf::sem_item_optimizer::execute()
	/gcc-src/gcc/gcc/ipa-icf.c:2394
0x10e6aa6 ipa_icf_driver
	/gcc-src/gcc/gcc/ipa-icf.c:3304
0x10e6aa6 ipa_icf::pass_ipa_icf::execute(function*)
	/gcc-src/gcc/gcc/ipa-icf.c:3351
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

Re: [PATCH] Fix PR ipa/65557

[H.J. Lu]

On Mon, Mar 30, 2015 at 3:19 PM, Martin Liška <mliska@suse.cz> wrote:
> On 03/30/2015 07:03 PM, Jan Hubicka wrote:
>>>
>>> Hello.
>>>
>>> Following patch fixed the issue, where we have to check if function
>>> summary
>>> for IPA CP has been already created.
>>>
>>> Tested on x86_64-linux-pc w/o any new regression introduced.
>>> Ready for trunk?
>>>
>>> Thanks,
>>> Martin
>>
>>
>>> >From c33680093e67328863836a845e847bf1b1b23d0e Mon Sep 17 00:00:00 2001
>>> From: mliska <mliska@suse.cz>
>>> Date: Sun, 29 Mar 2015 20:20:33 +0200
>>> Subject: [PATCH] Fix PR65557.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-03-29  Martin Liska  <mliska@suse.cz>
>>>
>>>         PR ipa/65557
>>>         * ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
>>>         has already filled up function summary.
>>
>>
>> OK, there is one extra occurence of flag_ipa_cp test, so please update it
>> consistently.
>>
>> Honza
>>
>
> Hi.
>
> You are right, there's one more occurrence of the usage.
> I'm going to install the patch I've attached.
>

This caused:

FAIL: g++.dg/torture/pr64378.C   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (internal compiler error)
FAIL: g++.dg/torture/pr64378.C   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)

on x86-64.

-- 
H.J.

Re: [PATCH] Fix PR ipa/65557

[Martin Liška]

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

On 03/30/2015 07:03 PM, Jan Hubicka wrote:
>> Hello.
>>
>> Following patch fixed the issue, where we have to check if function summary
>> for IPA CP has been already created.
>>
>> Tested on x86_64-linux-pc w/o any new regression introduced.
>> Ready for trunk?
>> 	
>> Thanks,
>> Martin
>
>> >From c33680093e67328863836a845e847bf1b1b23d0e Mon Sep 17 00:00:00 2001
>> From: mliska <mliska@suse.cz>
>> Date: Sun, 29 Mar 2015 20:20:33 +0200
>> Subject: [PATCH] Fix PR65557.
>>
>> gcc/ChangeLog:
>>
>> 2015-03-29  Martin Liska  <mliska@suse.cz>
>>
>> 	PR ipa/65557
>> 	* ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
>> 	has already filled up function summary.
>
> OK, there is one extra occurence of flag_ipa_cp test, so please update it consistently.
>
> Honza
>

Hi.

You are right, there's one more occurrence of the usage.
I'm going to install the patch I've attached.

Thanks,
Martin

[-- Attachment #2: 0001-Fix-PR65557.patch --]
[-- Type: text/x-patch, Size: 2024 bytes --]

From d27d5c159b28922858a6f99dc81c1a3e64ea8230 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Sun, 29 Mar 2015 20:20:33 +0200
Subject: [PATCH] Fix PR65557.

gcc/ChangeLog:

2015-03-29  Martin Liska  <mliska@suse.cz>

	PR ipa/65557
	* ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
	has already filled up function summary.
	(sem_item_optimizer::update_hash_by_addr_refs): Likewise.

gcc/testsuite/ChangeLog:

2015-03-29  Martin Liska <mliska@suse.cz>

	* g++.dg/ipa/pr65557.C: New test.
---
 gcc/ipa-icf.c                      |  4 ++--
 gcc/testsuite/g++.dg/ipa/pr65557.C | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr65557.C

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index ad868e1..8626730 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -534,7 +534,7 @@ sem_function::equals_wpa (sem_item *item,
   if (opt_for_fn (decl, flag_devirtualize)
       && (TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE
           || TREE_CODE (TREE_TYPE (item->decl)) == METHOD_TYPE)
-      && (!flag_ipa_cp
+      && (ipa_node_params_sum == NULL
 	  || ipa_is_param_used (IPA_NODE_REF (dyn_cast <cgraph_node *>(node)),
 				0))
       && compare_polymorphic_p ())
@@ -2505,7 +2505,7 @@ sem_item_optimizer::update_hash_by_addr_refs ()
 	      && contains_polymorphic_type_p
 		   (method_class_type (TREE_TYPE (m_items[i]->decl)))
 	      && (DECL_CXX_CONSTRUCTOR_P (m_items[i]->decl)
-		  || ((!flag_ipa_cp
+		  || ((ipa_node_params_sum == NULL
 		       || ipa_is_param_used (
 			    IPA_NODE_REF
 			      (dyn_cast <cgraph_node *>(m_items[i]->node)), 0))
diff --git a/gcc/testsuite/g++.dg/ipa/pr65557.C b/gcc/testsuite/g++.dg/ipa/pr65557.C
new file mode 100644
index 0000000..2250bb0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr65557.C
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
+
+struct S0
+{
+  S0 ()
+  {
+  }
+};
+
+struct S1
+{
+  S1 ()
+  {
+  }
+};
+
+S0 s0;
+S1 s1;
-- 
2.1.4

Re: [PATCH] Fix PR ipa/65557

[Jan Hubicka]

> Hello.
> 
> Following patch fixed the issue, where we have to check if function summary
> for IPA CP has been already created.
> 
> Tested on x86_64-linux-pc w/o any new regression introduced.
> Ready for trunk?
> 	
> Thanks,
> Martin

> >From c33680093e67328863836a845e847bf1b1b23d0e Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Sun, 29 Mar 2015 20:20:33 +0200
> Subject: [PATCH] Fix PR65557.
> 
> gcc/ChangeLog:
> 
> 2015-03-29  Martin Liska  <mliska@suse.cz>
> 
> 	PR ipa/65557
> 	* ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
> 	has already filled up function summary.

OK, there is one extra occurence of flag_ipa_cp test, so please update it consistently.

Honza

[PATCH] Fix PR ipa/65557

[Martin Liška]

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

Hello.

Following patch fixed the issue, where we have to check if function summary
for IPA CP has been already created.

Tested on x86_64-linux-pc w/o any new regression introduced.
Ready for trunk?
	
Thanks,
Martin

[-- Attachment #2: 0001-Fix-PR65557.patch --]
[-- Type: text/x-patch, Size: 1567 bytes --]

From c33680093e67328863836a845e847bf1b1b23d0e Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Sun, 29 Mar 2015 20:20:33 +0200
Subject: [PATCH] Fix PR65557.

gcc/ChangeLog:

2015-03-29  Martin Liska  <mliska@suse.cz>

	PR ipa/65557
	* ipa-icf.c (sem_function::equals_wpa): Check if IPA CP
	has already filled up function summary.

gcc/testsuite/ChangeLog:

2015-03-29  Martin Liska <mliska@suse.cz>

	* g++.dg/ipa/pr65557.C: New test.
---
 gcc/ipa-icf.c                      |  2 +-
 gcc/testsuite/g++.dg/ipa/pr65557.C | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/ipa/pr65557.C

diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index ad868e1..ee7dabe 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -534,7 +534,7 @@ sem_function::equals_wpa (sem_item *item,
   if (opt_for_fn (decl, flag_devirtualize)
       && (TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE
           || TREE_CODE (TREE_TYPE (item->decl)) == METHOD_TYPE)
-      && (!flag_ipa_cp
+      && (ipa_node_params_sum == NULL
 	  || ipa_is_param_used (IPA_NODE_REF (dyn_cast <cgraph_node *>(node)),
 				0))
       && compare_polymorphic_p ())
diff --git a/gcc/testsuite/g++.dg/ipa/pr65557.C b/gcc/testsuite/g++.dg/ipa/pr65557.C
new file mode 100644
index 0000000..2250bb0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr65557.C
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-icf-details"  } */
+
+struct S0
+{
+  S0 ()
+  {
+  }
+};
+
+struct S1
+{
+  S1 ()
+  {
+  }
+};
+
+S0 s0;
+S1 s1;
-- 
2.1.4