Patchwork Fix debug stmt handling in -ftree-loop-distribution (PR debug/48159)

login
register
mail settings
Submitter Jakub Jelinek
Date May 11, 2011, 12:15 p.m.
Message ID <20110511121553.GD17079@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/95157/
State New
Headers show

Comments

Jakub Jelinek - May 11, 2011, 12:15 p.m.
Hi!

The first testcase below ICEs, because generate_loops_for_partition
removes phis and stmts in the get_loop_body_in_dom_order order of bbs
and within within the bbs starting with phi nodes and then from beginning to
end of the bb.

While looking at it, I've noticed that stmts_from_loop and all the analysis
happily considers debug stmts as any other stmts, so I fear it is
problematic for -fcompare-debug, furthermore debug stmts should be just
always kept and only reset when needed.

The following patch implements that, debug stmts are ignored by the
analysis, kept around unless the whole loop is transformed into memset
and reset if they are using SSA_NAMES defined in the loop that are going to
be removed (well, moved to another loop or done using memset).

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

2011-05-11  Jakub Jelinek  <jakub@redhat.com>

	PR debug/48159
	* tree-data-ref.c (stmts_from_loop): Ignore debug stmts.
	* tree-loop-distribution.c (reset_debug_uses): New function.
	(generate_loops_for_partition): Call reset_debug_uses on the stmts
	that will be removed.  Keep around all debug stmts, don't count them
	as bits in partition bitmap.
	(generate_builtin): Don't count debug stmts or labels as bits in
	partition bitmap.

	* gcc.dg/pr48159-1.c: New test.
	* gcc.dg/pr48159-2.c: New test.


	Jakub
Richard Guenther - May 11, 2011, 12:18 p.m.
On Wed, May 11, 2011 at 2:15 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The first testcase below ICEs, because generate_loops_for_partition
> removes phis and stmts in the get_loop_body_in_dom_order order of bbs
> and within within the bbs starting with phi nodes and then from beginning to
> end of the bb.
>
> While looking at it, I've noticed that stmts_from_loop and all the analysis
> happily considers debug stmts as any other stmts, so I fear it is
> problematic for -fcompare-debug, furthermore debug stmts should be just
> always kept and only reset when needed.
>
> The following patch implements that, debug stmts are ignored by the
> analysis, kept around unless the whole loop is transformed into memset
> and reset if they are using SSA_NAMES defined in the loop that are going to
> be removed (well, moved to another loop or done using memset).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6?

Ok if you move reset_debug_uses to ... hmmm, say, tree-ssa.c near
the other debug related fns.

Thanks,
Richard.

