From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56405) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPZ8w-0008TQ-Qa for qemu-devel@nongnu.org; Wed, 12 Aug 2015 12:44:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPZ8r-00019p-AA for qemu-devel@nongnu.org; Wed, 12 Aug 2015 12:44:46 -0400 Received: from mail-qg0-x236.google.com ([2607:f8b0:400d:c04::236]:32864) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPZ8r-00019k-5g for qemu-devel@nongnu.org; Wed, 12 Aug 2015 12:44:41 -0400 Received: by qged69 with SMTP id d69so14250753qge.0 for ; Wed, 12 Aug 2015 09:44:40 -0700 (PDT) Sender: Richard Henderson References: <1439151229-27747-1-git-send-email-laurent@vivier.eu> <1439151229-27747-18-git-send-email-laurent@vivier.eu> From: Richard Henderson Message-ID: <55CB77F5.4070603@twiddle.net> Date: Wed, 12 Aug 2015 09:44:37 -0700 MIME-Version: 1.0 In-Reply-To: <1439151229-27747-18-git-send-email-laurent@vivier.eu> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.5 17/30] m68k: ori/andi/subi/addi/eori/cmpi can modify SR/CCR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, Andreas Schwab , gerg@uclinux.org On 08/09/2015 01:13 PM, Laurent Vivier wrote: > Signed-off-by: Laurent Vivier > --- > target-m68k/translate.c | 95 ++++++++++++++++++++++++++++++------------------- > 1 file changed, 58 insertions(+), 37 deletions(-) > > diff --git a/target-m68k/translate.c b/target-m68k/translate.c > index 6a426e1..9e379b3 100644 > --- a/target-m68k/translate.c > +++ b/target-m68k/translate.c > @@ -1343,6 +1343,42 @@ DISAS_INSN(bitop_im) > DEST_EA(env, insn, opsize, tmp, &addr); > } > } > + > +static TCGv gen_get_ccr(DisasContext *s) > +{ > + TCGv dest; > + > + gen_flush_flags(s); > + dest = tcg_temp_new(); > + tcg_gen_shli_i32(dest, QREG_CC_X, 4); > + tcg_gen_or_i32(dest, dest, QREG_CC_DEST); > + return dest; > +} > + > +static TCGv gen_get_sr(DisasContext *s) > +{ > + TCGv ccr; > + TCGv sr; > + > + ccr = gen_get_ccr(s); > + sr = tcg_temp_new(); > + tcg_gen_andi_i32(sr, QREG_SR, 0xffe0); > + tcg_gen_or_i32(sr, sr, ccr); > + return sr; > +} Leaking the temporary produced by gen_get_ccr. > + > +static void gen_set_sr(DisasContext *s, TCGv val, int ccr_only) > +{ > + TCGv tmp; > + tmp = tcg_temp_new(); > + tcg_gen_andi_i32(QREG_CC_DEST, val, 0xf); > + tcg_gen_shri_i32(tmp, val, 4); > + tcg_gen_andi_i32(QREG_CC_X, tmp, 1); > + if (!ccr_only) { > + gen_helper_set_sr(cpu_env, val); > + } > +} Leaking tmp. And you don't even need to allocate it -- perform the shift into QREG_CC_X. > + > DISAS_INSN(arith_im) > { > int op; > @@ -1367,7 +1403,20 @@ DISAS_INSN(arith_im) > default: > abort(); > } > - SRC_EA(env, src1, opsize, -1, (op == 6) ? NULL : &addr); > + if ((op == 0 || op == 1) && The subject line is surely misleading, as this is only ANDI/ORI, right? Again, some more commentary would be helpful. > + (insn & 0x3f) == 0x3c) { > + if (opsize == OS_BYTE) { > + src1 = gen_get_ccr(s); > + } else { > + if (IS_USER(s)) { > + gen_exception(s, s->pc - 2, EXCP_PRIVILEGE); > + return; > + } > + src1 = gen_get_sr(s); > + } > + } else { > + SRC_EA(env, src1, opsize, -1, (op == 6) ? NULL : &addr); > + } > dest = tcg_temp_new(); > switch (op) { > case 0: /* ori */ > @@ -1405,7 +1454,14 @@ DISAS_INSN(arith_im) > default: > abort(); > } > - DEST_EA(env, insn, opsize, dest, &addr); > + if (op != 6) { > + if ((op == 0 || op == 1) && > + (insn & 0x3f) == 0x3c) { > + gen_set_sr(s, dest, opsize == OS_BYTE); > + } else { > + DEST_EA(env, insn, opsize, dest, &addr); > + } > + } That said, I think this code should be rearranged so that you don't have to replicate so many conditionals. In particular, the only thing of use in the middle of import for the ccr insns are two lines: tcg_gen_andi/ori_tl. I think it would be better to structure as if ((insn & 0x3f) == 0x3c && (op == 0 || op == 1)) { if (opsize == OS_BYTE) { src1 = gen_get_ccr (); } else { ... } if (op == 0) { tcg_gen_ori_i32 ... } else { tcg_gen_andi_i32 ... } gen_set_sr(s, dest, opsize == OS_BYTE); return; } // existing code r~