This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [RFC][PATCH] Preferred rename register in regrename pass


Hi Bernd,

Sorry for late reply.

The updated patch was bootstrapped on x86_64-unknown-linux-gnu and cross tested
on mips-img-linux-gnu using r229786.

The results below were generated for CSiBE benchmark and the numbers in
columns express bytes in format 'net (gain/loss)' to show the difference
with and without the patch when -frename-registers switch is used. 

I looked at the gains, especially for MIPS and 'teem', and it appears
that renaming registers affects the rtl_dce pass i.e. makes it less effective.
However, on average case the patch appears to reduce the code size
slightly and moves are genuinely removed.  

I haven't tested the performance extensively but the SPEC benchmarks
showed almost the same results, which could be just the noise. 


               | MIPS n64 -Os   | MIPS o32 -Os   | x86_64 -Os       |
---------------+----------------+----------------+------------------+
bzip2-1.0.2    | -32  (0/-32)   | -24  (0/-24)   | -34   (1/-35)    |
cg_compiler    | -172 (0/-172)  | -156 (0/-156)  | -46   (0/-46)    |
compiler       | -36  (0/-36)   | -24  (0/-24)   | -6    (0/-6)     |
flex-2.5.31    | -68  (0/-68)   | -80  (0/-80)   | -98   (7/-105)   |
jikespg-1.3    | -284 (0/-284)  | -204 (0/-204)  | -127  (9/-136)   |
jpeg-6b        | -52  (8/-60)   | -20  (0/-20)   | -80   (11/-91)   |
libmspack      | -136 (0/-136)  | -28  (0/-28)   | -33   (23/-56)   |
libpng-1.2.5   | -72  (0/-72)   | -64  (0/-64)   | -176  (14/-190)  |
linux-2.4.23   | -700 (20/-720) | -384 (0/-384)  | -691  (44/-735)  |
lwip-0.5.3     | -4   (0/-4)    | -4   (0/-4)    | +4    (13/-9)    |
mpeg2dec-0.3.1 | -16  (0/-16)   |                | -142  (6/-148)   |
mpgcut-1.1     | -24  (0/-24)   | -12  (4/-16)   | -2    (0/-2)     |
OpenTCP-1.0.4  | -28  (0/-28)   | -12  (0/-12)   | -1    (0/-1)     |
replaypc-0.4.0 | -32  (0/-32)   | -12  (0/-12)   | -4    (2/-6)     |
teem-1.6.0     | -88  (480/-568)| +108 (564/-456)| -1272 (117/-1389)|
ttt-0.10.1     | -24  (0/-24)   | -20  (0/-20)   | -16   (0/-16)    |
unrarlib-0.4.0 | -20  (0/-20)   | -8   (0/-8)    | -59   (9/-68)    |
zlib-1.1.4     | -12  (0/-12)   | -4   (0/-4)    | -23   (8/-31)    |


               | MIPS n64 -O2   | MIPS o32 -O2   | x86_64 -O2       |
---------------+----------------+----------------+------------------+
bzip2-1.0.2    | -104 (0/-104)  | -48  (0/-48)   | -55   (0/-55)    |
cg_compiler    | -184 (4/-188)  | -232 (0/-232)  | -31   (5/-36)    |
compiler       | -32  (0/-32)   | -12  (0/-12)   | -4    (1/-5)     |
flex-2.5.31    | -96  (0/-96)   | -112 (0/-112)  | -12   (34/-46)   |
jikespg-1.3    | -540 (20/-560) | -476 (4/-480)  | -154  (30/-184)  |
jpeg-6b        | -112 (16/-128) | -60  (0/-60)   | -136  (84/-220)  |
libmspack      | -164 (0/-164)  | -40  (0/-40)   | -87   (32/-119)  |
libpng-1.2.5   | -120 (8/-128)  | -92  (4/-96)   | -140  (53/-193)  |
linux-2.4.23   | -596 (12/-608) | -320 (8/-328)  | -794  (285/-1079)|
lwip-0.5.3     | -8   (0/-8)    | -8   (0/-8)    | +2    (4/-2)     |
mpeg2dec-0.3.1 | -44  (0/-44)   | -4   (0/-4)    | -122  (8/-130)   |
mpgcut-1.1     | -8   (0/-8)    | -8   (0/-8)    | +28   (32/-4)    |
OpenTCP-1.0.4  | -4   (0/-4)    | -4   (0/-4)    | -2    (0/-2)     |
replaypc-0.4.0 | -20  (0/-20)   | -24  (0/-24)   | -13   (0/-13)    |
teem-1.6.0     | +100 (740/-640)| +84  (736/-652)| -1998 (168/-2166)|
ttt-0.10.1     | -16  (0/-16)   |                |                  |
unrarlib-0.4.0 | -16  (0/-16)   | -8   (0/-8)    | +19   (37/-18)   |
zlib-1.1.4     | -12  (0/-12)   | -4   (0/-4)    | -15   (1/-16)    |

