From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44140) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1blws4-0007yX-5w for qemu-devel@nongnu.org; Mon, 19 Sep 2016 07:36:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1blwry-0006nE-7v for qemu-devel@nongnu.org; Mon, 19 Sep 2016 07:36:23 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:61216) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1blwry-0006mF-2F for qemu-devel@nongnu.org; Mon, 19 Sep 2016 07:36:18 -0400 Date: Mon, 19 Sep 2016 12:35:58 +0100 From: Leon Alrae Message-ID: <20160919113557.GA3262@hhmipssw201.hh.imgtec.org> References: <1473929062-5548-1-git-send-email-leon.alrae@imgtec.com> <1473929062-5548-3-git-send-email-leon.alrae@imgtec.com> <50da89d8-bc60-13f8-47fc-f69b40c13b58@twiddle.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <50da89d8-bc60-13f8-47fc-f69b40c13b58@twiddle.net> Subject: Re: [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org, aurelien@aurel32.net On Fri, Sep 16, 2016 at 09:48:51AM -0700, Richard Henderson wrote: > On 09/15/2016 01:44 AM, Leon Alrae wrote: > > /* Store conditional */ > >+static void gen_st_cond(DisasContext *ctx, int rt, int base, int offset, > >+ int size) > > { > >+ TCGv addr, t0, val; > >+ TCGLabel *l1 = gen_new_label(); > >+ TCGLabel *l2 = gen_new_label(); > >+ TCGLabel *done = gen_new_label(); > > > >-#ifdef CONFIG_USER_ONLY > > t0 = tcg_temp_local_new(); > >+ addr = tcg_temp_local_new(); > >+ /* check the alignment of the address */ > >+ gen_base_offset_addr(ctx, addr, base, offset); > >+ tcg_gen_andi_tl(t0, addr, size - 1); > > You shouldn't have to test the alignment here, as the alignment > should have been tested during the load-locked, and the (aligned) > address will be compared. This is to satisfy the requirement that unaligned SC generates Address Error exception. But I agree that in practice this doesn't seem particularly useful since LL will do that. > > > >+ /* compare the address against that of the preceeding LL */ > >+ tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l2); > >+ tcg_gen_movi_tl(t0, 0); > >+ tcg_gen_br(done); > ... > >+#ifdef TARGET_MIPS64 > >+ case 8: /* SCD */ > >+ tcg_gen_atomic_cmpxchg_i64(t0, addr, cpu_llval, val, > >+ ctx->mem_idx, MO_TEQ); > > break; > > #endif > >- case OPC_SC: > >- case R6_OPC_SC: > >- op_st_sc(t1, t0, rt, ctx); > >+ case 4: /* SC */ > >+ { > >+ TCGv_i32 val32 = tcg_temp_new_i32(); > >+ TCGv_i32 llval32 = tcg_temp_new_i32(); > >+ TCGv_i32 old32 = tcg_temp_new_i32(); > >+ tcg_gen_trunc_tl_i32(val32, val); > >+ tcg_gen_trunc_tl_i32(llval32, cpu_llval); > >+ > >+ tcg_gen_atomic_cmpxchg_i32(old32, addr, llval32, val32, > >+ ctx->mem_idx, MO_TESL); > >+ tcg_gen_ext_i32_tl(t0, old32); > > You can use tcg_gen_atomic_cmpxchg_tl so that you do not need to do > all of this truncation yourself. Which means that if you replace > the size parameter with a TCGMemOp parameter (MO_TEQ vs MO_TESL) you > can make all this code common. Ah, yes. > > Further, local temporaries are less than ideal and should be avoided > if possible. Using them results in an extra store into the local > stack frame. > > We can avoid this for addr by noting that once you have compared > addr to cpu_lladdr, you can free addr and use cpu_lladdr in the > actual cmpxchg. Ok. I'll correct in v2. Thanks, Leon