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