Regards,
Robert

> Hi Robert,
> > gcc/
> > 	* regrename.c (create_new_chain): Initialize terminated_dead,
> > 	renamed and tied_chain.
> > 	(find_best_rename_reg): Pick and check register from the tied chain.
> > 	(regrename_do_replace): Mark head as renamed.
> > 	(scan_rtx_reg): Tie chains in move insns.  Set terminate_dead flag.
> > 	* regrename.h (struct du_head): Add tied_chain, renamed and
> > 	terminated_dead members.
> 
> Thanks - this looks a lot better already. You didn't say how it was
> bootstrapped and tested; please include this information for future
> submissions. For a patch like this, some data on the improvement you got
> would also be appreciated.
> 
> I'd still like to investigate the possibility of further simplification:
> 
> > +	    {
> > +	      /* Find the input chain.  */
> > +	      for (i = c->id - 1; id_to_chain.iterate (i, &head); i--)
> > +		if (head->last && head->last->insn == insn
> > +		    && head->terminated_dead)
> > +		  {
> > +		    gcc_assert (head->regno == REGNO (recog_data.operand[1]));
> > +		    c->tied_chain = head;
> > +		    head->tied_chain = c;
> > +
> > +		    if (dump_file)
> > +		      fprintf (dump_file, "Tying chain %s (%d) with %s (%d)\n",
> > +			       reg_names[c->regno], c->id,
> > +			       reg_names[head->regno], head->id);
> > +		    /* Once tied, we're done.  */
> > +		    break;
> > +		  }
> > +	    }
> > +	}
> > +
> This looks like it's a little more complicated than necessary. Couldn't
> you add a static var "terminated_this_insn" which gets initialized to
> NULL and set when a reg dies, and then you check this here rather than
> having a loop? That would also eliminate the new "terminated_dead" field.
> 
> Other than that I'm pretty happy with this.
> 
> 
> Bernd

gcc/
	* regrename.c (create_new_chain): Initialize renamed and tied_chain.
	(build_def_use): Initialize terminated_this_insn.
	(find_best_rename_reg): Pick and check register from the tied chain.
	(regrename_do_replace): Mark head as renamed.
	(struct du_head *terminated_this_insn). New static variable.
	(scan_rtx_reg): Tie chains in move insns.  Set terminated_this_insn.
	* regrename.h (struct du_head): Add tied_chain, renamed members.
---
 gcc/regrename.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 gcc/regrename.h |  4 ++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/gcc/regrename.c b/gcc/regrename.c
index 5f383fc..d3f9951 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -130,6 +130,9 @@ static HARD_REG_SET live_hard_regs;
    record_operand_use.  */
 static operand_rr_info *cur_operand;
 
+/* Set while scanning RTL if a register dies.  Used to tie chains.  */
+static struct du_head *terminated_this_insn;
+
 /* Return the chain corresponding to id number ID.  Take into account that
    chains may have been merged.  */
 du_head_p
