Patchwork regrename: don't update REG_DEAD notes

login
register
mail settings
Submitter Bernd Schmidt
Date June 16, 2011, 2:33 p.m.
Message ID <4DFA141D.7010007@codesourcery.com>
Download mbox | patch
Permalink /patch/100652/
State New
Headers show

Comments

Bernd Schmidt - June 16, 2011, 2:33 p.m.
We're updating REG_DEAD notes in regrename.c, but it seems this is for
historical reasons only. As far as I can tell, every pass that needs
such notes recomputes them with df_note_add_problem/df_analyze, and the
following patch bootstrapped and tested successfully on i686-linux (some
random libmudflap failures mentioned previously), with the following
change to ensure it's tested:

+++ gcc/config/i386/i386.c	(working copy)
@@ -5099,6 +5099,9 @@ static const struct default_options ix86
 #ifdef INSN_SCHEDULING
     { OPT_LEVELS_ALL, OPT_fschedule_insns, NULL, 0 },
 #endif
+#ifdef INSN_SCHEDULING
+    { OPT_LEVELS_2_PLUS, OPT_frename_registers, NULL, 1 },
+#endif

Ok?


Bernd
* regrename.c (do_replace): Don't update notes.
Richard Henderson - June 16, 2011, 3:13 p.m.
On 06/16/2011 07:33 AM, Bernd Schmidt wrote:
> 	* regrename.c (do_replace): Don't update notes.

Ok.


r~
Hans-Peter Nilsson - June 16, 2011, 3:25 p.m.
On Thu, 16 Jun 2011, Bernd Schmidt wrote:
> We're updating REG_DEAD notes in regrename.c, but it seems this is for
> historical reasons only. As far as I can tell, every pass that needs
> such notes recomputes them with df_note_add_problem/df_analyze, and the
> following patch bootstrapped and tested successfully on i686-linux (some
> random libmudflap failures mentioned previously), with the following
> change to ensure it's tested:
>
> +++ gcc/config/i386/i386.c	(working copy)

(Not a dbr target...)

> Ok?

NO!  They're still used by reorg.c+resource.c

brgds, H-P
Bernd Schmidt - June 16, 2011, 3:40 p.m.
On 06/16/2011 05:25 PM, Hans-Peter Nilsson wrote:
> On Thu, 16 Jun 2011, Bernd Schmidt wrote:
>> We're updating REG_DEAD notes in regrename.c, but it seems this is for
>> historical reasons only. As far as I can tell, every pass that needs
>> such notes recomputes them with df_note_add_problem/df_analyze, and the
>> following patch bootstrapped and tested successfully on i686-linux (some
>> random libmudflap failures mentioned previously), with the following
>> change to ensure it's tested:
>>
>> +++ gcc/config/i386/i386.c	(working copy)
> 
> (Not a dbr target...)
> 
>> Ok?
> 
> NO!  They're still used by reorg.c+resource.c

Well, that's reorg's problem. In between regrename and reorg.c, we have
a number of opportunities, such as pass_sched2, which use

  df_set_flags (DF_LR_RUN_DCE);
  df_note_add_problem ();

to recompute notes. If reorg.c wants accurate notes it should do the
same, especially since the scheduler (and presumably several other
passes) already makes no effort to update such notes. It seems this was
removed with the df merge:

        * sched-rgn.c (check_dead_notes1, deaths_in_region): Removed.
        (init_regions, add_block1): Removed last of note counting code.


Bernd
Richard Henderson - June 16, 2011, 3:54 p.m.
On 06/16/2011 08:25 AM, Hans-Peter Nilsson wrote:
>> +++ gcc/config/i386/i386.c	(working copy)
> 
> (Not a dbr target...)
> 
>> Ok?
> 
> NO!  They're still used by reorg.c+resource.c

Already handled:

rest_of_pass_free_cfg (void)
{
#ifdef DELAY_SLOTS
  /* The resource.c machinery uses DF but the CFG isn't guaranteed to be
     valid at that point so it would be too late to call df_analyze.  */
  if (optimize > 0 && flag_delayed_branch)
    {
      df_note_add_problem ();
      df_analyze ();
    }
#endif



r~

Patch

Index: gcc/regrename.c
===================================================================
--- gcc/regrename.c	(revision 174842)
+++ gcc/regrename.c	(working copy)
@@ -432,7 +432,6 @@  do_replace (struct du_head *head, int re
 {
   struct du_chain *chain;
   unsigned int base_regno = head->regno;
-  bool found_note = false;
 
   gcc_assert (! DEBUG_INSN_P (head->first->insn));
 
@@ -446,46 +445,15 @@  do_replace (struct du_head *head, int re
 	INSN_VAR_LOCATION_LOC (chain->insn) = gen_rtx_UNKNOWN_VAR_LOC ();
       else
 	{
-	  rtx note;
-
 	  *chain->loc = gen_raw_REG (GET_MODE (*chain->loc), reg);
 	  if (regno >= FIRST_PSEUDO_REGISTER)
 	    ORIGINAL_REGNO (*chain->loc) = regno;
 	  REG_ATTRS (*chain->loc) = attr;
 	  REG_POINTER (*chain->loc) = reg_ptr;
-
-	  for (note = REG_NOTES (chain->insn); note; note = XEXP (note, 1))
-	    {
-	      enum reg_note kind = REG_NOTE_KIND (note);
-	      if (kind == REG_DEAD || kind == REG_UNUSED)
-		{
-		  rtx reg = XEXP (note, 0);
-		  gcc_assert (HARD_REGISTER_P (reg));
-
-		  if (REGNO (reg) == base_regno)
-		    {
-		      found_note = true;
-		      if (kind == REG_DEAD
-			  && reg_set_p (*chain->loc, chain->insn))
-			remove_note (chain->insn, note);
-		      else
-			XEXP (note, 0) = *chain->loc;
-		      break;
-		    }
-		}
-	    }
 	}
 
       df_insn_rescan (chain->insn);
     }
-  if (!found_note)
-    {
-      /* If the chain's first insn is the same as the last, we should have
-	 found a REG_UNUSED note.  */
-      gcc_assert (head->first->insn != head->last->insn);
-      if (!reg_set_p (*head->last->loc, head->last->insn))
-	add_reg_note (head->last->insn, REG_DEAD, *head->last->loc);
-    }
 }