From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28036 invoked by alias); 9 Nov 2015 13:32:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 28013 invoked by uid 89); 9 Nov 2015 13:32:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mailapp01.imgtec.com Received: from mailapp01.imgtec.com (HELO mailapp01.imgtec.com) (195.59.15.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Nov 2015 13:32:04 +0000 Received: from hhmail02.hh.imgtec.org (unknown [10.100.10.20]) by Websense Email Security Gateway with ESMTPS id 5A9888297A1DD; Mon, 9 Nov 2015 13:31:58 +0000 (GMT) Received: from hhmail02.hh.imgtec.org ([::1]) by hhmail02.hh.imgtec.org ([::1]) with mapi id 14.03.0235.001; Mon, 9 Nov 2015 13:32:01 +0000 From: Robert Suchanek To: Bernd Schmidt , "ebotcazou@adacore.com" , "gcc-patches@gcc.gnu.org" Subject: RE: [RFC][PATCH] Preferred rename register in regrename pass Date: Mon, 09 Nov 2015 13:32:00 -0000 Message-ID: References: <55FC2B8D.6000106@redhat.com> <56179FC6.3040503@redhat.com> In-Reply-To: <56179FC6.3040503@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg00955.txt.bz2 Hi Bernd, Sorry for late reply. The updated patch was bootstrapped on x86_64-unknown-linux-gnu and cross te= sted 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.=20 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 effecti= ve. However, on average case the patch appears to reduce the code size slightly and moves are genuinely removed.=20=20 I haven't tested the performance extensively but the SPEC benchmarks showed almost the same results, which could be just the noise.=20 | 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. >=20 > 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. >=20 > I'd still like to investigate the possibility of further simplification: >=20 > > + { > > + /* Find the input chain. */ > > + for (i =3D c->id - 1; id_to_chain.iterate (i, &head); i--) > > + if (head->last && head->last->insn =3D=3D insn > > + && head->terminated_dead) > > + { > > + gcc_assert (head->regno =3D=3D REGNO (recog_data.operand[1])); > > + c->tied_chain =3D head; > > + head->tied_chain =3D 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. >=20 > Other than that I'm pretty happy with this. >=20 >=20 > 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; =20 +/* 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_nr= egs, rtx *loc, head->nregs =3D this_nregs; head->need_caller_save_reg =3D 0; head->cannot_rename =3D 0; + head->renamed =3D 0; + head->tied_chain =3D NULL; =20 id_to_chain.safe_push (head); head->id =3D current_id++; @@ -366,6 +371,13 @@ find_rename_reg (du_head_p this_head, enum reg_class s= uper_class, preferred_class =3D (enum reg_class) targetm.preferred_rename_class (super_class); =20 + /* 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; =20 mode =3D GET_MODE (*head->first->loc); + head->renamed =3D 1; head->regno =3D reg; head->nregs =3D hard_regno_nregs[reg][mode]; return true; @@ -1043,7 +1056,34 @@ scan_rtx_reg (rtx_insn *insn, rtx *loc, enum reg_cla= ss cl, enum scan_actions act if (action =3D=3D mark_write) { if (type =3D=3D OP_OUT) - create_new_chain (this_regno, this_nregs, loc, insn, cl); + { + du_head_p c; + rtx pat =3D PATTERN (insn); + + c =3D 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 =3D=3D 2 + && GET_CODE (pat) =3D=3D SET + && GET_CODE (SET_DEST (pat)) =3D=3D REG + && GET_CODE (SET_SRC (pat)) =3D=3D REG + && terminated_this_insn) + { + gcc_assert + (terminated_this_insn->regno =3D=3D REGNO (recog_data.operand[1])); + + c->tied_chain =3D terminated_this_insn; + terminated_this_insn->tied_chain =3D 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; } =20 @@ -1151,6 +1191,8 @@ scan_rtx_reg (rtx_insn *insn, rtx *loc, enum reg_clas= s cl, enum scan_actions act SET_HARD_REG_BIT (live_hard_regs, head->regno + nregs); } =20 + if (action =3D=3D terminate_dead) + terminated_this_insn =3D *p; *p =3D 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); =20 + terminated_this_insn =3D 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; }; =20 typedef struct du_head *du_head_p; --=20 2.4.5