ICF: compare type attributes for gimple_call_fntypes.

Message ID d6847297-ed1f-4406-fc2d-fa92f17be0b1@suse.cz
State New
Headers show
Series
  • ICF: compare type attributes for gimple_call_fntypes.
Related show

Commit Message

Martin Liška April 2, 2020, 3:16 p.m.
Hi.

The patch compares type attributes for gimple_call_fntypes in IPA ICF.
Note that we were unable to find a generic function attribute that
can be used on a function type definition.

For a one which is allowed assume_aligned(16) I get affects_type_identity == false
which seems suspicious to me.

Note that we currently use comp_type_attributes in ICF for both variable and
function declarations.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-04-02  Martin Liska  <mliska@suse.cz>

	PR ipa/94445
	* ipa-icf-gimple.c (func_checker::compare_gimple_call):
	  Compare type attributes for gimple_call_fntypes.
---
  gcc/ipa-icf-gimple.c | 4 ++++
  1 file changed, 4 insertions(+)

Comments

Xionghu Luo via Gcc-patches April 2, 2020, 5:17 p.m. | #1
On Thu, 2 Apr 2020 at 17:16, Martin Liška <mliska@suse.cz> wrote:
>

> Hi.

>

> The patch compares type attributes for gimple_call_fntypes in IPA ICF.

> Note that we were unable to find a generic function attribute that

> can be used on a function type definition.

>

> For a one which is allowed assume_aligned(16) I get affects_type_identity == false

> which seems suspicious to me.

>

> Note that we currently use comp_type_attributes in ICF for both variable and

> function declarations.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>

> Ready to be installed?

> Thanks,

> Martin

>


Hi,

Thanks for the quick patch!

I confirm it fixes the problem I noticed on arm with the cmse-15.c for
-O2 and -O3.

However, the testcase still fails with -Os. I haven't looked at the
details, so it may be a different cause.

Thanks,

Christophe

> gcc/ChangeLog:

>

> 2020-04-02  Martin Liska  <mliska@suse.cz>

>

>         PR ipa/94445

>         * ipa-icf-gimple.c (func_checker::compare_gimple_call):

>           Compare type attributes for gimple_call_fntypes.

> ---

>   gcc/ipa-icf-gimple.c | 4 ++++

>   1 file changed, 4 insertions(+)

>

>
Xionghu Luo via Gcc-patches April 3, 2020, 6:59 a.m. | #2
On Thu, Apr 2, 2020 at 5:16 PM Martin Liška <mliska@suse.cz> wrote:
>

> Hi.

>

> The patch compares type attributes for gimple_call_fntypes in IPA ICF.

> Note that we were unable to find a generic function attribute that

> can be used on a function type definition.

>

> For a one which is allowed assume_aligned(16) I get affects_type_identity == false

> which seems suspicious to me.

>

> Note that we currently use comp_type_attributes in ICF for both variable and

> function declarations.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>

> Ready to be installed?


OK.

Richard.

> Thanks,

> Martin

>

> gcc/ChangeLog:

>

> 2020-04-02  Martin Liska  <mliska@suse.cz>

>

>         PR ipa/94445

>         * ipa-icf-gimple.c (func_checker::compare_gimple_call):

>           Compare type attributes for gimple_call_fntypes.

> ---

>   gcc/ipa-icf-gimple.c | 4 ++++

>   1 file changed, 4 insertions(+)

>

>
Martin Liška April 3, 2020, 7:04 a.m. | #3
On 4/2/20 7:17 PM, Christophe Lyon wrote:
> On Thu, 2 Apr 2020 at 17:16, Martin Liška <mliska@suse.cz> wrote:

>>

>> Hi.

>>

>> The patch compares type attributes for gimple_call_fntypes in IPA ICF.

>> Note that we were unable to find a generic function attribute that

>> can be used on a function type definition.

>>

>> For a one which is allowed assume_aligned(16) I get affects_type_identity == false

>> which seems suspicious to me.

>>

>> Note that we currently use comp_type_attributes in ICF for both variable and

>> function declarations.

>>

>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>>

>> Ready to be installed?

>> Thanks,

>> Martin

>>

> 

> Hi,

> 

> Thanks for the quick patch!


Hi.

Good!

> 

> I confirm it fixes the problem I noticed on arm with the cmse-15.c for

> -O2 and -O3.

> 

> However, the testcase still fails with -Os. I haven't looked at the

> details, so it may be a different cause.


I analyzed that slightly and there are still IPA ICF merged that happen
(this time valid transformations):

/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c:14:5: optimized: Semantic equality hit:nonsecure0/4->nonsecure2/6
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c:14:5: optimized: Assembler symbol names:nonsecure0/4->nonsecure2/6

/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c:33:5: optimized: Semantic equality hit:secure0/8->secure2/10
/home/marxin/Programming/gcc/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c:33:5: optimized: Assembler symbol names:secure0/8->secure2/10

and with -Os nosecure2 -> mosecure0 call is not inlined bad again. That makes the scanning pattern to miss:

-Os version:
nonsecure2:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	b	nonsecure0

-O2 version:
nonsecure2:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	push	{r4, lr}
	lsrs	r0, r0, #1
	lsls	r0, r0, #1
	mov	r1, r0
	mov	r2, r0
	mov	r3, r0
	bl	__gnu_cmse_nonsecure_call

Martin

> 

> Thanks,

> 

> Christophe

> 

>> gcc/ChangeLog:

>>

>> 2020-04-02  Martin Liska  <mliska@suse.cz>

>>

>>          PR ipa/94445

>>          * ipa-icf-gimple.c (func_checker::compare_gimple_call):

>>            Compare type attributes for gimple_call_fntypes.

>> ---

>>    gcc/ipa-icf-gimple.c | 4 ++++

>>    1 file changed, 4 insertions(+)

>>

>>

Patch

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 3e5b2d4bd6d..fa02809defd 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -37,6 +37,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-eh.h"
 #include "builtins.h"
 #include "cfgloop.h"
+#include "attribs.h"
 
 #include "ipa-icf-gimple.h"
 
@@ -568,6 +569,9 @@  func_checker::compare_gimple_call (gcall *s1, gcall *s2)
       || (fntype1 && !types_compatible_p (fntype1, fntype2)))
     return return_false_with_msg ("call function types are not compatible");
 
+  if (fntype1 && fntype2 && comp_type_attributes (fntype1, fntype2) != 1)
+    return return_false_with_msg ("different fntype attributes");
+
   tree chain1 = gimple_call_chain (s1);
   tree chain2 = gimple_call_chain (s2);
   if ((chain1 && !chain2)