tree-optimization/112827 - corrupt SCEV cache during SCCP

Message ID 20231204115340.CA9C813588@imap2.dmz-prg2.suse.org
State Committed
Commit de0ab339a795352c843f6e9b2dfce222f26588de
Headers
Series tree-optimization/112827 - corrupt SCEV cache during SCCP |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 warning Patch is already merged

Commit Message

Richard Biener Dec. 4, 2023, 11:53 a.m. UTC
  The following avoids corrupting the SCEV cache by my last change
to propagate constant final values immediately.  The easiest fix
is to keep a dead initialization around.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

	PR tree-optimization/112827
	* tree-scalar-evolution.cc (final_value_replacement_loop):
	Do not release SSA name but keep a dead initialization around.

	* gcc.dg/torture/pr112827-1.c: New testcase.
	* gcc.dg/torture/pr112827-2.c: Likewise.
---
 gcc/testsuite/gcc.dg/torture/pr112827-1.c | 14 ++++++++++++++
 gcc/testsuite/gcc.dg/torture/pr112827-2.c | 18 ++++++++++++++++++
 gcc/tree-scalar-evolution.cc              |  9 +++------
 3 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-1.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-2.c
  

Comments

Li, Pan2 Dec. 5, 2023, 7:19 a.m. UTC | #1
Hi Richard,

It looks like this patch result in one ICE for RISC-V backend for case tree-ssa.exp=ssa-sink-16.c, could you please help to double check about it?
Any more information required please feel free to let me know. Thanks.

compiler error: Segmentation fault
0x1903067 crash_signal
        ../../../../gcc/gcc/toplev.cc:316
0x111a24e loop_outer(loop const*)
        ../../../../gcc/gcc/cfgloop.h:549
0x1ac2143 find_uses_to_rename_use
        ../../../../gcc/gcc/tree-ssa-loop-manip.cc:424
0x1ac2295 find_uses_to_rename_stmt
        ../../../../gcc/gcc/tree-ssa-loop-manip.cc:464
0x1ac2456 find_uses_to_rename_bb
        ../../../../gcc/gcc/tree-ssa-loop-manip.cc:495
0x1ac2585 find_uses_to_rename
        ../../../../gcc/gcc/tree-ssa-loop-manip.cc:521
0x1ac267c rewrite_into_loop_closed_ssa_1
        ../../../../gcc/gcc/tree-ssa-loop-manip.cc:588
0x1ac2735 rewrite_into_loop_closed_ssa(bitmap_head*, unsigned int)
        ../../../../gcc/gcc/tree-ssa-loop-manip.cc:628
0x19682a3 repair_loop_structures
        ../../../../gcc/gcc/tree-cfgcleanup.cc:1190
0x196831d cleanup_tree_cfg(unsigned int)
        ../../../../gcc/gcc/tree-cfgcleanup.cc:1209
0x16e654b execute_function_todo
        ../../../../gcc/gcc/passes.cc:2057
0x16e534d do_per_function
        ../../../../gcc/gcc/passes.cc:1687
0x16e68b0 execute_todo
        ../../../../gcc/gcc/passes.cc:2142

Pan

-----Original Message-----
From: Richard Biener <rguenther@suse.de> 
Sent: Monday, December 4, 2023 7:54 PM
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] tree-optimization/112827 - corrupt SCEV cache during SCCP

The following avoids corrupting the SCEV cache by my last change
to propagate constant final values immediately.  The easiest fix
is to keep a dead initialization around.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

	PR tree-optimization/112827
	* tree-scalar-evolution.cc (final_value_replacement_loop):
	Do not release SSA name but keep a dead initialization around.

	* gcc.dg/torture/pr112827-1.c: New testcase.
	* gcc.dg/torture/pr112827-2.c: Likewise.
