public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [dataflow]: PATCH COMMITTED to fix problems with dce.
@ 2007-05-16 13:36 Kenneth Zadeck
  2007-05-18 17:05 ` Richard Sandiford
  0 siblings, 1 reply; 2+ messages in thread
From: Kenneth Zadeck @ 2007-05-16 13:36 UTC (permalink / raw)
  To: gcc-patches, Steven Bosscher, Park, Seongbae, Zadeck, Kenneth

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

This patch fixes a quality and a correctness regression in dce.  It also
upgrades some dumping and monitoring info.

The two dce regressions are:

1) RTX_FRAME_RELATED_P insns were being deleted if they looked to be
dead.  This messed up some exception processing.
2) The live_out set was not being cleared before being recomputed during
dce.  Since
confluence functions can only add bits and dce makes changes that only
delete bits, this was inhibiting the non local effects of doing dce. 

This patch has been bootstrapped and regression tested on x86-64,
x86-32, ppc, and ia-64.  The regressions fixed were in libffi on ppc,
but the failure would likely occur on other platforms. 

Kenny

2007-05-16  Kenneth Zadeck <zadeck@naturalbridge.com>

    * regstat.c (regstat_init_n_sets_and_refs, regstat_compute_ri,
    regstat_compute_calls_crossed): Added time variable TV_REG_STATS.
    * timevar.def (TV_REG_STATS): Added.
    (TV_DF_RI): Deleted.
    * df-problems.c (df_print_bb_index): Added identifer for eh blocks.
    * dce.c (deletable_insn_p): Added code to not delete
    RTX_FRAME_RELATED_P insns.
    (dce_process_block): Removed insns_deleted and added code to reset
    live set before confluence function.  
    (rest_of_handle_fast_dce): Added code to turn off df's version of
dce if
    flag_dce is not set and added DF_NO_INSN_RESCAN to flags that are
cleared.


[-- Attachment #2: dce6.diff --]
[-- Type: text/x-patch, Size: 6653 bytes --]

Index: regstat.c
===================================================================
--- regstat.c	(revision 124596)
+++ regstat.c	(working copy)
@@ -58,6 +58,7 @@ regstat_init_n_sets_and_refs (void)
   unsigned int i;
   unsigned int max_regno = max_reg_num ();
 
+  timevar_push (TV_REG_STATS);
   df_grow_reg_info ();
   gcc_assert (!regstat_n_sets_and_refs);
 
@@ -68,6 +69,8 @@ regstat_init_n_sets_and_refs (void)
       SET_REG_N_SETS (i, DF_REG_DEF_COUNT (i));
       SET_REG_N_REFS (i, DF_REG_USE_COUNT (i) + REG_N_SETS (i));
     }
+  timevar_pop (TV_REG_STATS);
+
 }
 
 
@@ -338,6 +341,7 @@ regstat_compute_ri (void)
 
   gcc_assert (!reg_info_p);
 
+  timevar_push (TV_REG_STATS);
   setjmp_crosses = BITMAP_ALLOC (&df_bitmap_obstack);
   max_regno = max_reg_num ();
   reg_info_p_size = max_regno;
@@ -362,6 +366,7 @@ regstat_compute_ri (void)
   
   BITMAP_FREE (local_live);
   BITMAP_FREE (local_processed);
+  timevar_pop (TV_REG_STATS);
 }
 
 
@@ -478,6 +483,7 @@ regstat_compute_calls_crossed (void)
   /* Initialize everything.  */
   gcc_assert (!reg_info_p);
 
+  timevar_push (TV_REG_STATS);
   max_regno = max_reg_num ();
   reg_info_p_size = max_regno;
   reg_info_p = xcalloc (max_regno, sizeof (struct reg_info_t));
@@ -488,6 +494,7 @@ regstat_compute_calls_crossed (void)
     }
 
   BITMAP_FREE (live);
+  timevar_pop (TV_REG_STATS);
 }
 
 
Index: timevar.def
===================================================================
--- timevar.def	(revision 124596)
+++ timevar.def	(working copy)
@@ -66,7 +66,7 @@ DEFTIMEVAR (TV_DF_LIVE		     , "df live&
 DEFTIMEVAR (TV_DF_UREC		     , "df uninitialized regs 2")
 DEFTIMEVAR (TV_DF_CHAIN		     , "df use-def / def-use chains")
 DEFTIMEVAR (TV_DF_NOTE		     , "df reg dead/unused notes")
-DEFTIMEVAR (TV_DF_RI		     , "df register information")
+DEFTIMEVAR (TV_REG_STATS	     , "register information")
 
 DEFTIMEVAR (TV_ALIAS_ANALYSIS	     , "alias analysis")
 DEFTIMEVAR (TV_REG_SCAN		     , "register scan")
Index: df-problems.c
===================================================================
--- df-problems.c	(revision 124596)
+++ df-problems.c	(working copy)
@@ -166,13 +166,13 @@ df_print_bb_index (basic_block bb, FILE 
     FOR_EACH_EDGE (e, ei, bb->preds)
     {
       basic_block pred = e->src;
-      fprintf (file, "%d ", pred->index);
+      fprintf (file, "%d%s ", pred->index, e->flags & EDGE_EH ? "(EH)" : "");
     } 
   fprintf (file, ")->[%d]->( ", bb->index);
   FOR_EACH_EDGE (e, ei, bb->succs)
     {
       basic_block succ = e->dest;
-      fprintf (file, "%d ", succ->index);
+      fprintf (file, "%d%s ", succ->index, e->flags & EDGE_EH ? "(EH)" : "");
     } 
   fprintf (file, ")\n");
 }
Index: dce.c
===================================================================
--- dce.c	(revision 124596)
+++ dce.c	(working copy)
@@ -66,6 +66,11 @@ deletable_insn_p (rtx insn, bool fast)
 {
   rtx x;
 
+  /* These insns may not have real uses but are there because the
+     dwarf unwinder may need to see the values they compute.  */
+  if (RTX_FRAME_RELATED_P (insn))
+    return false;
+
   switch (GET_CODE (PATTERN (insn)))
     {
     case USE:
@@ -506,9 +511,6 @@ dce_process_block (basic_block bb, bool 
   bitmap local_live = BITMAP_ALLOC (&dce_tmp_bitmap_obstack);
   rtx insn;
   bool block_changed;
-#ifdef ENABLE_CHECKING
-  bool insns_deleted = false;
-#endif
   struct df_ref **def_rec, **use_rec;
   unsigned int bb_index = bb->index;
 
@@ -520,10 +522,17 @@ dce_process_block (basic_block bb, bool 
       edge e;
       edge_iterator ei;
       df_confluence_function_n con_fun_n = df_lr->problem->con_fun_n;
+      bitmap_clear (DF_LR_OUT (bb));
       FOR_EACH_EDGE (e, ei, bb->succs)
 	(*con_fun_n) (e);
     }
 
+  if (dump_file)
+    {
+      fprintf (dump_file, "processing block %d live out = ", bb->index);
+      df_print_regset (dump_file, DF_LR_OUT (bb));
+    }
+
   bitmap_copy (local_live, DF_LR_OUT (bb));
 
   /* Process the artificial defs and uses at the bottom of the block.  */
@@ -551,7 +560,6 @@ dce_process_block (basic_block bb, bool 
 	  {	
 	    bool needed = false;
 
-
 	    /* The insn is needed if there is someone who uses the output.  */
 	    for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
 	      if (bitmap_bit_p (local_live, DF_REF_REGNO (*def_rec)))
@@ -580,10 +588,6 @@ dce_process_block (basic_block bb, bool 
 		else
 		  mark_insn (insn, true);
 	      }
-#ifdef ENABLE_CHECKING
-	    else
-	      insns_deleted = true;
-#endif
 	  }
 	
 	/* No matter if the instruction is needed or not, we remove
@@ -660,12 +664,7 @@ dce_process_block (basic_block bb, bool 
 
   block_changed = !bitmap_equal_p (local_live, DF_LR_IN (bb));
   if (block_changed)
-    {
-#ifdef ENABLE_CHECKING
-      gcc_assert (insns_deleted);
-#endif
-      bitmap_copy (DF_LR_IN (bb), local_live);
-    }
+    bitmap_copy (DF_LR_IN (bb), local_live);
 
   BITMAP_FREE (local_live);
   return block_changed;
@@ -717,15 +716,10 @@ fast_dce (void)
 	      edge_iterator ei;
 	      FOR_EACH_EDGE (e, ei, bb->preds)
 		if (bitmap_bit_p (processed, e->src->index))
-		  /* Be very tricky about when we need to iterate the
-		     analysis.  We only have redo the analysis if we
-		     delete an instrution from a block that used
-		     something that was live on entry to the block and
-		     we have already processed the pred of that block.
-
-		     Since we are processing the blocks postorder,
-		     that will only happen if the block is at the top
-		     of a loop and the pred is inside the loop.  */
+		  /* Be tricky about when we need to iterate the
+		     analysis.  We only have redo the analysis if the
+		     bitmaps change at the top of a block that is the
+		     entry to a loop.  */
 		  global_changed = true;
 		else
 		  bitmap_set_bit (redo_out, e->src->index);
@@ -792,16 +786,20 @@ rest_of_handle_fast_dce (void)
 void
 run_fast_df_dce (void)
 {
-  /* If dce is able to delete something, it has to happen immediately.
-     Otherwise there will be problems handling the eq_notes.  */
-  enum df_changeable_flags old_flags = df_clear_flags (DF_DEFER_INSN_RESCAN);
-
-  df_in_progress = true;
-  rest_of_handle_fast_dce ();
-  df_set_flags (old_flags);
+  if (flag_dce)
+    {
+      /* If dce is able to delete something, it has to happen
+	 immediately.  Otherwise there will be problems handling the
+	 eq_notes.  */
+      enum df_changeable_flags old_flags 
+	= df_clear_flags (DF_DEFER_INSN_RESCAN + DF_NO_INSN_RESCAN);
+      
+      df_in_progress = true;
+      rest_of_handle_fast_dce ();
+      df_set_flags (old_flags);
+    }
 }
 
-
 static bool
 gate_fast_dce (void)
 {

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

* Re: [dataflow]: PATCH COMMITTED to fix problems with dce.
  2007-05-16 13:36 [dataflow]: PATCH COMMITTED to fix problems with dce Kenneth Zadeck
@ 2007-05-18 17:05 ` Richard Sandiford
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Sandiford @ 2007-05-18 17:05 UTC (permalink / raw)
  To: Kenneth Zadeck; +Cc: gcc-patches, Steven Bosscher, Park, Seongbae

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> Index: dce.c
> ===================================================================
> --- dce.c	(revision 124596)
> +++ dce.c	(working copy)
> @@ -66,6 +66,11 @@ deletable_insn_p (rtx insn, bool fast)
>  {
>    rtx x;
>  
> +  /* These insns may not have real uses but are there because the
> +     dwarf unwinder may need to see the values they compute.  */
> +  if (RTX_FRAME_RELATED_P (insn))
> +    return false;
> +
>    switch (GET_CODE (PATTERN (insn)))
>      {
>      case USE:

Hmm.  I'm a little uneasy about this.  I didn't think frame-relatedness
was a reason in itself not to delete something.

As far as prologue and epilogue instructions in general go -- not just
frame-related ones -- I thought the backend was supposed to add a
REG_MAYBE_DEAD note to instructions that might legitimately be deleted
as dead.  And I thought that gcc should abort if it thinks a prologue
or epilogue instruction without such a note is dead.  I think that
applies to frame-related as well as non-frame-related instructions.

(It's quite possible I'm misunderstanding what you're doing here,
and that this is just noise.  Sorry if so.)

Richard

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

end of thread, other threads:[~2007-05-18 17:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-05-16 13:36 [dataflow]: PATCH COMMITTED to fix problems with dce Kenneth Zadeck
2007-05-18 17:05 ` Richard Sandiford

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