diff mbox series

gimplify: Small RAW_DATA_CST gimplification fix

Message ID Zq5zZfILnVxJN9vy@tucnak
State New
Headers
Series gimplify: Small RAW_DATA_CST gimplification fix |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Jakub Jelinek Aug. 3, 2024, 6:13 p.m. UTC 
  Hi!

I've noticed the following testcase on top of the #embed patchset:
https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655012.html                                                                                                                       
https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655013.html                                                                                                                       
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657049.html                                                                                                                       
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657053.html  
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658137.html
hangs during gimplification.

While it is gimplifying an assignment from a VAR_DECL .LCNNN to MEM_REF,
because the VAR_DECL is TREE_READONLY, it will happily pick its initializer
and try to gimplify that, which means recursing to the exact same code.

The following patch fixes that by just gimplifying the lhs and building
assignment, because the code decided that it should use copying from
a static var.

Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?

2024-08-03  Jakub Jelinek  <jakub@redhat.com>

	* gimplify.cc (gimplify_init_ctor_eval): For larger RAW_DATA_CST,
	just gimplify cref as lvalue and add gimple assignment of rctor
	to cref instead of going through gimplification of INIT_EXPR, as
	the latter can suffer from infinite recursion.

	* c-c++-common/cpp/embed-24.c: New test.

                                                                                                                     

	Jakub

Comments

Richard Biener Aug. 3, 2024, 6:39 p.m. UTC | #1 
> Am 03.08.2024 um 20:14 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> I've noticed the following testcase on top of the #embed patchset:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655012.html                                                                                                                       
> https://gcc.gnu.org/pipermail/gcc-patches/2024-June/655013.html                                                                                                                       
> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657049.html                                                                                                                       
> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657053.html  
> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658137.html
> hangs during gimplification.
> 
> While it is gimplifying an assignment from a VAR_DECL .LCNNN to MEM_REF,
> because the VAR_DECL is TREE_READONLY, it will happily pick its initializer
> and try to gimplify that, which means recursing to the exact same code.
> 
> The following patch fixes that by just gimplifying the lhs and building
> assignment, because the code decided that it should use copying from
> a static var.
> 
> Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?

Ok

Richard 

> 2024-08-03  Jakub Jelinek  <jakub@redhat.com>
> 
>    * gimplify.cc (gimplify_init_ctor_eval): For larger RAW_DATA_CST,
>    just gimplify cref as lvalue and add gimple assignment of rctor
>    to cref instead of going through gimplification of INIT_EXPR, as
>    the latter can suffer from infinite recursion.
> 
>    * c-c++-common/cpp/embed-24.c: New test.
> 
> --- gcc/gimplify.cc.jj    2024-08-02 11:34:03.651027803 +0200
> +++ gcc/gimplify.cc    2024-08-03 10:19:47.071916653 +0200
> @@ -5400,9 +5400,10 @@ gimplify_init_ctor_eval (tree object, ve
>          cref = build2 (MEM_REF, rtype, addr,
>                 build_int_cst (ptr_type_node, 0));
>          rctor = tree_output_constant_def (rctor);
> -          tree init = build2 (INIT_EXPR, rtype, cref, rctor);
> -          gimplify_and_add (init, pre_p);
> -          ggc_free (init);
> +          if (gimplify_expr (&cref, pre_p, NULL, is_gimple_lvalue,
> +                 fb_lvalue) != GS_ERROR)
> +        gimplify_seq_add_stmt (pre_p,
> +                       gimple_build_assign (cref, rctor));
>        }
>    }
>       else
> --- gcc/testsuite/c-c++-common/cpp/embed-24.c.jj    2024-08-03 10:10:42.145076112 +0200
> +++ gcc/testsuite/c-c++-common/cpp/embed-24.c    2024-08-03 19:40:58.693297792 +0200
> @@ -0,0 +1,52 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-std=c23" { target c } } */
> +
> +static unsigned char a[] = {
> +#embed __FILE__ limit (125)
> +};
> +
> +void
> +foo (unsigned char *p)
> +{
> +  for (int i = 0; i < 128; ++i)
> +    if (p[i] != ((i < 64 || i == 127) ? (unsigned char) -1 : i - 64 + 33))
> +      __builtin_abort ();
> +  if (__builtin_memcmp (p + 128, a, 125))
> +    __builtin_abort ();
> +  for (int i = 253; i < 256; ++i)
> +    if (p[i] != (unsigned char) -1)
> +      __builtin_abort ();
> +}
> +
> +#ifdef __cplusplus
> +#define M1 (unsigned char) -1
> +#else
> +#define M1 -1
> +#endif
> +
> +int
> +main ()
> +{
> +  unsigned char res[256] = {
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    M1, M1, M1, M1, M1, M1, M1, M1,
> +    33, 34, 35, 36, 37, 38, 39, 40,
> +    41, 42, 43, 44, 45, 46, 47, 48,
> +    49, 50, 51, 52, 53, 54, 55, 56,
> +    57, 58, 59, 60, 61, 62, 63, 64,
> +    65, 66, 67, 68, 69, 70, 71, 72,
> +    73, 74, 75, 76, 77, 78, 79, 80,
> +    81, 82, 83, 84, 85, 86, 87, 88,
> +    89, 90, 91, 92, 93, 94, 95, M1,
> +  #embed __FILE__ limit (125) suffix (,)
> +    M1, M1, M1
> +  };
> +  foo (res);
> +}
> 
> 
>    Jakub
>
diff mbox series

