From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkwJx-0001IQ-D4 for qemu-devel@nongnu.org; Fri, 16 Sep 2016 12:49:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bkwJs-0004IC-AH for qemu-devel@nongnu.org; Fri, 16 Sep 2016 12:49:01 -0400 Received: from mail-yb0-x22d.google.com ([2607:f8b0:4002:c09::22d]:36131) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bkwJs-0004Hi-4e for qemu-devel@nongnu.org; Fri, 16 Sep 2016 12:48:56 -0400 Received: by mail-yb0-x22d.google.com with SMTP id u125so54775110ybg.3 for ; Fri, 16 Sep 2016 09:48:55 -0700 (PDT) Sender: Richard Henderson References: <1473929062-5548-1-git-send-email-leon.alrae@imgtec.com> <1473929062-5548-3-git-send-email-leon.alrae@imgtec.com> From: Richard Henderson Message-ID: <50da89d8-bc60-13f8-47fc-f69b40c13b58@twiddle.net> Date: Fri, 16 Sep 2016 09:48:51 -0700 MIME-Version: 1.0 In-Reply-To: <1473929062-5548-3-git-send-email-leon.alrae@imgtec.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Leon Alrae , qemu-devel@nongnu.org Cc: aurelien@aurel32.net 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. > + /* 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. 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. r~