[PATCH 0/2] RX target update

[PATCH 0/2] RX target update

[Yoshinori Sato]

I found some problem in RX target.
This patches fix it.

Yoshinori Sato (2):
  target/rx: Fix helper definiton.
  target/rx: gdbstub add acc register operation.

 target/rx/helper.h  | 36 ++++++++++++++++++------------------
 target/rx/gdbstub.c |  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

-- 
2.30.2

[PATCH 1/2] target/rx: Fix helper definiton.

[Yoshinori Sato]

Due to an incorrect definition of helper,
TCG optimization could sometimes behave unexpectedly.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 target/rx/helper.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/target/rx/helper.h b/target/rx/helper.h
index f0b7ebbbf7..efbca7a1b8 100644
--- a/target/rx/helper.h
+++ b/target/rx/helper.h
@@ -5,27 +5,27 @@ DEF_HELPER_1(wait, noreturn, env)
 DEF_HELPER_1(debug, noreturn, env)
 DEF_HELPER_2(rxint, noreturn, env, i32)
 DEF_HELPER_1(rxbrk, noreturn, env)
-DEF_HELPER_FLAGS_3(fadd, TCG_CALL_NO_WG, f32, env, f32, f32)
-DEF_HELPER_FLAGS_3(fsub, TCG_CALL_NO_WG, f32, env, f32, f32)
-DEF_HELPER_FLAGS_3(fmul, TCG_CALL_NO_WG, f32, env, f32, f32)
-DEF_HELPER_FLAGS_3(fdiv, TCG_CALL_NO_WG, f32, env, f32, f32)
-DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_WG, void, env, f32, f32)
-DEF_HELPER_FLAGS_2(ftoi, TCG_CALL_NO_WG, i32, env, f32)
-DEF_HELPER_FLAGS_2(round, TCG_CALL_NO_WG, i32, env, f32)
-DEF_HELPER_FLAGS_2(itof, TCG_CALL_NO_WG, f32, env, i32)
+DEF_HELPER_3(fadd, f32, env, f32, f32)
+DEF_HELPER_3(fsub, f32, env, f32, f32)
+DEF_HELPER_3(fmul, f32, env, f32, f32)
+DEF_HELPER_3(fdiv, f32, env, f32, f32)
+DEF_HELPER_3(fcmp, void, env, f32, f32)
+DEF_HELPER_2(ftoi, i32, env, f32)
+DEF_HELPER_2(round, i32, env, f32)
+DEF_HELPER_2(itof, f32, env, i32)
 DEF_HELPER_2(set_fpsw, void, env, i32)
-DEF_HELPER_FLAGS_2(racw, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_FLAGS_2(set_psw_rte, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_FLAGS_2(set_psw, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_1(pack_psw, i32, env)
-DEF_HELPER_FLAGS_3(div, TCG_CALL_NO_WG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(divu, TCG_CALL_NO_WG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_1(scmpu, TCG_CALL_NO_WG, void, env)
+DEF_HELPER_2(racw, void, env, i32)
+DEF_HELPER_2(set_psw_rte, void, env, i32)
+DEF_HELPER_2(set_psw, void, env, i32)
+DEF_HELPER_FLAGS_1(pack_psw, TCG_CALL_NO_WG, i32, env)
+DEF_HELPER_3(div,  i32, env, i32, i32)
+DEF_HELPER_3(divu, i32, env, i32, i32)
+DEF_HELPER_1(scmpu, void, env)
 DEF_HELPER_1(smovu, void, env)
 DEF_HELPER_1(smovf, void, env)
 DEF_HELPER_1(smovb, void, env)
 DEF_HELPER_2(sstr, void, env, i32)
-DEF_HELPER_FLAGS_2(swhile, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_FLAGS_2(suntil, TCG_CALL_NO_WG, void, env, i32)
-DEF_HELPER_FLAGS_2(rmpa, TCG_CALL_NO_WG, void, env, i32)
+DEF_HELPER_2(swhile, void, env, i32)
+DEF_HELPER_2(suntil, void, env, i32)
+DEF_HELPER_2(rmpa, void, env, i32)
 DEF_HELPER_1(satr, void, env)
-- 
2.30.2

Re: [PATCH 1/2] target/rx: Fix helper definiton.

[Peter Maydell]

On Thu, 9 Sept 2021 at 13:10, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
>
> Due to an incorrect definition of helper,
> TCG optimization could sometimes behave unexpectedly.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Could you give more detail, please?

I had a look at one or two -- it looks like the floating point
related helpers like fadd write to env->fpsw. That is a TCG global,
but we only use it in the implementation of the "move from FPSW
to a general-purpose register" instruction. So I think the
better fix for that case would be to stop defining fpsw as
a TCG global and instead make the "move from FPSW" insn
use tcg_gen_ld_i32() to read from env->fpsw.

Probably doing the same thing for every helper function is
not right -- some will be better handled by reducing
the use of TCG globals, and some will indeed need to have
the TCG_CALL_NO_WG flag removed.

thanks
-- PMM

[PATCH 2/2] target/rx: gdbstub add acc register operation.

[Yoshinori Sato]

I added it because the operation of the acc register was not implemented.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 target/rx/gdbstub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/rx/gdbstub.c b/target/rx/gdbstub.c
index c811d4810b..b5da5c42cb 100644
--- a/target/rx/gdbstub.c
+++ b/target/rx/gdbstub.c
@@ -47,7 +47,7 @@ int rx_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
     case 24:
         return gdb_get_regl(mem_buf, env->fpsw);
     case 25:
-        return 0;
+        return gdb_get_reg64(mem_buf, env->acc);
     }
     return 0;
 }
@@ -103,6 +103,7 @@ int rx_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         env->fpsw = ldl_p(mem_buf);
         break;
     case 25:
+        env->acc = ldq_p(mem_buf);
         return 8;
     default:
         return 0;
-- 
2.30.2

Re: [PATCH 2/2] target/rx: gdbstub add acc register operation.

[Peter Maydell]

On Thu, 9 Sept 2021 at 13:08, Yoshinori Sato <ysato@users.sourceforge.jp> wrote:
>
> I added it because the operation of the acc register was not implemented.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  target/rx/gdbstub.c | 3 ++-

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM