diff mbox

target-sh4: add atomic tas

Message ID 1478178763-12189-1-git-send-email-laurent@vivier.eu
State New
Headers show

Commit Message

Laurent Vivier Nov. 3, 2016, 1:12 p.m. UTC
Implement real atomic tas.

As tas is:

    When (Rn) = 0, 1 -> T
    Otherwise, 0 -> T
    In both cases, 1 -> MSB of (Rn)

It can't be implemented using cmpxchg().

So we use an helper and "parallel_cpus"+cpu_loop_exit_atomic().

Tested with image from:
http://wiki.qemu.org/download/sh-test-0.2.tar.bz2

This image contains a "tas_test" that runs without
error with this change.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target-sh4/helper.h    |  1 +
 target-sh4/op_helper.c | 24 ++++++++++++++++++++++++
 target-sh4/translate.c | 13 +------------
 3 files changed, 26 insertions(+), 12 deletions(-)

Comments

Paolo Bonzini Nov. 3, 2016, 1:29 p.m. UTC | #1
On 03/11/2016 14:12, Laurent Vivier wrote:
> Implement real atomic tas.
> 
> As tas is:
> 
>     When (Rn) = 0, 1 -> T
>     Otherwise, 0 -> T
>     In both cases, 1 -> MSB of (Rn)
> 
> It can't be implemented using cmpxchg().

Why not?

This is essentially

    movi		temp0, 0x80
    atomic_fetch_or_i32 temp1, Rn, temp0, ctx->memidx, MO_UB
    setcondi_i32	cpu_sr_t, TCG_COND_EQ, temp1, 0

But the patch looks good anyway.

Paolo

> So we use an helper and "parallel_cpus"+cpu_loop_exit_atomic().
> 
> Tested with image from:
> http://wiki.qemu.org/download/sh-test-0.2.tar.bz2
> 
> This image contains a "tas_test" that runs without
> error with this change.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  target-sh4/helper.h    |  1 +
>  target-sh4/op_helper.c | 24 ++++++++++++++++++++++++
>  target-sh4/translate.c | 13 +------------
>  3 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/target-sh4/helper.h b/target-sh4/helper.h
> index dce859c..4bfa98c 100644
> --- a/target-sh4/helper.h
> +++ b/target-sh4/helper.h
> @@ -6,6 +6,7 @@ DEF_HELPER_1(raise_slot_fpu_disable, noreturn, env)
>  DEF_HELPER_1(debug, noreturn, env)
>  DEF_HELPER_1(sleep, noreturn, env)
>  DEF_HELPER_2(trapa, noreturn, env, i32)
> +DEF_HELPER_2(tas, void, env, i32)
>  
>  DEF_HELPER_3(movcal, void, env, i32, i32)
>  DEF_HELPER_1(discard_movcal_backup, void, env)
> diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
> index 684d3f3..17afa92 100644
> --- a/target-sh4/op_helper.c
> +++ b/target-sh4/op_helper.c
> @@ -105,6 +105,30 @@ void helper_trapa(CPUSH4State *env, uint32_t tra)
>      raise_exception(env, 0x160, 0);
>  }
>  
> +void helper_tas(CPUSH4State *env, uint32_t addr)
> +{
> +    uint8_t l;
> +    uintptr_t ra = GETPC();
> +
> +    if (parallel_cpus) {
> +        /* Tell the main loop we need to serialize this insn.  */
> +        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
> +    } else {
> +        /* We're executing in a serial context -- no need to be atomic.  */
> +#ifdef CONFIG_USER_ONLY
> +        uint8_t *haddr = g2h(addr);
> +        l = ldub_p(haddr);
> +        stb_p(haddr, l | 0x80);
> +#else
> +        int mmu_idx = cpu_mmu_index(env, 0);
> +        TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
> +        l = helper_ret_ldub_mmu(env, addr, oi, ra);
> +        helper_ret_stb_mmu(env, addr,  l | 0x80, oi, ra);
> +#endif
> +        env->sr_t = l == 0;
> +    }
> +}
> +
>  void helper_movcal(CPUSH4State *env, uint32_t address, uint32_t value)
>  {
>      if (cpu_sh4_is_cached (env, address))
> diff --git a/target-sh4/translate.c b/target-sh4/translate.c
> index c89a147..44238fc 100644
> --- a/target-sh4/translate.c
> +++ b/target-sh4/translate.c
> @@ -1640,18 +1640,7 @@ static void _decode_opc(DisasContext * ctx)
>  	tcg_gen_shri_i32(REG(B11_8), REG(B11_8), 16);
>  	return;
>      case 0x401b:		/* tas.b @Rn */
> -	{
> -	    TCGv addr, val;
> -	    addr = tcg_temp_local_new();
> -	    tcg_gen_mov_i32(addr, REG(B11_8));
> -	    val = tcg_temp_local_new();
> -            tcg_gen_qemu_ld_i32(val, addr, ctx->memidx, MO_UB);
> -            tcg_gen_setcondi_i32(TCG_COND_EQ, cpu_sr_t, val, 0);
> -	    tcg_gen_ori_i32(val, val, 0x80);
> -            tcg_gen_qemu_st_i32(val, addr, ctx->memidx, MO_UB);
> -	    tcg_temp_free(val);
> -	    tcg_temp_free(addr);
> -	}
> +        gen_helper_tas(cpu_env, REG(B11_8));
>  	return;
>      case 0xf00d: /* fsts FPUL,FRn - FPSCR: Nothing */
>  	CHECK_FPU_ENABLED
>
Laurent Vivier Nov. 3, 2016, 1:32 p.m. UTC | #2
Le 03/11/2016 à 14:29, Paolo Bonzini a écrit :
> 
> 
> On 03/11/2016 14:12, Laurent Vivier wrote:
>> Implement real atomic tas.
>>
>> As tas is:
>>
>>     When (Rn) = 0, 1 -> T
>>     Otherwise, 0 -> T
>>     In both cases, 1 -> MSB of (Rn)
>>
>> It can't be implemented using cmpxchg().
> 
> Why not?

Only because I don't know :)

> This is essentially
> 
>     movi		temp0, 0x80
>     atomic_fetch_or_i32 temp1, Rn, temp0, ctx->memidx, MO_UB
>     setcondi_i32	cpu_sr_t, TCG_COND_EQ, temp1, 0
> 
> But the patch looks good anyway.

I'm going to rewrite it as you say...

Thanks,
Laurent
diff mbox

Patch

diff --git a/target-sh4/helper.h b/target-sh4/helper.h
index dce859c..4bfa98c 100644
--- a/target-sh4/helper.h
+++ b/target-sh4/helper.h
@@ -6,6 +6,7 @@  DEF_HELPER_1(raise_slot_fpu_disable, noreturn, env)
 DEF_HELPER_1(debug, noreturn, env)
 DEF_HELPER_1(sleep, noreturn, env)
 DEF_HELPER_2(trapa, noreturn, env, i32)
+DEF_HELPER_2(tas, void, env, i32)
 
 DEF_HELPER_3(movcal, void, env, i32, i32)
 DEF_HELPER_1(discard_movcal_backup, void, env)
diff --git a/target-sh4/op_helper.c b/target-sh4/op_helper.c
index 684d3f3..17afa92 100644
--- a/target-sh4/op_helper.c
+++ b/target-sh4/op_helper.c
@@ -105,6 +105,30 @@  void helper_trapa(CPUSH4State *env, uint32_t tra)
     raise_exception(env, 0x160, 0);
 }
 
