From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44318) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMgCl-0003mC-Mt for qemu-devel@nongnu.org; Mon, 01 Apr 2013 10:59:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UMgCg-0008C5-MK for qemu-devel@nongnu.org; Mon, 01 Apr 2013 10:59:27 -0400 Received: from hall.aurel32.net ([2001:470:1f15:c4f::1]:52571) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UMgCg-0008C1-Em for qemu-devel@nongnu.org; Mon, 01 Apr 2013 10:59:22 -0400 Date: Mon, 1 Apr 2013 16:59:20 +0200 From: Aurelien Jarno Message-ID: <20130401145919.GA20384@ohm.aurel32.net> References: <1362443590-28191-1-git-send-email-rth@twiddle.net> <1362443590-28191-23-git-send-email-rth@twiddle.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <1362443590-28191-23-git-send-email-rth@twiddle.net> Subject: Re: [Qemu-devel] [PATCH v2 22/27] tcg-ppc64: Use TCGType throughout compares List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: av1474@comtv.ru, qemu-devel@nongnu.org On Mon, Mar 04, 2013 at 04:33:05PM -0800, Richard Henderson wrote: > The optimization/bug being fixed is that tcg_out_cmp was not applying the > right type to loading a constant, in the case it can't be implemented > directly. Rather than recomputing the TCGType enum from the arch64 bool, > pass around the original TCGType throughout. > > Signed-off-by: Richard Henderson > --- > tcg/ppc64/tcg-target.c | 38 +++++++++++++++++++------------------- > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c > index e34e548..355689a 100644 > --- a/tcg/ppc64/tcg-target.c > +++ b/tcg/ppc64/tcg-target.c > @@ -1053,8 +1053,8 @@ static void tcg_out_st (TCGContext *s, TCGType type, TCGReg arg, TCGReg arg1, > tcg_out_ldsta (s, arg, arg1, arg2, STD, STDX); > } > > -static void tcg_out_cmp (TCGContext *s, int cond, TCGArg arg1, TCGArg arg2, > - int const_arg2, int cr, int arch64) > +static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2, > + int const_arg2, int cr, TCGType type) > { > int imm; > uint32_t op; > @@ -1111,19 +1111,17 @@ static void tcg_out_cmp (TCGContext *s, int cond, TCGArg arg1, TCGArg arg2, > default: > tcg_abort (); > } > - op |= BF (cr) | (arch64 << 21); > + op |= BF(cr) | ((type == TCG_TYPE_I64) << 21); > > - if (imm) > - tcg_out32 (s, op | RA (arg1) | (arg2 & 0xffff)); > - else { > + if (imm) { > + tcg_out32(s, op | RA(arg1) | (arg2 & 0xffff)); > + } else { > if (const_arg2) { > - tcg_out_movi (s, TCG_TYPE_I64, 0, arg2); > - tcg_out32 (s, op | RA (arg1) | RB (0)); > + tcg_out_movi(s, type, 0, arg2); > + arg2 = 0; > } > - else > - tcg_out32 (s, op | RA (arg1) | RB (arg2)); > + tcg_out32(s, op | RA(arg1) | RB(arg2)); > } > - > } I do wonder if brcond should accept operand of the 'i' type, while in practice it might only load 'I' or 'J' type. That said your patch is already an improvement, so: Reviewed-by: Aurelien Jarno > static void tcg_out_setcond (TCGContext *s, TCGType type, TCGCond cond, > @@ -1217,7 +1215,7 @@ static void tcg_out_setcond (TCGContext *s, TCGType type, TCGCond cond, > sh = 31; > crop = CRNOR | BT (7, CR_EQ) | BA (7, CR_GT) | BB (7, CR_GT); > crtest: > - tcg_out_cmp (s, cond, arg1, arg2, const_arg2, 7, type == TCG_TYPE_I64); > + tcg_out_cmp(s, cond, arg1, arg2, const_arg2, 7, type); > if (crop) tcg_out32 (s, crop); > tcg_out32 (s, MFCR | RT (0)); > tcg_out_rlw(s, RLWINM, arg0, 0, sh, 31, 31); > @@ -1243,12 +1241,12 @@ static void tcg_out_bc (TCGContext *s, int bc, int label_index) > } > } > > -static void tcg_out_brcond (TCGContext *s, TCGCond cond, > - TCGArg arg1, TCGArg arg2, int const_arg2, > - int label_index, int arch64) > +static void tcg_out_brcond(TCGContext *s, TCGCond cond, > + TCGArg arg1, TCGArg arg2, int const_arg2, > + int label_index, TCGType type) > { > - tcg_out_cmp (s, cond, arg1, arg2, const_arg2, 7, arch64); > - tcg_out_bc (s, tcg_to_bc[cond], label_index); > + tcg_out_cmp(s, cond, arg1, arg2, const_arg2, 7, type); > + tcg_out_bc(s, tcg_to_bc[cond], label_index); > } > > void ppc_tb_set_jmp_target (unsigned long jmp_addr, unsigned long addr) > @@ -1532,11 +1530,13 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, > break; > > case INDEX_op_brcond_i32: > - tcg_out_brcond (s, args[2], args[0], args[1], const_args[1], args[3], 0); > + tcg_out_brcond(s, args[2], args[0], args[1], const_args[1], > + args[3], TCG_TYPE_I32); > break; > > case INDEX_op_brcond_i64: > - tcg_out_brcond (s, args[2], args[0], args[1], const_args[1], args[3], 1); > + tcg_out_brcond(s, args[2], args[0], args[1], const_args[1], > + args[3], TCG_TYPE_I64); > break; > > case INDEX_op_neg_i32: > -- > 1.8.1.2 > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net