Fix thinko in insert_reciprocals (PR tree-optimization/86835)

Message ID 20180803155924.GR17988@tucnak
State New
Headers show
Series
  • Fix thinko in insert_reciprocals (PR tree-optimization/86835)
Related show

Commit Message

Jakub Jelinek Aug. 3, 2018, 3:59 p.m.
Hi!

As the comment say, we want to insert (if should_insert_square_recip
is true) new_square_stmt right after new_stmt.  The current code does
that correctly if new_stmt is inserted with
  gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
because the following
    gsi_insert_before (&gsi, new_square_stmt, GSI_SAME_STMT);
will keep inserting before whatever gsi points to.
But one of the 3 cases inserts new_stmt instead after def_gsi, and the
current
  gsi = *def_gsi;
  gsi_insert_after (def_gsi, new_stmt, GSI_NEW_STMT);
    gsi_insert_before (&gsi, new_square_stmt, GSI_SAME_STMT);
certainly doesn't do that, it inserts new_stmt after the def_gsi stmt and
then inserts new_square_stmt before the def_gsi stmt, so we have order of:
  new_square_stmt
  whatever-def_gsi-pointed-to-stmt
  new_stmt
where new_square_stmt uses the lhs of new_stmt.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

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

	PR tree-optimization/86835
	* tree-ssa-math-opts.c (insert_reciprocals): Even when inserting
	new_stmt after def_gsi, make sure to insert new_square_stmt after
	that stmt, not 2 stmts before it.

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


	Jakub

Comments

Jakub Jelinek Aug. 11, 2018, 11:06 a.m. | #1
On Fri, Aug 03, 2018 at 05:59:24PM +0200, Jakub Jelinek wrote:
> 2018-08-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/86835
> 	* tree-ssa-math-opts.c (insert_reciprocals): Even when inserting
> 	new_stmt after def_gsi, make sure to insert new_square_stmt after
> 	that stmt, not 2 stmts before it.
> 
> 	* gcc.dg/pr86835.c: New test.

I'd like to ping this patch.

Thanks.

	Jakub
Richard Biener Aug. 11, 2018, 11:11 a.m. | #2
On August 11, 2018 1:06:59 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>On Fri, Aug 03, 2018 at 05:59:24PM +0200, Jakub Jelinek wrote:
>> 2018-08-03  Jakub Jelinek  <jakub@redhat.com>
>> 
>> 	PR tree-optimization/86835
>> 	* tree-ssa-math-opts.c (insert_reciprocals): Even when inserting
>> 	new_stmt after def_gsi, make sure to insert new_square_stmt after
>> 	that stmt, not 2 stmts before it.
>> 
>> 	* gcc.dg/pr86835.c: New test.
>
>I'd like to ping this patch.
>

OK. 

Richard. 
>Thanks.
>
>	Jakub

Patch

--- gcc/tree-ssa-math-opts.c.jj	2018-07-12 21:31:27.698410833 +0200
+++ gcc/tree-ssa-math-opts.c	2018-08-03 12:58:27.475961037 +0200
@@ -422,6 +422,8 @@  insert_reciprocals (gimple_stmt_iterator
 	    gsi_next (&gsi);
 
 	  gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
+	  if (should_insert_square_recip)
+	    gsi_insert_before (&gsi, new_square_stmt, GSI_SAME_STMT);
 	}
       else if (def_gsi && occ->bb == def_gsi->bb)
 	{
@@ -429,21 +431,19 @@  insert_reciprocals (gimple_stmt_iterator
 	     never happen if the definition statement can throw, because in
 	     that case the sole successor of the statement's basic block will
 	     dominate all the uses as well.  */
-	  gsi = *def_gsi;
 	  gsi_insert_after (def_gsi, new_stmt, GSI_NEW_STMT);
+	  if (should_insert_square_recip)
+	    gsi_insert_after (def_gsi, new_square_stmt, GSI_NEW_STMT);
 	}
       else
 	{
 	  /* Case 3: insert in a basic block not containing defs/uses.  */
 	  gsi = gsi_after_labels (occ->bb);
 	  gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
+	  if (should_insert_square_recip)
+	    gsi_insert_before (&gsi, new_square_stmt, GSI_SAME_STMT);
 	}
 
-      /* Regardless of which case the reciprocal as inserted in,
-	 we insert the square immediately after the reciprocal.  */
-      if (should_insert_square_recip)
-	gsi_insert_before (&gsi, new_square_stmt, GSI_SAME_STMT);
-
       reciprocal_stats.rdivs_inserted++;
 
       occ->recip_def_stmt = new_stmt;
--- gcc/testsuite/gcc.dg/pr86835.c.jj	2018-08-03 13:07:28.834159126 +0200
+++ gcc/testsuite/gcc.dg/pr86835.c	2018-08-03 13:07:14.119126559 +0200
@@ -0,0 +1,29 @@ 
+/* PR tree-optimization/86835 */
+/* { dg-do run } */
+/* { dg-options "-O2 -ffast-math -Wuninitialized" } */
+
+__attribute__((noipa)) void
+foo (int n, double *x, double *y)
+{		/* { dg-bogus "is used uninitialized in this function" "" { target *-*-* } 0 } */
+  int i;
+  double b = y[4];
+  for (i = 0; i < n; ++i)
+    y[3] += __builtin_sin (x[i] / b);
+  y[0] /= b;
+  y[1] /= b * b;
+  y[2] /= b;
+}
+
+int
+main ()
+{
+  double y[] = { 16.0, 64.0, 128.0, 0.0, 2.0 };
+  foo (0, y, y);
+  if (__builtin_fabs (y[0] - 8.0) > 0.0001
+      || __builtin_fabs (y[1] - 16.0) > 0.0001
+      || __builtin_fabs (y[2] - 64.0) > 0.0001
+      || y[3] != 0.0
+      || y[4] != 2.0)
+    __builtin_abort ();
+  return 0;
+}