+void helper_tas(CPUSH4State *env, uint32_t addr)
+{
+    uint8_t l;
+    uintptr_t ra = GETPC();
+
+    if (parallel_cpus) {
+        /* Tell the main loop we need to serialize this insn.  */
+        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+    } else {
+        /* We're executing in a serial context -- no need to be atomic.  */
+#ifdef CONFIG_USER_ONLY
+        uint8_t *haddr = g2h(addr);
+        l = ldub_p(haddr);
+        stb_p(haddr, l | 0x80);
+#else
+        int mmu_idx = cpu_mmu_index(env, 0);
+        TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
+        l = helper_ret_ldub_mmu(env, addr, oi, ra);
+        helper_ret_stb_mmu(env, addr,  l | 0x80, oi, ra);
+#endif
+        env->sr_t = l == 0;
+    }
+}
+
 void helper_movcal(CPUSH4State *env, uint32_t address, uint32_t value)
 {
     if (cpu_sh4_is_cached (env, address))
diff --git a/target-sh4/translate.c b/target-sh4/translate.c
index c89a147..44238fc 100644
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -1640,18 +1640,7 @@  static void _decode_opc(DisasContext * ctx)
 	tcg_gen_shri_i32(REG(B11_8), REG(B11_8), 16);
 	return;
     case 0x401b:		/* tas.b @Rn */
-	{
-	    TCGv addr, val;
-	    addr = tcg_temp_local_new();
-	    tcg_gen_mov_i32(addr, REG(B11_8));
-	    val = tcg_temp_local_new();
-            tcg_gen_qemu_ld_i32(val, addr, ctx->memidx, MO_UB);
-            tcg_gen_setcondi_i32(TCG_COND_EQ, cpu_sr_t, val, 0);
-	    tcg_gen_ori_i32(val, val, 0x80);
-            tcg_gen_qemu_st_i32(val, addr, ctx->memidx, MO_UB);
-	    tcg_temp_free(val);
-	    tcg_temp_free(addr);
-	}
+        gen_helper_tas(cpu_env, REG(B11_8));
 	return;
     case 0xf00d: /* fsts FPUL,FRn - FPSCR: Nothing */
 	CHECK_FPU_ENABLED