Re: coalesce SSA versions in var-tracking notes

[Alexandre Oliva <aoliva at redhat dot com>]

On Aug 11, 2006, Steve Ellcey <sje@cup.hp.com> wrote:

> I was wondering if you have looked at PR 28672?

I hadn't got to it yet, but last night I did.  Here's the patch I came
up with.  Bootstrapped on amd64-linux-gnu, verified that it fixes the
problem in H.J.'s testcase with a cross to ia64-linux-gnu, and the
patch even makes sense :-)

Ok to install?

> I am not sure why this is and am not very familiar with this part of
> GCC.

The problem was that clobber_variable_part was reusing logic that did
more than intended, killing any other variable parts that might be
live at the same register as any register that might hold that same
variable part.  So, in one round, we'd kill all such registers, which
then enabled the next round to make the variable parts available,
which the interacted again to kill them for the next round, and so
on.  Oops.

The solution was to do only what was intended, namely, mark that part
as no longer available anywhere, as opposed to pretending we're
modifying every location that might hold that variable part.  This
stabilizes the dataflow analysis.


I also enclose an improved version of the dataflow debugging patch I'd
posted before, that at least helped me realize the problem was not
what I'd thought at first :-) (namely, merging info from incoming
edges incorrectly, creating multiple entries for the same decl set)

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

	PR target/28672
	* var-tracking.c (dump_dataflow_set): Start dumping at
	register zero.
	(clobber_variable_part): Kill only the variable part in
	registers holding it, leaving other variables alone.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2006-08-15 07:06:38.000000000 -0300
+++ gcc/var-tracking.c	2006-08-15 07:21:01.000000000 -0300
@@ -1960,7 +1960,7 @@ dump_dataflow_set (dataflow_set *set)
 
   fprintf (dump_file, "Stack adjustment: " HOST_WIDE_INT_PRINT_DEC "\n",
 	   set->stack_adjust);
