public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] graphite: Fix non-INTEGER_TYPE integral comparison handling [PR114041]
@ 2024-02-28  8:09 Jakub Jelinek
  2024-02-28  8:28 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2024-02-28  8:09 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

The following testcases are miscompiled, because graphite ignores boolean,
enumerated or _BitInt comparisons, rewrites the code as if the comparisons
were always true or always false.

The INTEGER_TYPE checks were initially added in r6-2239 but at that point
it was both in add_conditions_to_domain and in parameter_index_in_region.
Later on the check was also added to stmt_simple_for_scop_p, and finally
r8-3931 changed the stmt_simple_for_scop_p check to INTEGRAL_TYPE_P
and turned the parameter_index_in_region -> assign_parameter_index_in_region
into INTEGRAL_TYPE_P assertion, but the add_conditions_to_domain check
for INTEGER_TYPE remained.

The following patch uses INTEGRAL_TYPE_P to complete the change.

Bootstrapped/regtested on x86_64-linux and i686-linux (--with-isl only
on the former though), ok for trunk?

2024-02-28  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/114041
	* graphite-sese-to-poly.cc (add_conditions_to_domain): Check for
	INTEGRAL_TYPE_P check rather than INTEGER_TYPE.

	* gcc.dg/graphite/run-id-pr114041-1.c: New test.
	* gcc.dg/graphite/run-id-pr114041-2.c: New test.

--- gcc/graphite-sese-to-poly.cc.jj	2024-01-03 11:51:29.136764430 +0100
+++ gcc/graphite-sese-to-poly.cc	2024-02-27 19:35:07.668304435 +0100
@@ -391,8 +391,9 @@ add_conditions_to_domain (poly_bb_p pbb)
       {
       case GIMPLE_COND:
 	  {
-            /* Don't constrain on anything else than INTEGER_TYPE.  */
-	    if (TREE_CODE (TREE_TYPE (gimple_cond_lhs (stmt))) != INTEGER_TYPE)
+	    /* Don't constrain on anything else than INTEGRAL_TYPE_P.  */
+	    tree cmp_type = TREE_TYPE (gimple_cond_lhs (stmt));
+	    if (!INTEGRAL_TYPE_P (cmp_type))
               break;
 
 	    gcond *cond_stmt = as_a <gcond *> (stmt);
--- gcc/testsuite/gcc.dg/graphite/run-id-pr114041-1.c.jj	2024-02-27 18:42:26.864025806 +0100
+++ gcc/testsuite/gcc.dg/graphite/run-id-pr114041-1.c	2024-02-27 18:43:07.310466262 +0100
@@ -0,0 +1,23 @@
+/* PR tree-optimization/114041 */
+/* { dg-require-effective-target bitint } */
+/* { dg-options "-O -fgraphite-identity" } */
+
+unsigned a[24], b[24];
+
+__attribute__((noipa)) unsigned
+foo (unsigned _BitInt(8) x)
+{
+  for (int i = 0; i < 24; ++i)
+    a[i] = i;
+  unsigned e = __builtin_stdc_bit_ceil (x);
+  for (int i = 0; i < 24; ++i)
+    b[i] = i;
+  return e;
+}
+
+int
+main ()
+{
+  if (foo (0) != 1)
+    __builtin_abort ();
+}
--- gcc/testsuite/gcc.dg/graphite/run-id-pr114041-2.c.jj	2024-02-27 19:36:02.373547881 +0100
+++ gcc/testsuite/gcc.dg/graphite/run-id-pr114041-2.c	2024-02-27 19:36:22.515269333 +0100
@@ -0,0 +1,27 @@
+/* PR tree-optimization/114041 */
+/* { dg-options "-O -fgraphite-identity" } */
+
+unsigned a[24], b[24];
+enum E { E0 = 0, E1 = 1, E42 = 42, E56 = 56 };
+
+__attribute__((noipa)) unsigned
+foo (enum E x)
+{
+  for (int i = 0; i < 24; ++i)
+    a[i] = i;
+  unsigned e;
+  if (x >= E42)
+    e = __builtin_clz ((unsigned) x);
+  else
+    e = 42;
+  for (int i = 0; i < 24; ++i)
+    b[i] = i;
+  return e;
+}
+
+int
+main ()
+{
+  if (foo (E1) != 42 || foo (E56) != __SIZEOF_INT__ * __CHAR_BIT__ - 6)
+    __builtin_abort ();
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] graphite: Fix non-INTEGER_TYPE integral comparison handling [PR114041]
  2024-02-28  8:09 [PATCH] graphite: Fix non-INTEGER_TYPE integral comparison handling [PR114041] Jakub Jelinek
@ 2024-02-28  8:28 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2024-02-28  8:28 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Wed, 28 Feb 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following testcases are miscompiled, because graphite ignores boolean,
> enumerated or _BitInt comparisons, rewrites the code as if the comparisons
> were always true or always false.
> 
> The INTEGER_TYPE checks were initially added in r6-2239 but at that point
> it was both in add_conditions_to_domain and in parameter_index_in_region.
> Later on the check was also added to stmt_simple_for_scop_p, and finally
> r8-3931 changed the stmt_simple_for_scop_p check to INTEGRAL_TYPE_P
> and turned the parameter_index_in_region -> assign_parameter_index_in_region
> into INTEGRAL_TYPE_P assertion, but the add_conditions_to_domain check
> for INTEGER_TYPE remained.
> 
> The following patch uses INTEGRAL_TYPE_P to complete the change.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux (--with-isl only
> on the former though), ok for trunk?

OK.  As said this should probably be an assert, but I won't bother you
with that (and the possible fallout).

Thanks,
Richard.

> 2024-02-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/114041
> 	* graphite-sese-to-poly.cc (add_conditions_to_domain): Check for
> 	INTEGRAL_TYPE_P check rather than INTEGER_TYPE.
> 
> 	* gcc.dg/graphite/run-id-pr114041-1.c: New test.
> 	* gcc.dg/graphite/run-id-pr114041-2.c: New test.
> 
> --- gcc/graphite-sese-to-poly.cc.jj	2024-01-03 11:51:29.136764430 +0100
> +++ gcc/graphite-sese-to-poly.cc	2024-02-27 19:35:07.668304435 +0100
> @@ -391,8 +391,9 @@ add_conditions_to_domain (poly_bb_p pbb)
>        {
>        case GIMPLE_COND:
>  	  {
> -            /* Don't constrain on anything else than INTEGER_TYPE.  */
> -	    if (TREE_CODE (TREE_TYPE (gimple_cond_lhs (stmt))) != INTEGER_TYPE)
> +	    /* Don't constrain on anything else than INTEGRAL_TYPE_P.  */
> +	    tree cmp_type = TREE_TYPE (gimple_cond_lhs (stmt));
> +	    if (!INTEGRAL_TYPE_P (cmp_type))
>                break;
>  
>  	    gcond *cond_stmt = as_a <gcond *> (stmt);
> --- gcc/testsuite/gcc.dg/graphite/run-id-pr114041-1.c.jj	2024-02-27 18:42:26.864025806 +0100
> +++ gcc/testsuite/gcc.dg/graphite/run-id-pr114041-1.c	2024-02-27 18:43:07.310466262 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/114041 */
> +/* { dg-require-effective-target bitint } */
> +/* { dg-options "-O -fgraphite-identity" } */
> +
> +unsigned a[24], b[24];
> +
> +__attribute__((noipa)) unsigned
> +foo (unsigned _BitInt(8) x)
> +{
> +  for (int i = 0; i < 24; ++i)
> +    a[i] = i;
> +  unsigned e = __builtin_stdc_bit_ceil (x);
> +  for (int i = 0; i < 24; ++i)
> +    b[i] = i;
> +  return e;
> +}
> +
> +int
> +main ()
> +{
> +  if (foo (0) != 1)
> +    __builtin_abort ();
> +}
> --- gcc/testsuite/gcc.dg/graphite/run-id-pr114041-2.c.jj	2024-02-27 19:36:02.373547881 +0100
> +++ gcc/testsuite/gcc.dg/graphite/run-id-pr114041-2.c	2024-02-27 19:36:22.515269333 +0100
> @@ -0,0 +1,27 @@
> +/* PR tree-optimization/114041 */
> +/* { dg-options "-O -fgraphite-identity" } */
> +
> +unsigned a[24], b[24];
> +enum E { E0 = 0, E1 = 1, E42 = 42, E56 = 56 };
> +
> +__attribute__((noipa)) unsigned
> +foo (enum E x)
> +{
> +  for (int i = 0; i < 24; ++i)
> +    a[i] = i;
> +  unsigned e;
> +  if (x >= E42)
> +    e = __builtin_clz ((unsigned) x);
> +  else
> +    e = 42;
> +  for (int i = 0; i < 24; ++i)
> +    b[i] = i;
> +  return e;
> +}
> +
> +int
> +main ()
> +{
> +  if (foo (E1) != 42 || foo (E56) != __SIZEOF_INT__ * __CHAR_BIT__ - 6)
> +    __builtin_abort ();
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-02-28  8:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-28  8:09 [PATCH] graphite: Fix non-INTEGER_TYPE integral comparison handling [PR114041] Jakub Jelinek
2024-02-28  8:28 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).