diff mbox series

tree-ssa-math-opts: Fix up gsi_remove order in match_uaddc_usubc [PR112430]

Message ID ZU864Q9ltkW8n94Z@tucnak
State Unresolved
Headers
Series tree-ssa-math-opts: Fix up gsi_remove order in match_uaddc_usubc [PR112430] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Nov. 11, 2023, 8:27 a.m. UTC 
  Hi!

The following testcase ICEs, because the temp_stmts were removed in
wrong order, from the ones appearing earlier in the IL to the later ones,
so insert_debug_temps_for_defs can reintroduce dead SSA_NAMEs back into the
IL.

The following patch fixes that by removing them in the order they were
pushed into the vector, which is from later ones to earlier ones.
Additionally, I've noticed I forgot to call release_defs on the removed
stmts.

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

2023-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/112430
	* tree-ssa-math-opts.cc (match_uaddc_usubc): Remove temp_stmts in the
	order they were pushed rather than in reverse order.  Call
	release_defs after gsi_remove.

	* gcc.dg/pr112430.c: New test.


	Jakub

Comments

Richard Biener Nov. 11, 2023, 1:16 p.m. UTC | #1 
> Am 11.11.2023 um 09:27 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> The following testcase ICEs, because the temp_stmts were removed in
> wrong order, from the ones appearing earlier in the IL to the later ones,
> so insert_debug_temps_for_defs can reintroduce dead SSA_NAMEs back into the
> IL.
> 
> The following patch fixes that by removing them in the order they were
> pushed into the vector, which is from later ones to earlier ones.
> Additionally, I've noticed I forgot to call release_defs on the removed
> stmts.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Richard 

> 2023-11-11  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR middle-end/112430
>    * tree-ssa-math-opts.cc (match_uaddc_usubc): Remove temp_stmts in the
>    order they were pushed rather than in reverse order.  Call
>    release_defs after gsi_remove.
> 
>    * gcc.dg/pr112430.c: New test.
> 
> --- gcc/tree-ssa-math-opts.cc.jj    2023-11-02 07:49:20.699815089 +0100
> +++ gcc/tree-ssa-math-opts.cc    2023-11-10 13:43:13.059912310 +0100
> @@ -5047,11 +5047,11 @@ match_uaddc_usubc (gimple_stmt_iterator
>       gsi_insert_before (gsi, g, GSI_SAME_STMT);
>       /* Remove some further statements which can't be kept in the IL because
>     they can use SSA_NAMEs whose setter is going to be removed too.  */
> -      while (temp_stmts.length ())
> +      for (gimple *g2 : temp_stmts)
>    {
> -      g = temp_stmts.pop ();
> -      gsi2 = gsi_for_stmt (g);
> +      gsi2 = gsi_for_stmt (g2);
>      gsi_remove (&gsi2, true);
> +      release_defs (g2);
>    }
>     }
>   else
> @@ -5068,10 +5068,12 @@ match_uaddc_usubc (gimple_stmt_iterator
>    rhs1 = gimple_assign_rhs1 (g);
>    gsi2 = gsi_for_stmt (g);
>    gsi_remove (&gsi2, true);
> +    release_defs (g);
>       }
>   gcc_checking_assert (rhs1 == gimple_assign_lhs (im2));
>   gsi2 = gsi_for_stmt (im2);
>   gsi_remove (&gsi2, true);
> +  release_defs (im2);
>   /* Replace the re2 statement with __real__ of the newly added
>      .UADDC/.USUBC call.  */
>   if (re2)
> --- gcc/testsuite/gcc.dg/pr112430.c.jj    2023-11-10 13:45:18.707163791 +0100
> +++ gcc/testsuite/gcc.dg/pr112430.c    2023-11-10 13:45:04.523361174 +0100
> @@ -0,0 +1,30 @@
> +/* PR middle-end/112430 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +int a, b, c, d, e;
> +unsigned int f;
> +
> +static void
> +foo (unsigned int x)
> +{
> +  unsigned int g = x < c;
> +  int h = f < b;
> +  x += h;
> +  g += x < h;
> +  f = x;
> +  x = g;
> +  g = f += a;
> +  h = f < a;
> +  x += h;
> +  c += f < d;
> +  x += c;
> +  g += x < c;
> +  e = g;
> +}
> +
> +void
> +bar (unsigned int x)
> +{
> +  foo (x);
> +}
> 
>    Jakub
>
diff mbox series

Patch

--- gcc/tree-ssa-math-opts.cc.jj	2023-11-02 07:49:20.699815089 +0100
+++ gcc/tree-ssa-math-opts.cc	2023-11-10 13:43:13.059912310 +0100
@@ -5047,11 +5047,11 @@  match_uaddc_usubc (gimple_stmt_iterator
       gsi_insert_before (gsi, g, GSI_SAME_STMT);
       /* Remove some further statements which can't be kept in the IL because
 	 they can use SSA_NAMEs whose setter is going to be removed too.  */
-      while (temp_stmts.length ())
+      for (gimple *g2 : temp_stmts)
 	{
-	  g = temp_stmts.pop ();
-	  gsi2 = gsi_for_stmt (g);
+	  gsi2 = gsi_for_stmt (g2);
 	  gsi_remove (&gsi2, true);
+	  release_defs (g2);
 	}
     }
   else
@@ -5068,10 +5068,12 @@  match_uaddc_usubc (gimple_stmt_iterator
 	rhs1 = gimple_assign_rhs1 (g);
 	gsi2 = gsi_for_stmt (g);
 	gsi_remove (&gsi2, true);
+	release_defs (g);
       }
   gcc_checking_assert (rhs1 == gimple_assign_lhs (im2));
   gsi2 = gsi_for_stmt (im2);
   gsi_remove (&gsi2, true);
+  release_defs (im2);
   /* Replace the re2 statement with __real__ of the newly added
      .UADDC/.USUBC call.  */
   if (re2)
--- gcc/testsuite/gcc.dg/pr112430.c.jj	2023-11-10 13:45:18.707163791 +0100
+++ gcc/testsuite/gcc.dg/pr112430.c	2023-11-10 13:45:04.523361174 +0100
@@ -0,0 +1,30 @@ 
+/* PR middle-end/112430 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+int a, b, c, d, e;
+unsigned int f;
+
+static void
+foo (unsigned int x)
+{
+  unsigned int g = x < c;
+  int h = f < b;
+  x += h;
+  g += x < h;
+  f = x;
+  x = g;
+  g = f += a;
+  h = f < a;
+  x += h;
+  c += f < d;
+  x += c;
+  g += x < c;
+  e = g;
+}
+
+void
+bar (unsigned int x)
+{
+  foo (x);
+}