---
 gcc/testsuite/gcc.dg/torture/pr112827-1.c | 14 ++++++++++++++
 gcc/testsuite/gcc.dg/torture/pr112827-2.c | 18 ++++++++++++++++++
 gcc/tree-scalar-evolution.cc              |  9 +++------
 3 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-1.c
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-2.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-1.c b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
new file mode 100644
index 00000000000..6838cbbe62f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+
+int a, b, c, d, e;
+int main() {
+  for (; c; c++) {
+    for (a = 0; a < 2; a++)
+      ;
+    for (; b; b++) {
+      e = d;
+      d = a;
+    }
+  }
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-2.c b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
new file mode 100644
index 00000000000..a7a2a70211b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+
+short a, b[1], f;
+char c, g;
+int d, e;
+int main() {
+  for (; f; f++) {
+    for (d = 0; d < 2; d++)
+      ;
+    if (a)
+      for (g = 0; g < 2; g++)
+        for (c = 0; c < 2; c += b[d+g])
+          ;
+    for (; e; e++)
+      ;
+  }
+  return 0;
+}
diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
index 065bcd0743d..7556d89e9f8 100644
--- a/gcc/tree-scalar-evolution.cc
+++ b/gcc/tree-scalar-evolution.cc
@@ -3847,13 +3847,10 @@ final_value_replacement_loop (class loop *loop)
       def = unshare_expr (def);
       remove_phi_node (&psi, false);
 
-      /* Propagate constants immediately.  */
+      /* Propagate constants immediately, but leave an unused initialization
+	 around to avoid invalidating the SCEV cache.  */
       if (CONSTANT_CLASS_P (def))
-	{
-	  replace_uses_by (rslt, def);
-	  release_ssa_name (rslt);
-	  continue;
-	}
+	replace_uses_by (rslt, def);
 
       /* Create the replacement statements.  */
       gimple_seq stmts;
  
Patrick O'Neill Dec. 5, 2023, 7:29 a.m. UTC | #2
Relevant bugzilla:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112848

Thanks,
Patrick

On Mon, Dec 4, 2023 at 11:20 PM Li, Pan2 <pan2.li@intel.com> wrote:

> Hi Richard,
>
> It looks like this patch result in one ICE for RISC-V backend for case
> tree-ssa.exp=ssa-sink-16.c, could you please help to double check about it?
> Any more information required please feel free to let me know. Thanks.
>
> compiler error: Segmentation fault
> 0x1903067 crash_signal
>         ../../../../gcc/gcc/toplev.cc:316
> 0x111a24e loop_outer(loop const*)
>         ../../../../gcc/gcc/cfgloop.h:549
> 0x1ac2143 find_uses_to_rename_use
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:424
> 0x1ac2295 find_uses_to_rename_stmt
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:464
> 0x1ac2456 find_uses_to_rename_bb
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:495
> 0x1ac2585 find_uses_to_rename
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:521
> 0x1ac267c rewrite_into_loop_closed_ssa_1
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:588
> 0x1ac2735 rewrite_into_loop_closed_ssa(bitmap_head*, unsigned int)
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:628
> 0x19682a3 repair_loop_structures
>         ../../../../gcc/gcc/tree-cfgcleanup.cc:1190
> 0x196831d cleanup_tree_cfg(unsigned int)
>         ../../../../gcc/gcc/tree-cfgcleanup.cc:1209
> 0x16e654b execute_function_todo
>         ../../../../gcc/gcc/passes.cc:2057
> 0x16e534d do_per_function
>         ../../../../gcc/gcc/passes.cc:1687
> 0x16e68b0 execute_todo
>         ../../../../gcc/gcc/passes.cc:2142
>
> Pan
>
> -----Original Message-----
> From: Richard Biener <rguenther@suse.de>
> Sent: Monday, December 4, 2023 7:54 PM
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH] tree-optimization/112827 - corrupt SCEV cache during SCCP
>
> The following avoids corrupting the SCEV cache by my last change
> to propagate constant final values immediately.  The easiest fix
> is to keep a dead initialization around.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>
>         PR tree-optimization/112827
>         * tree-scalar-evolution.cc (final_value_replacement_loop):
>         Do not release SSA name but keep a dead initialization around.
>
>         * gcc.dg/torture/pr112827-1.c: New testcase.
>         * gcc.dg/torture/pr112827-2.c: Likewise.
> ---
>  gcc/testsuite/gcc.dg/torture/pr112827-1.c | 14 ++++++++++++++
>  gcc/testsuite/gcc.dg/torture/pr112827-2.c | 18 ++++++++++++++++++
>  gcc/tree-scalar-evolution.cc              |  9 +++------
>  3 files changed, 35 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-2.c
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-1.c
> b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
> new file mode 100644
> index 00000000000..6838cbbe62f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +
> +int a, b, c, d, e;
> +int main() {
> +  for (; c; c++) {
> +    for (a = 0; a < 2; a++)
> +      ;
> +    for (; b; b++) {
> +      e = d;
> +      d = a;
> +    }
> +  }
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-2.c
> b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
> new file mode 100644
> index 00000000000..a7a2a70211b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +
> +short a, b[1], f;
> +char c, g;
> +int d, e;
> +int main() {
> +  for (; f; f++) {
> +    for (d = 0; d < 2; d++)
> +      ;
> +    if (a)
> +      for (g = 0; g < 2; g++)
> +        for (c = 0; c < 2; c += b[d+g])
> +          ;
> +    for (; e; e++)
> +      ;
> +  }
> +  return 0;
> +}
> diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> index 065bcd0743d..7556d89e9f8 100644
> --- a/gcc/tree-scalar-evolution.cc
> +++ b/gcc/tree-scalar-evolution.cc
> @@ -3847,13 +3847,10 @@ final_value_replacement_loop (class loop *loop)
>        def = unshare_expr (def);
>        remove_phi_node (&psi, false);
>
> -      /* Propagate constants immediately.  */
> +      /* Propagate constants immediately, but leave an unused
> initialization
> +        around to avoid invalidating the SCEV cache.  */
>        if (CONSTANT_CLASS_P (def))
> -       {
> -         replace_uses_by (rslt, def);
> -         release_ssa_name (rslt);
> -         continue;
> -       }
> +       replace_uses_by (rslt, def);
>
>        /* Create the replacement statements.  */
>        gimple_seq stmts;
> --
> 2.35.3
>
  
Richard Biener Dec. 5, 2023, 8:10 a.m. UTC | #3
On Tue, 5 Dec 2023, Li, Pan2 wrote:

> Hi Richard,
> 
> It looks like this patch result in one ICE for RISC-V backend for case tree-ssa.exp=ssa-sink-16.c, could you please help to double check about it?
> Any more information required please feel free to let me know. Thanks.

I have pushed a fix.

Richard.

> compiler error: Segmentation fault
> 0x1903067 crash_signal
>         ../../../../gcc/gcc/toplev.cc:316
> 0x111a24e loop_outer(loop const*)
>         ../../../../gcc/gcc/cfgloop.h:549
> 0x1ac2143 find_uses_to_rename_use
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:424
> 0x1ac2295 find_uses_to_rename_stmt
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:464
> 0x1ac2456 find_uses_to_rename_bb
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:495
> 0x1ac2585 find_uses_to_rename
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:521
> 0x1ac267c rewrite_into_loop_closed_ssa_1
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:588
> 0x1ac2735 rewrite_into_loop_closed_ssa(bitmap_head*, unsigned int)
>         ../../../../gcc/gcc/tree-ssa-loop-manip.cc:628
> 0x19682a3 repair_loop_structures
>         ../../../../gcc/gcc/tree-cfgcleanup.cc:1190
> 0x196831d cleanup_tree_cfg(unsigned int)
>         ../../../../gcc/gcc/tree-cfgcleanup.cc:1209
> 0x16e654b execute_function_todo
>         ../../../../gcc/gcc/passes.cc:2057
> 0x16e534d do_per_function
>         ../../../../gcc/gcc/passes.cc:1687
> 0x16e68b0 execute_todo
>         ../../../../gcc/gcc/passes.cc:2142
> 
> Pan
> 
> -----Original Message-----
> From: Richard Biener <rguenther@suse.de> 
> Sent: Monday, December 4, 2023 7:54 PM
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH] tree-optimization/112827 - corrupt SCEV cache during SCCP
> 
> The following avoids corrupting the SCEV cache by my last change
> to propagate constant final values immediately.  The easiest fix
> is to keep a dead initialization around.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> 
> 	PR tree-optimization/112827
> 	* tree-scalar-evolution.cc (final_value_replacement_loop):
> 	Do not release SSA name but keep a dead initialization around.
> 
> 	* gcc.dg/torture/pr112827-1.c: New testcase.
> 	* gcc.dg/torture/pr112827-2.c: Likewise.
> ---
>  gcc/testsuite/gcc.dg/torture/pr112827-1.c | 14 ++++++++++++++
>  gcc/testsuite/gcc.dg/torture/pr112827-2.c | 18 ++++++++++++++++++
>  gcc/tree-scalar-evolution.cc              |  9 +++------
>  3 files changed, 35 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr112827-2.c
> 
> diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-1.c b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
> new file mode 100644
> index 00000000000..6838cbbe62f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +
> +int a, b, c, d, e;
> +int main() {
> +  for (; c; c++) {
> +    for (a = 0; a < 2; a++)
> +      ;
> +    for (; b; b++) {
> +      e = d;
> +      d = a;
> +    }
> +  }
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-2.c b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
> new file mode 100644
> index 00000000000..a7a2a70211b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +
> +short a, b[1], f;
> +char c, g;
> +int d, e;
> +int main() {
> +  for (; f; f++) {
> +    for (d = 0; d < 2; d++)
> +      ;
> +    if (a)
> +      for (g = 0; g < 2; g++)
> +        for (c = 0; c < 2; c += b[d+g])
> +          ;
> +    for (; e; e++)
> +      ;
> +  }
> +  return 0;
> +}
> diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> index 065bcd0743d..7556d89e9f8 100644
> --- a/gcc/tree-scalar-evolution.cc
> +++ b/gcc/tree-scalar-evolution.cc
> @@ -3847,13 +3847,10 @@ final_value_replacement_loop (class loop *loop)
>        def = unshare_expr (def);
>        remove_phi_node (&psi, false);
>  
> -      /* Propagate constants immediately.  */
> +      /* Propagate constants immediately, but leave an unused initialization
> +	 around to avoid invalidating the SCEV cache.  */
>        if (CONSTANT_CLASS_P (def))
> -	{
> -	  replace_uses_by (rslt, def);
> -	  release_ssa_name (rslt);
> -	  continue;
> -	}
> +	replace_uses_by (rslt, def);
>  
>        /* Create the replacement statements.  */
>        gimple_seq stmts;
>
  

Patch

diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-1.c b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
new file mode 100644
index 00000000000..6838cbbe62f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr112827-1.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+
+int a, b, c, d, e;
+int main() {
+  for (; c; c++) {
+    for (a = 0; a < 2; a++)
+      ;
+    for (; b; b++) {
+      e = d;
+      d = a;
+    }
+  }
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/torture/pr112827-2.c b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
new file mode 100644
index 00000000000..a7a2a70211b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr112827-2.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+
+short a, b[1], f;
+char c, g;
+int d, e;
+int main() {
+  for (; f; f++) {
+    for (d = 0; d < 2; d++)
+      ;
+    if (a)
+      for (g = 0; g < 2; g++)
+        for (c = 0; c < 2; c += b[d+g])
+          ;
+    for (; e; e++)
+      ;
+  }
+  return 0;
+}
diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
index 065bcd0743d..7556d89e9f8 100644
--- a/gcc/tree-scalar-evolution.cc
+++ b/gcc/tree-scalar-evolution.cc
@@ -3847,13 +3847,10 @@  final_value_replacement_loop (class loop *loop)
       def = unshare_expr (def);
       remove_phi_node (&psi, false);
 
-      /* Propagate constants immediately.  */
+      /* Propagate constants immediately, but leave an unused initialization
+	 around to avoid invalidating the SCEV cache.  */
       if (CONSTANT_CLASS_P (def))
-	{
-	  replace_uses_by (rslt, def);
-	  release_ssa_name (rslt);
-	  continue;
-	}
+	replace_uses_by (rslt, def);
 
       /* Create the replacement statements.  */
       gimple_seq stmts;