diff mbox series

icf: Check return type of internal fn calls [PR99517]

Message ID 20210311084147.GG745611@tucnak
State New
Headers show
Series icf: Check return type of internal fn calls [PR99517] | expand

Commit Message

Jakub Jelinek March 11, 2021, 8:41 a.m. UTC
Hi!

The following testcase is miscompiled, because IPA-ICF considers the two
functions identical.  They aren't, the types of the .VEC_CONVERT call
lhs is different.  But for calls to internal functions, there is no
fntype nor callee with a function type to compare, so all we compare
is just the ifn, arguments and some call flags.

The following patch fixes it by checking the internal fn calls like e.g. gimple
assignments where the type of the lhs is checked too.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/99517
	* ipa-icf-gimple.c (func_checker::compare_gimple_call): For internal
	function calls with lhs fail if the lhs don't have compatible types.

	* gcc.target/i386/avx2-pr99517-1.c: New test.
	* gcc.target/i386/avx2-pr99517-2.c: New test.


	Jakub

Comments

Richard Biener March 11, 2021, 9:18 a.m. UTC | #1
On Thu, 11 Mar 2021, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled, because IPA-ICF considers the two
> functions identical.  They aren't, the types of the .VEC_CONVERT call
> lhs is different.  But for calls to internal functions, there is no
> fntype nor callee with a function type to compare, so all we compare
> is just the ifn, arguments and some call flags.
> 
> The following patch fixes it by checking the internal fn calls like e.g. gimple
> assignments where the type of the lhs is checked too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2021-03-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR ipa/99517
> 	* ipa-icf-gimple.c (func_checker::compare_gimple_call): For internal
> 	function calls with lhs fail if the lhs don't have compatible types.
> 
> 	* gcc.target/i386/avx2-pr99517-1.c: New test.
> 	* gcc.target/i386/avx2-pr99517-2.c: New test.
> 
> --- gcc/ipa-icf-gimple.c.jj	2021-01-04 10:25:38.752234741 +0100
> +++ gcc/ipa-icf-gimple.c	2021-03-10 15:02:06.287502784 +0100
> @@ -667,7 +667,7 @@ func_checker::compare_gimple_call (gcall
>    tree fntype1 = gimple_call_fntype (s1);
>    tree fntype2 = gimple_call_fntype (s2);
>  
> -  /* For direct calls we verify that types are comopatible so if we matced
> +  /* For direct calls we verify that types are compatible so if we matched
>       callees, callers must match, too.  For indirect calls however verify
>       function type.  */
>    if (!gimple_call_fndecl (s1))
> @@ -703,6 +703,14 @@ func_checker::compare_gimple_call (gcall
>    t1 = gimple_get_lhs (s1);
>    t2 = gimple_get_lhs (s2);
>  
> +  /* For internal calls, lhs types need to be verified, as neither fntype nor
> +     callee comparisons can catch that.  */
> +  if (gimple_call_internal_p (s1)
> +      && t1
> +      && t2
> +      && !compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
> +    return return_false_with_msg ("GIMPLE internal call LHS type mismatch");
> +
>    return compare_operand (t1, t2, get_operand_access_type (&map, t1));
>  }
>  
> --- gcc/testsuite/gcc.target/i386/avx2-pr99517-1.c.jj	2021-03-10 15:28:33.175959797 +0100
> +++ gcc/testsuite/gcc.target/i386/avx2-pr99517-1.c	2021-03-10 15:28:57.117695186 +0100
> @@ -0,0 +1,25 @@
> +/* PR ipa/99517 */
> +/* { dg-do run { target avx2 } } */
> +/* { dg-additional-sources "avx2-pr99517-2.c" } */
> +/* { dg-options "-O2 -mavx2" } */
> +
> +#include "avx2-check.h"
> +
> +typedef signed char v32qi __attribute__((vector_size(32)));
> +typedef int v4si __attribute__((vector_size(16)));
> +typedef long long int v4di __attribute__((vector_size(32)));
> +typedef double v4df __attribute__((vector_size(32)));
> +extern v32qi foo (v4si);
> +extern v32qi bar (v4si);
> +
> +static void
> +avx2_test (void)
> +{
> +  v4si a = { 1, -2, 3, -4 };
> +  __asm ("" : "+x" (a));
> +  v4di b = (v4di) bar (a);
> +  v4df c = (v4df) foo (a);
> +  if (b[0] != 1 || c[0] != 1.0 || b[1] != -2 || c[1] != -2.0
> +      || b[2] != 3 || c[2] != 3.0 || b[3] != -4 || c[3] != -4.0)
> +    __builtin_abort ();
> +}
> --- gcc/testsuite/gcc.target/i386/avx2-pr99517-2.c.jj	2021-03-10 15:30:33.974624677 +0100
> +++ gcc/testsuite/gcc.target/i386/avx2-pr99517-2.c	2021-03-10 15:30:28.529684856 +0100
> @@ -0,0 +1,20 @@
> +/* PR ipa/99517 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mavx2" } */
> +
> +typedef signed char v32qi __attribute__((vector_size(32)));
> +typedef int v4si __attribute__((vector_size(16)));
> +typedef long long int v4di __attribute__((vector_size(32)));
> +typedef double v4df __attribute__((vector_size(32)));
> +
> +v32qi
> +foo (v4si x)
> +{
> +  return (v32qi) __builtin_convertvector (x, v4df);
> +}
> +
> +v32qi
> +bar (v4si x)
> +{
> +  return (v32qi) __builtin_convertvector (x, v4di);
> +}
> 
> 	Jakub
> 
>
diff mbox series

Patch

--- gcc/ipa-icf-gimple.c.jj	2021-01-04 10:25:38.752234741 +0100
+++ gcc/ipa-icf-gimple.c	2021-03-10 15:02:06.287502784 +0100
@@ -667,7 +667,7 @@  func_checker::compare_gimple_call (gcall
   tree fntype1 = gimple_call_fntype (s1);
   tree fntype2 = gimple_call_fntype (s2);
 
-  /* For direct calls we verify that types are comopatible so if we matced
+  /* For direct calls we verify that types are compatible so if we matched
      callees, callers must match, too.  For indirect calls however verify
      function type.  */
   if (!gimple_call_fndecl (s1))
@@ -703,6 +703,14 @@  func_checker::compare_gimple_call (gcall
   t1 = gimple_get_lhs (s1);
   t2 = gimple_get_lhs (s2);
 
+  /* For internal calls, lhs types need to be verified, as neither fntype nor
+     callee comparisons can catch that.  */
+  if (gimple_call_internal_p (s1)
+      && t1
+      && t2
+      && !compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
+    return return_false_with_msg ("GIMPLE internal call LHS type mismatch");
+
   return compare_operand (t1, t2, get_operand_access_type (&map, t1));
 }
 
--- gcc/testsuite/gcc.target/i386/avx2-pr99517-1.c.jj	2021-03-10 15:28:33.175959797 +0100
+++ gcc/testsuite/gcc.target/i386/avx2-pr99517-1.c	2021-03-10 15:28:57.117695186 +0100
@@ -0,0 +1,25 @@ 
+/* PR ipa/99517 */
+/* { dg-do run { target avx2 } } */
+/* { dg-additional-sources "avx2-pr99517-2.c" } */
+/* { dg-options "-O2 -mavx2" } */
+
+#include "avx2-check.h"
+
+typedef signed char v32qi __attribute__((vector_size(32)));
+typedef int v4si __attribute__((vector_size(16)));
+typedef long long int v4di __attribute__((vector_size(32)));
+typedef double v4df __attribute__((vector_size(32)));
+extern v32qi foo (v4si);
+extern v32qi bar (v4si);
+
+static void
+avx2_test (void)
+{
+  v4si a = { 1, -2, 3, -4 };
+  __asm ("" : "+x" (a));
+  v4di b = (v4di) bar (a);
+  v4df c = (v4df) foo (a);
+  if (b[0] != 1 || c[0] != 1.0 || b[1] != -2 || c[1] != -2.0
+      || b[2] != 3 || c[2] != 3.0 || b[3] != -4 || c[3] != -4.0)
+    __builtin_abort ();
+}
--- gcc/testsuite/gcc.target/i386/avx2-pr99517-2.c.jj	2021-03-10 15:30:33.974624677 +0100
+++ gcc/testsuite/gcc.target/i386/avx2-pr99517-2.c	2021-03-10 15:30:28.529684856 +0100
@@ -0,0 +1,20 @@ 
+/* PR ipa/99517 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx2" } */
+
+typedef signed char v32qi __attribute__((vector_size(32)));
+typedef int v4si __attribute__((vector_size(16)));
+typedef long long int v4di __attribute__((vector_size(32)));
+typedef double v4df __attribute__((vector_size(32)));
+
+v32qi
+foo (v4si x)
+{
+  return (v32qi) __builtin_convertvector (x, v4df);
+}
+
+v32qi
+bar (v4si x)
+{
+  return (v32qi) __builtin_convertvector (x, v4di);
+}