Re: [vta, graphite?] propagate degenerate phi nodes into debug stmts

[Alexandre Oliva <aoliva@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

On Nov  8, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:

> For the rest it would be better to re-organize the code as

>    else if (gimple_code (def_stmt) == PHI_NODE)
>      {
>         value = degenerate_phi_result (def_stmt);
>         if (value
>             && walk_tree (&value, find_released_ssa_name, NULL, NULL))
>           value = NULL_TREE;
>      }

> that looks simpler and it avoids walking PHI args uselessly.

That doesn't work.  We crash deep within degenerate_phi_result given
expressions containing released SSA names.  It's the same reason why we
have to test for released SSA names before calling
gimple_assign_rhs_to_tree: IIRC it has to do with testing whether the
already-NULL type of the SSA name is a pointer type.

Regardless, I reorganized the code so as to not have to test for
PHI_NODEs as often.  There were two unrelated code paths intermixed with
tests every now and again.  Now they're two separate code flows.

> @@ -479,6 +508,13 @@ insert_debug_temps_for_defs (gimple_stmt

>    stmt = gsi_stmt (*gsi);

> +  if (gimple_code (stmt) == GIMPLE_PHI)
> +    {
> +      tree var = gimple_phi_result (stmt);
> +      insert_debug_temp_for_var_def (gsi, var);
> +      return;
> +    }
> +

> This looks odd.  SSA DEF operand iteration should walk the PHI defs
> as well, so the change should not be necessary.

I thought so, too, but by the time we get there, the operands of the PHI
stmt have already been disconnected.

Here's what I'm going to test now.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-ssa-dom-prop-degenerate.patch --]
[-- Type: text/x-diff, Size: 2580 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* tree-ssa.c (find_released_ssa_name): Handle NULL wi.
	(insert_debug_temp_for_var_def): Handle degenerate PHI nodes.
	(insert_debug_temps_for_defs): Handle PHI nodes.

Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c.orig	2009-11-16 18:24:07.000000000 -0200
+++ gcc/tree-ssa.c	2009-11-16 18:41:04.000000000 -0200
@@ -279,7 +279,7 @@ find_released_ssa_name (tree *tp, int *w
 {
   struct walk_stmt_info *wi = (struct walk_stmt_info *) data_;
 
-  if (wi->is_lhs)
+  if (wi && wi->is_lhs)
     return NULL_TREE;
 
   if (TREE_CODE (*tp) == SSA_NAME)
@@ -346,7 +346,33 @@ insert_debug_temp_for_var_def (gimple_st
   /* If we didn't get an insertion point, and the stmt has already
      been removed, we won't be able to insert the debug bind stmt, so
      we'll have to drop debug information.  */
-  if (is_gimple_assign (def_stmt))
+  if (gimple_code (def_stmt) == GIMPLE_PHI)
+    {
+      bool no_value = false;
+      struct walk_stmt_info wi;
+      size_t i;
+
+      memset (&wi, 0, sizeof (wi));
+
+      for (i = 0; i < gimple_phi_num_args (def_stmt); i++)
+	{
+	  tree *argp = gimple_phi_arg_def_ptr (def_stmt, i);
+
+	  if (!*argp
+	      || walk_tree (argp, find_released_ssa_name, NULL, NULL))
+	    {
+	      no_value = true;
+	      break;
+	    }
+	}
+
+      if (!no_value)
+	/* ??? We could handle at least some non-generate PHI
+	   nodes by inserting debug temps on every edge.  It's
+	   not clear how much this would improve debug info.  */
+	value = degenerate_phi_result (def_stmt);
+    }
+  else if (is_gimple_assign (def_stmt))
     {
       bool no_value = false;
 
@@ -408,6 +434,7 @@ insert_debug_temp_for_var_def (gimple_st
 	 at the expense of duplication of expressions.  */
 
       if (CONSTANT_CLASS_P (value)
+	  || gimple_code (def_stmt) == GIMPLE_PHI
 	  || (usecount == 1
 	      && (!gimple_assign_single_p (def_stmt)
 		  || is_gimple_min_invariant (value)))
@@ -478,6 +505,16 @@ insert_debug_temps_for_defs (gimple_stmt
 
   stmt = gsi_stmt (*gsi);
 
+  /* By the time we get here, the DEF in the PHI node seems to have
+     already been disconnected as an operand, so the FOR_EACH below
+     has no effect.  Handle it explicitly.  */
+  if (gimple_code (stmt) == GIMPLE_PHI)
+    {
+      tree var = gimple_phi_result (stmt);
+      insert_debug_temp_for_var_def (gsi, var);
+      return;
+    }
+
   FOR_EACH_SSA_DEF_OPERAND (def_p, stmt, op_iter, SSA_OP_DEF)
     {
       tree var = DEF_FROM_PTR (def_p);

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer