From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50280) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aynJs-0002cs-SW for qemu-devel@nongnu.org; Fri, 06 May 2016 17:30:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aynJg-0007Ft-M4 for qemu-devel@nongnu.org; Fri, 06 May 2016 17:29:51 -0400 Received: from mail-qk0-x241.google.com ([2607:f8b0:400d:c09::241]:35139) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aynJf-0007AB-Di for qemu-devel@nongnu.org; Fri, 06 May 2016 17:29:44 -0400 Received: by mail-qk0-x241.google.com with SMTP id z3so7127617qkb.2 for ; Fri, 06 May 2016 14:29:29 -0700 (PDT) Sender: Richard Henderson References: <1462392752-17703-1-git-send-email-laurent@vivier.eu> <1462396135-20925-1-git-send-email-laurent@vivier.eu> <1462396135-20925-5-git-send-email-laurent@vivier.eu> From: Richard Henderson Message-ID: <9aa78302-3cec-aea0-15b1-f3c19148fc54@twiddle.net> Date: Fri, 6 May 2016 11:29:17 -1000 MIME-Version: 1.0 In-Reply-To: <1462396135-20925-5-git-send-email-laurent@vivier.eu> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 37/52] target-m68k: add cas/cas2 ops List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier , qemu-devel@nongnu.org Cc: schwab@linux-m68k.org, gerg@uclinux.org, agraf@suse.de On 05/04/2016 11:08 AM, Laurent Vivier wrote: > Signed-off-by: Laurent Vivier > --- > linux-user/main.c | 193 ++++++++++++++++++++++++++++++++++++++++++++++++ > target-m68k/cpu.h | 9 +++ > target-m68k/qregs.def | 5 ++ > target-m68k/translate.c | 175 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 382 insertions(+) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 74b02c7..3c51afe 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -2994,6 +2994,194 @@ void cpu_loop(CPUMBState *env) > > #ifdef TARGET_M68K > > +static int do_cas(CPUM68KState *env) > +{ > + int size, is_cas; > + int cmp1_reg, upd1_reg; > + int cmp2_reg, upd2_reg; > + uint32_t dest1, cmp1, addr1; > + uint32_t dest2, cmp2, addr2; > + int segv = 0; > + int z; > + > + start_exclusive(); > + > + /* cas_param bits > + * 31 -> CAS(0) / CAS2(1) > + * 11:13 -> update reg 2 > + * 8:10 -> cmp reg 2 > + * 5:7 -> update reg 1 > + * 2:4 -> cmp reg 1 > + * 0:1 -> opsize > + */ > + > + is_cas = (env->cas_param & 0x80000000) == 0; > + > + size = env->cas_param & 0x3; > + > + cmp1_reg = (env->cas_param >> 2) & 7; > + upd1_reg = (env->cas_param >> 5) & 7; > + cmp2_reg = (env->cas_param >> 8) & 7; > + upd2_reg = (env->cas_param >> 11) & 7; > + > + addr1 = env->cas_addr1; > + addr2 = env->cas_addr2; > + > + switch (size) { > + case OS_BYTE: > + segv = get_user_u8(dest1, addr1); > + cmp1 = (uint8_t)env->dregs[cmp1_reg]; > + break; > + case OS_WORD: > + segv = get_user_u16(dest1, addr1); > + cmp1 = (uint16_t)env->dregs[cmp1_reg]; > + break; > + case OS_LONG: > + default: > + segv = get_user_u32(dest1, addr1); > + cmp1 = env->dregs[cmp1_reg]; > + break; > + } > + if (segv) { > + env->mmu.ar = addr1; > + goto done; > + } > + env->cc_n = dest1; > + env->cc_v = cmp1; > + z = dest1 - cmp1; > + env->cc_op = CC_OP_CMPB + size; Incorrect setting of CC_* before checking for all of the possible segv? > + dest = gen_load(s, opsize, addr, 0); > + > + zero = tcg_const_i32(0); > + cmp = gen_extend(DREG(ext, 0), opsize, 0); > + > + /* if dest - cmp == 0 */ > + > + res = tcg_temp_new(); > + tcg_gen_sub_i32(res, dest, cmp); > + > + /* then dest = update1 */ > + > + tcg_gen_movcond_i32(TCG_COND_EQ, dest, > + res, zero, > + DREG(ext, 6), dest); > + > + /* else cmp = dest */ > + > + tcg_gen_movcond_i32(TCG_COND_NE, cmp, > + res, zero, > + dest, cmp); Note that you don't need movcond for cmp here. If the condition is false, we know that cmp == dest anyway. So just a plain store into cmp. You also don't need to compute RES, since you can perform the first movcond based on dest and cmp directly. Although this does require an extra temp: load = gen_load(...) cmp = gen_extend(...) store = tcg_temp_new(); tcg_gen_movcond_i32(TCG_COND_EQ, store, load, cmp, DREG(ext, 6), load); tcg_gen_mov_i32(cmp, load); gen_partset_reg(..., cmp); gen_store(..., store); > + gen_logic_cc(s, res, opsize); gen_logic_cc is wrong, afaics, and clobbers the proper CMPB + opsize that you set up earlier. > + TCGv cmp1, cmp2; > + TCGv dest1, dest2; > + TCGv res1, res2; > + TCGv zero; > + zero = tcg_const_i32(0); > + dest1 = gen_load(s, opsize, addr1, 0); > + cmp1 = gen_extend(DREG(ext1, 0), opsize, 0); > + > + res1 = tcg_temp_new(); > + tcg_gen_sub_i32(res1, dest1, cmp1); > + dest2 = gen_load(s, opsize, addr2, 0); > + cmp2 = gen_extend(DREG(ext2, 0), opsize, 0); > + > + res2 = tcg_temp_new(); > + tcg_gen_sub_i32(res2, dest2, cmp2); > + > + /* if dest1 - cmp1 == 0 and dest2 - cmp2 == 0 */ > + > + tcg_gen_movcond_i32(TCG_COND_EQ, res1, > + res1, zero, > + res2, res1); z = (cmp1 - load1) | (cmp2 - load2) would be a better computation here than sub+sub+movcond. Of course, we also need to correctly compute N, C and V, so... N = (cmp1 == load1 ? cmp2 : cmp1); V = (cmp1 == load1 ? load2 : load1); cc_op = CC_OP_CMP; store1 = (N == V ? update1 : load1); store2 = (N == V ? update2 : load2); cmp1 = load1; cmp2 = load2; Oh, and we need to think about delaying all flag and register updates until after the stores, for both CAS and CAS2. Otherwise we don't get the right results at the exception handler for a write-protected page. r~