diff mbox

Restore bootstrap (PR53197)

Message ID Pine.LNX.4.64.1205030409560.25409@wotan.suse.de
State New
Headers show

Commit Message

Michael Matz May 3, 2012, 2:34 a.m. UTC 
Hi,

my gimple_seq reshuffling broke boostrap on some machines.  In particular 
on those for which contrib/compare-debug works, meaning that 
bootstrap-debug is selected as default BUILD_CONFIG (none of my machines 
pass the respective configure.ac test, and hence I wasn't seeing the 
problem anywhere).  Thanks for Jonathan for access to a breaking machine 
that set me on the right track.

The things is, that my patch causes different code to be emitted depending 
on -g or not -g (or -gtoggle for stage2, which is what the bootstrap-debug 
config is doing), when DSE wants to remove some stores.  It walks the 
sequence incorrectly, potentially with a stale iterator.  This was 
harmless before, but isn't anymore for backward walks due to the cyclic 
structure of the prev links.  The effect is that once the last statement 
is removed and gsi_prev is called on the original (now stale) iterator 
then gsi_end_p will be true, even if there are statement before the just 
removed one.

Hence with -g (where the last stmt is a DEBUG one, which isn't removed) 
all dead stores were removed in a certain block, whereas with -g0 (where 
the store was the last stmt) only that last stmt was removed.

So, do the same as most other backward walkers that potentially remove 
last statements (e.g. tree-cfg.c) do and update the iterator instead of a 
copy, as well as reset to the end of sequence in case we removed the last 
one.

I checked that this patch restores bootstrap when configured with 
--with-build-config=bootstrap-debug, but only for the C language.  The 
miscomparing files in the bug report are all in libbackend.a, so there 
shouldn't be more coming with more languages.

In order not to leave the tree broken for much longer I've committed the 
below patch as r187074.  A full regstrap is running, but I won't see the 
results until in a few hours.


Ciao,
Michael.
------------------------
	PR bootstrap/53197
	* tree-ssa-dse.c (dse_optimize_stmt): Take pointer to
	iterator.
	(dse_enter_block): Properly iterate the whole sequence even
	if the last statement was removed.

Comments

Alexandre Oliva May 3, 2012, 7:11 p.m. UTC | #1 
On May  2, 2012, Michael Matz <matz@suse.de> wrote:

> my gimple_seq reshuffling broke boostrap on some machines.  In particular 
> on those for which contrib/compare-debug works, meaning that 
> bootstrap-debug is selected as default BUILD_CONFIG (none of my machines 
> pass the respective configure.ac test, and hence I wasn't seeing the 
> problem anywhere).

Any chance you could adjust the script to strip out the .comment section
you mentioned elsewhere, so that you get faster bootstrap with
bootstrap-debug testing too?

> I checked that this patch restores bootstrap

Thanks!
diff mbox

Patch

Index: tree-ssa-dse.c
===================================================================
--- tree-ssa-dse.c	(revision 187053)
+++ tree-ssa-dse.c	(working copy)
@@ -199,9 +199,9 @@  dse_possible_dead_store_p (gimple stmt,
    post dominates the first store, then the first store is dead.  */
 
 static void
-dse_optimize_stmt (gimple_stmt_iterator gsi)
+dse_optimize_stmt (gimple_stmt_iterator *gsi)
 {
-  gimple stmt = gsi_stmt (gsi);
+  gimple stmt = gsi_stmt (*gsi);
 
   /* If this statement has no virtual defs, then there is nothing
      to do.  */
@@ -252,7 +252,7 @@  dse_optimize_stmt (gimple_stmt_iterator
 	  if (dump_file && (dump_flags & TDF_DETAILS))
             {
               fprintf (dump_file, "  Deleted dead store '");
-              print_gimple_stmt (dump_file, gsi_stmt (gsi), dump_flags, 0);
+              print_gimple_stmt (dump_file, gsi_stmt (*gsi), dump_flags, 0);
               fprintf (dump_file, "'\n");
             }
 
@@ -261,7 +261,7 @@  dse_optimize_stmt (gimple_stmt_iterator
 
 	  /* Remove the dead store.  */
 	  bb = gimple_bb (stmt);
-	  if (gsi_remove (&gsi, true))
+	  if (gsi_remove (gsi, true))
 	    bitmap_set_bit (need_eh_cleanup, bb->index);
 
 	  /* And release any SSA_NAMEs set in this statement back to the
@@ -277,8 +277,14 @@  dse_enter_block (struct dom_walk_data *w
 {
   gimple_stmt_iterator gsi;
 
-  for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
-    dse_optimize_stmt (gsi);
+  for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi);)
+    {
+      dse_optimize_stmt (&gsi);
+      if (gsi_end_p (gsi))
+	gsi = gsi_last_bb (bb);
+      else
+	gsi_prev (&gsi);
+    }
 }
 
 /* Main entry point.  */