@@ -224,6 +227,8 @@ create_new_chain (unsigned this_regno, unsigned this_nregs, rtx *loc,
   head->nregs = this_nregs;
   head->need_caller_save_reg = 0;
   head->cannot_rename = 0;
+  head->renamed = 0;
+  head->tied_chain = NULL;
 
   id_to_chain.safe_push (head);
   head->id = current_id++;
@@ -366,6 +371,13 @@ find_rename_reg (du_head_p this_head, enum reg_class super_class,
   preferred_class
     = (enum reg_class) targetm.preferred_rename_class (super_class);
 
+  /* Pick and check the register from the tied chain iff the tied chain
+     is not renamed.  */
+  if (this_head->tied_chain && !this_head->tied_chain->renamed
+      && check_new_reg_p (old_reg, this_head->tied_chain->regno,
+			  this_head, *unavailable))
+    return this_head->tied_chain->regno;
+
   /* If PREFERRED_CLASS is not NO_REGS, we iterate in the first pass
      over registers that belong to PREFERRED_CLASS and try to find the
      best register within the class.  If that failed, we iterate in
@@ -960,6 +972,7 @@ regrename_do_replace (struct du_head *head, int reg)
     return false;
 
   mode = GET_MODE (*head->first->loc);
+  head->renamed = 1;
   head->regno = reg;
   head->nregs = hard_regno_nregs[reg][mode];
   return true;
@@ -1043,7 +1056,34 @@ scan_rtx_reg (rtx_insn *insn, rtx *loc, enum reg_class cl, enum scan_actions act
   if (action == mark_write)
     {
       if (type == OP_OUT)
-	create_new_chain (this_regno, this_nregs, loc, insn, cl);
+	{
+	  du_head_p c;
+	  rtx pat = PATTERN (insn);
+
+	  c = create_new_chain (this_regno, this_nregs, loc, insn, cl);
+
+	  /* We try to tie chains in a move instruction for
+	     a single output.  */
+	  if (recog_data.n_operands == 2
+	      && GET_CODE (pat) == SET
+	      && GET_CODE (SET_DEST (pat)) == REG
+	      && GET_CODE (SET_SRC (pat)) == REG
+	      && terminated_this_insn)
+	    {
+	      gcc_assert
+		(terminated_this_insn->regno == REGNO (recog_data.operand[1]));
+
+	      c->tied_chain = terminated_this_insn;
+	      terminated_this_insn->tied_chain = c;
+
+	      if (dump_file)
+		fprintf (dump_file, "Tying chain %s (%d) with %s (%d)\n",
+			 reg_names[c->regno], c->id,
+			 reg_names[terminated_this_insn->regno],
+			 terminated_this_insn->id);
+	    }
+	}
+
       return;
     }
 
@@ -1151,6 +1191,8 @@ scan_rtx_reg (rtx_insn *insn, rtx *loc, enum reg_class cl, enum scan_actions act
 		SET_HARD_REG_BIT (live_hard_regs, head->regno + nregs);
 	    }
 
+	  if (action == terminate_dead)
+	    terminated_this_insn = *p;
 	  *p = next;
 	  if (dump_file)
 	    fprintf (dump_file,
@@ -1707,6 +1749,8 @@ build_def_use (basic_block bb)
 	      scan_rtx (insn, &XEXP (note, 0), ALL_REGS, mark_read,
 			OP_INOUT);
 
+	  terminated_this_insn = NULL;
+
 	  /* Step 4: Close chains for registers that die here, unless
 	     the register is mentioned in a REG_UNUSED note.  In that
 	     case we keep the chain open until step #7 below to ensure
diff --git a/gcc/regrename.h b/gcc/regrename.h
index bbe156d..9c72181 100644
--- a/gcc/regrename.h
+++ b/gcc/regrename.h
@@ -28,6 +28,8 @@ struct du_head
   struct du_head *next_chain;
   /* The first and last elements of this chain.  */
   struct du_chain *first, *last;
+  /* The chain that this chain is tied to.  */
+  struct du_head *tied_chain;
   /* Describes the register being tracked.  */
   unsigned regno;
   int nregs;
@@ -45,6 +47,8 @@ struct du_head
      such as the SET_DEST of a CALL_INSN or an asm operand that used
      to be a hard register.  */
   unsigned int cannot_rename:1;
+  /* Nonzero if the chain is renamed.  */
+  unsigned int renamed:1;
 };
 
 typedef struct du_head *du_head_p;
-- 
2.4.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]