Patchwork Fix tree-into-ssa.c for debug stmts (PR debug/49602)

login
register
mail settings
Submitter Jakub Jelinek
Date July 1, 2011, 9:21 p.m.
Message ID <20110701212148.GK16443@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/102971/
State New
Headers show

Comments

Jakub Jelinek - July 1, 2011, 9:21 p.m.
Hi!

For debug stmt uses, we don't want any PHI nodes to be created
just because of them, so the debug uses need to be ignored
during decisions which PHI nodes to add.
Unfortunately that means get_current_def can't be always trusted.
The following patch trusts it in a few cases where I'm reasonably
sure it is trustable.  Perhaps some more cases could be added in
the future if anyone has ideas...

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

2011-07-01  Jakub Jelinek  <jakub@redhat.com>

	PR debug/49602
	* tree-into-ssa.c (rewrite_debug_stmt_uses): Disregard
	get_current_def return value if it can't be trusted to be
	the current value of the variable in the current bb.

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


	Jakub
Richard Guenther - July 2, 2011, 12:20 p.m.
On Fri, Jul 1, 2011 at 11:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> For debug stmt uses, we don't want any PHI nodes to be created
> just because of them, so the debug uses need to be ignored
> during decisions which PHI nodes to add.
> Unfortunately that means get_current_def can't be always trusted.
> The following patch trusts it in a few cases where I'm reasonably
> sure it is trustable.  Perhaps some more cases could be added in
> the future if anyone has ideas...
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-07-01  Jakub Jelinek  <jakub@redhat.com>
>
>        PR debug/49602
>        * tree-into-ssa.c (rewrite_debug_stmt_uses): Disregard
>        get_current_def return value if it can't be trusted to be
>        the current value of the variable in the current bb.
>
>        * gcc.dg/pr49602.c: New test.
>
> --- gcc/tree-into-ssa.c.jj      2011-06-23 10:13:58.000000000 +0200
> +++ gcc/tree-into-ssa.c 2011-07-01 20:39:40.000000000 +0200
> @@ -1343,7 +1343,41 @@ rewrite_debug_stmt_uses (gimple stmt)
>            }
>        }
>       else
> -       def = get_current_def (var);
> +       {
> +         def = get_current_def (var);
> +         /* Check if get_current_def can be trusted.  */
> +         if (def)
> +           {
> +             basic_block bb = gimple_bb (stmt);
> +             basic_block def_bb
> +               = SSA_NAME_IS_DEFAULT_DEF (def)
> +                 ? NULL : gimple_bb (SSA_NAME_DEF_STMT (def));
> +
> +             /* If definition is in current bb, it is fine.  */
> +             if (bb == def_bb)
> +               ;
> +             /* If definition bb doesn't dominate the current bb,
> +                it can't be used.  */
> +             else if (def_bb && !dominated_by_p (CDI_DOMINATORS, bb, def_bb))
> +               def = NULL;
> +             /* If there is just one definition and dominates the current
> +                bb, it is fine.  */
> +             else if (get_phi_state (var) == NEED_PHI_STATE_NO)
> +               ;
> +             else
> +               {
> +                 struct def_blocks_d *db_p = get_def_blocks_for (var);
> +
> +                 /* If there are some non-debug uses in the current bb,
> +                    it is fine.  */
> +                 if (bitmap_bit_p (db_p->livein_blocks, bb->index))
> +                   ;
> +                 /* Otherwise give up for now.  */
> +                 else
> +                   def = NULL;
> +               }
> +           }
> +       }
>       if (def == NULL)
>        {
>          gimple_debug_bind_reset_value (stmt);
> --- gcc/testsuite/gcc.dg/pr49602.c.jj   2011-07-01 20:42:44.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr49602.c      2011-07-01 18:49:04.000000000 +0200
> @@ -0,0 +1,17 @@
> +/* PR debug/49602 */
> +/* { dg-do compile } */
> +/* { dg-options "-g -O2" } */
> +
> +static void
> +foo (int *x)
> +{
> +}
> +
> +void
> +bar (int *x)
> +{
> +  int i;
> +  for (i = 0; i == 1; ++i)
> +    x = 0;
> +  foo (x);
> +}
>
>        Jakub
>

Patch

--- gcc/tree-into-ssa.c.jj	2011-06-23 10:13:58.000000000 +0200
+++ gcc/tree-into-ssa.c	2011-07-01 20:39:40.000000000 +0200
@@ -1343,7 +1343,41 @@  rewrite_debug_stmt_uses (gimple stmt)
 	    }
 	}
       else
-	def = get_current_def (var);
+	{
+	  def = get_current_def (var);
+	  /* Check if get_current_def can be trusted.  */
+	  if (def)
+	    {
+	      basic_block bb = gimple_bb (stmt);
+	      basic_block def_bb
+		= SSA_NAME_IS_DEFAULT_DEF (def)
+		  ? NULL : gimple_bb (SSA_NAME_DEF_STMT (def));
+
+	      /* If definition is in current bb, it is fine.  */
+	      if (bb == def_bb)
+		;
+	      /* If definition bb doesn't dominate the current bb,
+		 it can't be used.  */
+	      else if (def_bb && !dominated_by_p (CDI_DOMINATORS, bb, def_bb))
+		def = NULL;
+	      /* If there is just one definition and dominates the current
+		 bb, it is fine.  */
+	      else if (get_phi_state (var) == NEED_PHI_STATE_NO)
+		;
+	      else
+		{
+		  struct def_blocks_d *db_p = get_def_blocks_for (var);
+
+		  /* If there are some non-debug uses in the current bb,
+		     it is fine.  */
+		  if (bitmap_bit_p (db_p->livein_blocks, bb->index))
+		    ;
+		  /* Otherwise give up for now.  */
+		  else
+		    def = NULL;
+		}
+	    }
+	}
       if (def == NULL)
 	{
 	  gimple_debug_bind_reset_value (stmt);
--- gcc/testsuite/gcc.dg/pr49602.c.jj	2011-07-01 20:42:44.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr49602.c	2011-07-01 18:49:04.000000000 +0200
@@ -0,0 +1,17 @@ 
+/* PR debug/49602 */
+/* { dg-do compile } */
+/* { dg-options "-g -O2" } */
+
+static void
+foo (int *x)
+{
+}
+
+void
+bar (int *x)
+{
+  int i;
+  for (i = 0; i == 1; ++i)
+    x = 0;
+  foo (x);
+}