backprop: Fix deleting of a phi node [PR116922]

Message ID 20241002031236.1448655-1-quic_apinski@quicinc.com
State Committed
Commit cea87c84eacdb422caeada734ba5138c994d7022
Headers
Series backprop: Fix deleting of a phi node [PR116922] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-arm warning Patch is already merged

Commit Message

Andrew Pinski Oct. 2, 2024, 3:12 a.m. UTC
  The problem here is remove_unused_var is called on a name that is
defined by a phi node but it deletes it like removing a normal statement.
remove_phi_node should be called rather than gsi_remove for phinodes.

Note there is a possibility of using simple_dce_from_worklist instead
but that is for another day.

Bootstrapped and tested on x86_64-linux-gnu.

	PR tree-optimization/116922

gcc/ChangeLog:

	* gimple-ssa-backprop.cc (remove_unused_var): Handle phi
	nodes correctly.

gcc/testsuite/ChangeLog:

	* gcc.dg/torture/pr116922.c: New test.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/gimple-ssa-backprop.cc              | 10 ++++++++--
 gcc/testsuite/gcc.dg/torture/pr116922.c | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr116922.c
  

Comments

Richard Biener Oct. 2, 2024, 7:34 a.m. UTC | #1
On Wed, Oct 2, 2024 at 5:13 AM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> The problem here is remove_unused_var is called on a name that is
> defined by a phi node but it deletes it like removing a normal statement.
> remove_phi_node should be called rather than gsi_remove for phinodes.
>
> Note there is a possibility of using simple_dce_from_worklist instead
> but that is for another day.
>
> Bootstrapped and tested on x86_64-linux-gnu.

OK (not that I like the two conflicting APIs, maybe we can work towards removing
remove_phi_node and make PHI node handling in gsi_* better)

Thanks,
Richard.

>         PR tree-optimization/116922
>
> gcc/ChangeLog:
>
>         * gimple-ssa-backprop.cc (remove_unused_var): Handle phi
>         nodes correctly.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/torture/pr116922.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/gimple-ssa-backprop.cc              | 10 ++++++++--
>  gcc/testsuite/gcc.dg/torture/pr116922.c | 19 +++++++++++++++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr116922.c
>
> diff --git a/gcc/gimple-ssa-backprop.cc b/gcc/gimple-ssa-backprop.cc
> index fe27ef51cdf..e3374b18138 100644
> --- a/gcc/gimple-ssa-backprop.cc
> +++ b/gcc/gimple-ssa-backprop.cc
> @@ -663,8 +663,14 @@ remove_unused_var (tree var)
>        print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
>      }
>    gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> -  gsi_remove (&gsi, true);
> -  release_defs (stmt);
> +  if (gimple_code (stmt) == GIMPLE_PHI)
> +    remove_phi_node (&gsi, true);
> +  else
> +    {
> +      unlink_stmt_vdef (stmt);
> +      gsi_remove (&gsi, true);
> +      release_defs (stmt);
> +    }
>  }
>
>  /* Note that we're replacing OLD_RHS with NEW_RHS in STMT.  */
> diff --git a/gcc/testsuite/gcc.dg/torture/pr116922.c b/gcc/testsuite/gcc.dg/torture/pr116922.c
> new file mode 100644
> index 00000000000..0fcf912930f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr116922.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-ffast-math" } */
> +/* PR tree-optimization/116922 */
> +
> +
> +static int g;
> +
> +void
> +foo (int c, double v, double *r)
> +{
> +b:
> +  do
> +    v /= g - v;
> +  while (c);
> +  *r = v;
> +
> +  double x;
> +  foo (5, (double)0, &x);
> +}
> --
> 2.43.0
>
  

Patch

diff --git a/gcc/gimple-ssa-backprop.cc b/gcc/gimple-ssa-backprop.cc
index fe27ef51cdf..e3374b18138 100644
--- a/gcc/gimple-ssa-backprop.cc
+++ b/gcc/gimple-ssa-backprop.cc
@@ -663,8 +663,14 @@  remove_unused_var (tree var)
       print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
     }
   gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
-  gsi_remove (&gsi, true);
-  release_defs (stmt);
+  if (gimple_code (stmt) == GIMPLE_PHI)
+    remove_phi_node (&gsi, true);
+  else
+    {
+      unlink_stmt_vdef (stmt);
+      gsi_remove (&gsi, true);
+      release_defs (stmt);
+    }
 }
 
 /* Note that we're replacing OLD_RHS with NEW_RHS in STMT.  */
diff --git a/gcc/testsuite/gcc.dg/torture/pr116922.c b/gcc/testsuite/gcc.dg/torture/pr116922.c
new file mode 100644
index 00000000000..0fcf912930f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr116922.c
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-ffast-math" } */
+/* PR tree-optimization/116922 */
+
+
+static int g;
+
+void
+foo (int c, double v, double *r)
+{
+b:
+  do
+    v /= g - v;
+  while (c);
+  *r = v;
+
+  double x;
+  foo (5, (double)0, &x);
+}