-  for (i = 1; i < FIRST_PSEUDO_REGISTER; i++)
+  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     {
       if (set->regs[i])
 	{
@@ -2212,8 +2212,32 @@ clobber_variable_part (dataflow_set *set
 	  for (node = next; node; node = next)
 	    {
 	      next = node->next;
-	      if (REG_P (node->loc) && node->loc != loc)
-		var_reg_delete (set, node->loc, false);
+	      if (node->loc != loc)
+		{
+		  if (REG_P (node->loc))
+		    {
+		      attrs anode, anext;
+		      attrs *anextp;
+
+		      /* Remove the variable part from the register's
+			 list, but preserve any other variable parts
+			 that might be regarded as live in that same
+			 register.  */
+		      anextp = &set->regs[REGNO (node->loc)];
+		      for (anode = *anextp; anode; anode = anext)
+			{
+			  anext = anode->next;
+			  if (anode->decl == decl
+			      && anode->offset == offset)
+			    {
+			      pool_free (attrs_pool, anode);
+			      *anextp = anext;
+			    }
+			}
+		    }
+
+		  delete_variable_part (set, node->loc, decl, offset);
+		}
 	    }
 	}
     }

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2006-08-15 07:03:59.000000000 -0300
+++ gcc/var-tracking.c	2006-08-15 07:05:55.000000000 -0300
@@ -1418,6 +1418,114 @@ dataflow_set_different (dataflow_set *ol
   return dataflow_set_different_value;
 }
 
+/* A trivial wrapper around dump_variable.  */
+
+static void
+dump_variable_simple (variable var)
+{
+  void *v = var;
+
+  dump_variable (&v, 0);
+}
+
+/* Compare variable *SLOT with the same variable in hash table DATA
+   and dump the changes if they are different.  */
+
+static int
+dataflow_set_differences_1 (void **slot, void *data)
+{
+  htab_t htab = (htab_t) data;
+  variable var1, var2;
+
+  var1 = *(variable *) slot;
+  var2 = htab_find_with_hash (htab, var1->decl,
+			      VARIABLE_HASH_VAL (var1->decl));
+  if (!var2)
+    {
+      fprintf (dump_file, "-");
+      dump_variable_simple (var1);
+
+      /* Stop traversing the hash table.  */
+      return 1;
+    }
+
+  if (variable_different_p (var1, var2, false))
+    {
+      fprintf (dump_file, "-");
+      dump_variable_simple (var1);
+      fprintf (dump_file, "+");
+      dump_variable_simple (var2);
+      /* Stop traversing the hash table.  */
+      return 1;
+    }
+
+  /* Continue traversing the hash table.  */
+  return 1;
+}
+
+/* Compare variable *SLOT with the same variable in hash table DATA
+   and set DATAFLOW_SET_DIFFERENT_VALUE if they are different.  */
+
+static int
+dataflow_set_differences_2 (void **slot, void *data)
+{
+  htab_t htab = (htab_t) data;
+  variable var1, var2;
+
+  var1 = *(variable *) slot;
+  var2 = htab_find_with_hash (htab, var1->decl,
+			      VARIABLE_HASH_VAL (var1->decl));
+  if (!var2)
+    {
+      fprintf (dump_file, "+");
+      dump_variable_simple (var1);
+
+      /* Stop traversing the hash table.  */
+      return 1;
+    }
+
+  /* Continue traversing the hash table.  */
+  return 1;
+}
+
+/* Dump all differences between two dataflow sets.  */
+
+static void
+dump_dataflow_differences (int bbidx, dataflow_set *old_set,
+			   dataflow_set *new_set)
+{
+  int i;
+
+  fprintf (dump_file, "\nBasic block %d OUT changes:\n", bbidx);
+
+  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+    {
+      attrs list;
+
+      for (list = old_set->regs[i]; list; list = list->next)
+	if (!attrs_list_member (new_set->regs[i],
+				list->decl, list->offset))
+	  {
+	    fprintf (dump_file, "-Reg %d:", i);
+	    print_mem_expr (dump_file, list->decl);
+	    fprintf (dump_file, " (%p) +" HOST_WIDE_INT_PRINT_DEC "\n",
+		     list->decl, list->offset);
+	  }
+
+      for (list = new_set->regs[i]; list; list = list->next)
+	if (!attrs_list_member (old_set->regs[i],
+				list->decl, list->offset))
+	  {
+	    fprintf (dump_file, "+Reg %d:", i);
+	    print_mem_expr (dump_file, list->decl);
+	    fprintf (dump_file, " (%p) +" HOST_WIDE_INT_PRINT_DEC "\n",
+		     list->decl, list->offset);
+	  }
+    }
+  htab_traverse (old_set->vars, dataflow_set_differences_1, new_set->vars);
+  htab_traverse (new_set->vars, dataflow_set_differences_2, old_set->vars);
+}
+
 /* Free the contents of dataflow set SET.  */
 
 static void
@@ -1791,10 +1899,14 @@ compute_bb_dataflow (basic_block bb)
     }
 
   changed = dataflow_set_different (&old_out, out);
+  if (changed && dump_file)
+    dump_dataflow_differences (bb->index, &old_out, out);
   dataflow_set_destroy (&old_out);
   return changed;
 }
 
+static FILE **var_tracking_debug_out __attribute__ ((__used__)) = &stderr;
+
 /* Find the locations of variables in the whole function.  */
 
 static void
@@ -1922,8 +1922,8 @@ dump_variable (void **slot, void *data A
   int i;
   location_chain node;
 
-  fprintf (dump_file, "  name: %s\n",
-	   IDENTIFIER_POINTER (DECL_NAME (var->decl)));
+  fprintf (dump_file, "  name: %s (%p)\n",
+	   IDENTIFIER_POINTER (DECL_NAME (var->decl)), (void *)var->decl);
   for (i = 0; i < var->n_var_parts; i++)
     {
       fprintf (dump_file, "    offset %ld\n",

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}