Patch

--- gcc/gimplify.cc.jj	2024-08-02 11:34:03.651027803 +0200
+++ gcc/gimplify.cc	2024-08-03 10:19:47.071916653 +0200
@@ -5400,9 +5400,10 @@  gimplify_init_ctor_eval (tree object, ve
 	      cref = build2 (MEM_REF, rtype, addr,
 			     build_int_cst (ptr_type_node, 0));
 	      rctor = tree_output_constant_def (rctor);
-	      tree init = build2 (INIT_EXPR, rtype, cref, rctor);
-	      gimplify_and_add (init, pre_p);
-	      ggc_free (init);
+	      if (gimplify_expr (&cref, pre_p, NULL, is_gimple_lvalue,
+				 fb_lvalue) != GS_ERROR)
+		gimplify_seq_add_stmt (pre_p,
+				       gimple_build_assign (cref, rctor));
 	    }
 	}
       else
--- gcc/testsuite/c-c++-common/cpp/embed-24.c.jj	2024-08-03 10:10:42.145076112 +0200
+++ gcc/testsuite/c-c++-common/cpp/embed-24.c	2024-08-03 19:40:58.693297792 +0200
@@ -0,0 +1,52 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+/* { dg-additional-options "-std=c23" { target c } } */
+
+static unsigned char a[] = {
+#embed __FILE__ limit (125)
+};
+
+void
+foo (unsigned char *p)
+{
+  for (int i = 0; i < 128; ++i)
+    if (p[i] != ((i < 64 || i == 127) ? (unsigned char) -1 : i - 64 + 33))
+      __builtin_abort ();
+  if (__builtin_memcmp (p + 128, a, 125))
+    __builtin_abort ();
+  for (int i = 253; i < 256; ++i)
+    if (p[i] != (unsigned char) -1)
+      __builtin_abort ();
+}
+
+#ifdef __cplusplus
+#define M1 (unsigned char) -1
+#else
+#define M1 -1
+#endif
+
+int
+main ()
+{
+  unsigned char res[256] = {
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    M1, M1, M1, M1, M1, M1, M1, M1, 
+    33, 34, 35, 36, 37, 38, 39, 40, 
+    41, 42, 43, 44, 45, 46, 47, 48, 
+    49, 50, 51, 52, 53, 54, 55, 56, 
+    57, 58, 59, 60, 61, 62, 63, 64, 
+    65, 66, 67, 68, 69, 70, 71, 72, 
+    73, 74, 75, 76, 77, 78, 79, 80, 
+    81, 82, 83, 84, 85, 86, 87, 88, 
+    89, 90, 91, 92, 93, 94, 95, M1,
+  #embed __FILE__ limit (125) suffix (,)
+    M1, M1, M1
+  };
+  foo (res);
+}