> 2011-05-11  Jakub Jelinek  <jakub@redhat.com>
>
>        PR debug/48159
>        * tree-data-ref.c (stmts_from_loop): Ignore debug stmts.
>        * tree-loop-distribution.c (reset_debug_uses): New function.
>        (generate_loops_for_partition): Call reset_debug_uses on the stmts
>        that will be removed.  Keep around all debug stmts, don't count them
>        as bits in partition bitmap.
>        (generate_builtin): Don't count debug stmts or labels as bits in
>        partition bitmap.
>
>        * gcc.dg/pr48159-1.c: New test.
>        * gcc.dg/pr48159-2.c: New test.
>
> --- gcc/tree-data-ref.c.jj      2011-05-02 18:39:28.000000000 +0200
> +++ gcc/tree-data-ref.c 2011-05-11 10:08:15.000000000 +0200
> @@ -5017,7 +5017,7 @@ stmts_from_loop (struct loop *loop, VEC
>       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
>        {
>          stmt = gsi_stmt (bsi);
> -         if (gimple_code (stmt) != GIMPLE_LABEL)
> +         if (gimple_code (stmt) != GIMPLE_LABEL && !is_gimple_debug (stmt))
>            VEC_safe_push (gimple, heap, *stmts, stmt);
>        }
>     }
> --- gcc/tree-loop-distribution.c.jj     2011-01-03 09:54:28.000000000 +0100
> +++ gcc/tree-loop-distribution.c        2011-05-11 10:36:27.000000000 +0200
> @@ -152,6 +152,34 @@ create_bb_after_loop (struct loop *loop)
>   split_edge (exit);
>  }
>
> +/* Reset all debug stmts that use SSA_NAME(s) defined in STMT.  */
> +
> +static void
> +reset_debug_uses (gimple stmt)
> +{
> +  ssa_op_iter op_iter;
> +  def_operand_p def_p;
> +  imm_use_iterator imm_iter;
> +  gimple use_stmt;
> +
> +  FOR_EACH_PHI_OR_STMT_DEF (def_p, stmt, op_iter, SSA_OP_DEF)
> +    {
> +      tree var = DEF_FROM_PTR (def_p);
> +
> +      if (TREE_CODE (var) != SSA_NAME)
> +       continue;
> +
> +      FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, var)
> +       {
> +         if (!gimple_debug_bind_p (use_stmt))
> +           continue;
> +
> +         gimple_debug_bind_reset_value (use_stmt);
> +         update_stmt (use_stmt);
> +       }
> +    }
> +}
> +
>  /* Generate code for PARTITION from the code in LOOP.  The loop is
>    copied when COPY_P is true.  All the statements not flagged in the
>    PARTITION bitmap are removed from the loop or from its copy.  The
> @@ -181,6 +209,25 @@ generate_loops_for_partition (struct loo
>      stmts_from_loop.  */
>   bbs = get_loop_body_in_dom_order (loop);
>
> +  if (MAY_HAVE_DEBUG_STMTS)
> +    for (x = 0, i = 0; i < loop->num_nodes; i++)
> +      {
> +       basic_block bb = bbs[i];
> +
> +       for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi))
> +         if (!bitmap_bit_p (partition, x++))
> +           reset_debug_uses (gsi_stmt (bsi));
> +
> +       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
> +         {
> +           gimple stmt = gsi_stmt (bsi);
> +           if (gimple_code (stmt) != GIMPLE_LABEL
> +               && !is_gimple_debug (stmt)
> +               && !bitmap_bit_p (partition, x++))
> +             reset_debug_uses (stmt);
> +         }
> +      }
> +
>   for (x = 0, i = 0; i < loop->num_nodes; i++)
>     {
>       basic_block bb = bbs[i];
> @@ -199,7 +246,8 @@ generate_loops_for_partition (struct loo
>       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi);)
>        {
>          gimple stmt = gsi_stmt (bsi);
> -         if (gimple_code (gsi_stmt (bsi)) != GIMPLE_LABEL
> +         if (gimple_code (stmt) != GIMPLE_LABEL
> +             && !is_gimple_debug (stmt)
>              && !bitmap_bit_p (partition, x++))
>            {
>              unlink_stmt_vdef (stmt);
> @@ -312,7 +360,9 @@ generate_builtin (struct loop *loop, bit
>        {
>          gimple stmt = gsi_stmt (bsi);
>
> -         if (bitmap_bit_p (partition, x++)
> +         if (gimple_code (stmt) != GIMPLE_LABEL
> +             && !is_gimple_debug (stmt)
> +             && bitmap_bit_p (partition, x++)
>              && is_gimple_assign (stmt)
>              && !is_gimple_reg (gimple_assign_lhs (stmt)))
>            {
> --- gcc/testsuite/gcc.dg/pr48159-1.c.jj 2011-05-11 10:38:03.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr48159-1.c    2011-05-11 10:37:34.000000000 +0200
> @@ -0,0 +1,10 @@
> +/* PR debug/48159 */
> +/* { dg-do compile } */
> +/* { dg-options "-O3 -fcompare-debug" } */
> +
> +void
> +foo (double x, int y, double *__restrict z, double *__restrict w)
> +{
> +  while (y--)
> +    *z++ = (*w++ = 0) * x;
> +}
> --- gcc/testsuite/gcc.dg/pr48159-2.c.jj 2011-05-11 11:22:54.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr48159-2.c    2011-05-11 11:21:42.000000000 +0200
> @@ -0,0 +1,22 @@
> +/* PR debug/48159 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-loop-distribution -fcompare-debug" } */
> +
> +int foo (int * __restrict__ ia, int * __restrict__ ib,
> +        int * __restrict__ oxa, int * __restrict__ oxb)
> +{
> +  int i;
> +  int oya[52], oyb[52];
> +  for (i = 0; i < 52; i++)
> +    {
> +      int w1 = ia[i];
> +      int w2 = oxa[i];
> +      int w3 = ib[i];
> +      int w4 = oxb[i];
> +      int w5 = w1 + w2 + 5;
> +      oya[i] = (w1 * w2) >> 10;
> +      int w6 = w3 + w4 + 6;
> +      oyb[i] = (w3 * w4) >> 10;
> +    }
> +  return oya[22] + oyb[21];
> +}
>
>        Jakub
>

Patch

--- gcc/tree-data-ref.c.jj	2011-05-02 18:39:28.000000000 +0200
+++ gcc/tree-data-ref.c	2011-05-11 10:08:15.000000000 +0200
@@ -5017,7 +5017,7 @@  stmts_from_loop (struct loop *loop, VEC 
       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
 	{
 	  stmt = gsi_stmt (bsi);
-	  if (gimple_code (stmt) != GIMPLE_LABEL)
+	  if (gimple_code (stmt) != GIMPLE_LABEL && !is_gimple_debug (stmt))
 	    VEC_safe_push (gimple, heap, *stmts, stmt);
 	}
     }
--- gcc/tree-loop-distribution.c.jj	2011-01-03 09:54:28.000000000 +0100
+++ gcc/tree-loop-distribution.c	2011-05-11 10:36:27.000000000 +0200
@@ -152,6 +152,34 @@  create_bb_after_loop (struct loop *loop)
   split_edge (exit);
 }
 
+/* Reset all debug stmts that use SSA_NAME(s) defined in STMT.  */
+
+static void
+reset_debug_uses (gimple stmt)
+{
+  ssa_op_iter op_iter;
+  def_operand_p def_p;
+  imm_use_iterator imm_iter;
+  gimple use_stmt;
+
+  FOR_EACH_PHI_OR_STMT_DEF (def_p, stmt, op_iter, SSA_OP_DEF)
+    {
+      tree var = DEF_FROM_PTR (def_p);
+
+      if (TREE_CODE (var) != SSA_NAME)
+	continue;
+
+      FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, var)
+	{
+	  if (!gimple_debug_bind_p (use_stmt))
+	    continue;
+
+	  gimple_debug_bind_reset_value (use_stmt);
+	  update_stmt (use_stmt);
+	}
+    }
+}
+
 /* Generate code for PARTITION from the code in LOOP.  The loop is
    copied when COPY_P is true.  All the statements not flagged in the
    PARTITION bitmap are removed from the loop or from its copy.  The
@@ -181,6 +209,25 @@  generate_loops_for_partition (struct loo
      stmts_from_loop.  */
   bbs = get_loop_body_in_dom_order (loop);
 
+  if (MAY_HAVE_DEBUG_STMTS)
+    for (x = 0, i = 0; i < loop->num_nodes; i++)
+      {
+	basic_block bb = bbs[i];
+
+	for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi))
+	  if (!bitmap_bit_p (partition, x++))
+	    reset_debug_uses (gsi_stmt (bsi));
+
+	for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
+	  {
+	    gimple stmt = gsi_stmt (bsi);
+	    if (gimple_code (stmt) != GIMPLE_LABEL
+		&& !is_gimple_debug (stmt)
+		&& !bitmap_bit_p (partition, x++))
+	      reset_debug_uses (stmt);
+	  }
+      }
+
   for (x = 0, i = 0; i < loop->num_nodes; i++)
     {
       basic_block bb = bbs[i];
@@ -199,7 +246,8 @@  generate_loops_for_partition (struct loo
       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi);)
 	{
 	  gimple stmt = gsi_stmt (bsi);
-	  if (gimple_code (gsi_stmt (bsi)) != GIMPLE_LABEL
+	  if (gimple_code (stmt) != GIMPLE_LABEL
+	      && !is_gimple_debug (stmt)
 	      && !bitmap_bit_p (partition, x++))
 	    {
 	      unlink_stmt_vdef (stmt);
@@ -312,7 +360,9 @@  generate_builtin (struct loop *loop, bit
 	{
 	  gimple stmt = gsi_stmt (bsi);
 
-	  if (bitmap_bit_p (partition, x++)
+	  if (gimple_code (stmt) != GIMPLE_LABEL
+	      && !is_gimple_debug (stmt)
+	      && bitmap_bit_p (partition, x++)
 	      && is_gimple_assign (stmt)
 	      && !is_gimple_reg (gimple_assign_lhs (stmt)))
 	    {
--- gcc/testsuite/gcc.dg/pr48159-1.c.jj	2011-05-11 10:38:03.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr48159-1.c	2011-05-11 10:37:34.000000000 +0200
@@ -0,0 +1,10 @@ 
+/* PR debug/48159 */
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcompare-debug" } */
+
+void
+foo (double x, int y, double *__restrict z, double *__restrict w)
+{
+  while (y--)
+    *z++ = (*w++ = 0) * x;
+}
--- gcc/testsuite/gcc.dg/pr48159-2.c.jj	2011-05-11 11:22:54.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr48159-2.c	2011-05-11 11:21:42.000000000 +0200
@@ -0,0 +1,22 @@ 
+/* PR debug/48159 */
+/* { dg-do compile } */ 
+/* { dg-options "-O2 -ftree-loop-distribution -fcompare-debug" } */
+
+int foo (int * __restrict__ ia, int * __restrict__ ib,
+	 int * __restrict__ oxa, int * __restrict__ oxb)
+{
+  int i;
+  int oya[52], oyb[52];
+  for (i = 0; i < 52; i++)
+    {
+      int w1 = ia[i];
+      int w2 = oxa[i];
+      int w3 = ib[i];
+      int w4 = oxb[i];
+      int w5 = w1 + w2 + 5;
+      oya[i] = (w1 * w2) >> 10;
+      int w6 = w3 + w4 + 6;
+      oyb[i] = (w3 * w4) >> 10;
+    }
+  return oya[22] + oyb[21];
+}