public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix tree-into-ssa.c for debug stmts (PR debug/49602)
@ 2011-07-01 21:22 Jakub Jelinek
  2011-07-02 12:20 ` Richard Guenther
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2011-07-01 21:22 UTC (permalink / raw)
  To: gcc-patches

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.

--- 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix tree-into-ssa.c for debug stmts (PR debug/49602)
  2011-07-01 21:22 [PATCH] Fix tree-into-ssa.c for debug stmts (PR debug/49602) Jakub Jelinek
@ 2011-07-02 12:20 ` Richard Guenther
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Guenther @ 2011-07-02 12:20 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-07-02 12:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-01 21:22 [PATCH] Fix tree-into-ssa.c for debug stmts (PR debug/49602) Jakub Jelinek
2011-07-02 12:20 ` Richard Guenther

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).