Re: [PATCH 2/2] target/sh4: fixup tcg for sh4 fipr/ftrv instructions

[email protected] Tue, 30 Jun 2026 21:43:35 +0900
Newsgroups org.nongnu.qemu-trivial,org.nongnu.qemu-devel
Message-ID <[email protected]>
On Tue, 30 Jun 2026 06:49:13 +0900,
Randy Schifflin wrote:
> 
> Fixes TCG generation for sh4 `fipr` and `ftrv` instructions.
> Updates the current logic for these instructions to check the
> FPSCR register appropriately (according to the sh4 cpu manual, `fipr`
> and `ftrv` are only defined when the FPSCR register PR flag is 0).
> Also fixes the mth/nth-vector operands by multiplying by 4 to convert
> to the correct floating point register offset.
> 
> Signed-off-by: Randy Schifflin <[email protected]>
> ---
>  target/sh4/op_helper.c |  2 +-
>  target/sh4/translate.c | 15 +++++----------
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> index 90c065b217..777380f8e4 100644
> --- a/target/sh4/op_helper.c
> +++ b/target/sh4/op_helper.c
> @@ -488,7 +488,7 @@ void helper_ftrv(CPUSH4State *env, uint32_t n)
>      float32 p;
>  
>      bank_matrix = (env->sr & FPSCR_FR) ? 0 : 16;
> -    bank_vector = (env->sr & FPSCR_FR) ? 16 : 0;
> +    bank_vector = (env->sr & FPSCR_FR) ? 16 + n : n;
>      set_float_exception_flags(0, &env->fp_status);
>      for (i = 0 ; i < 4 ; i++) {
>          r[i] = float32_zero;
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index d38a6bd352..373950fd66 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -377,11 +377,6 @@ static inline void gen_store_fpr64(DisasContext *ctx, TCGv_i64 t, int reg)
>          goto do_illegal;                    \
>      }
>  
> -#define CHECK_FPSCR_PR_1 \
> -    if (!(ctx->tbflags & FPSCR_PR)) {       \
> -        goto do_illegal;                    \
> -    }
> -
>  #define CHECK_SH4A \
>      if (!(ctx->features & SH_FEATURE_SH4A)) { \
>          goto do_illegal;                      \
> @@ -1740,22 +1735,22 @@ static void _decode_opc(DisasContext * ctx)
>          return;
>      case 0xf0ed: /* fipr FVm,FVn */
>          CHECK_FPU_ENABLED
> -        CHECK_FPSCR_PR_1
> +        CHECK_FPSCR_PR_0
>          {
> -            TCGv m = tcg_constant_i32((ctx->opcode >> 8) & 3);
> -            TCGv n = tcg_constant_i32((ctx->opcode >> 10) & 3);
> +            TCGv m = tcg_constant_i32(((ctx->opcode >> 8) & 3) << 2);
> +            TCGv n = tcg_constant_i32(((ctx->opcode >> 10) & 3) << 2);
>              gen_helper_fipr(tcg_env, m, n);
>              return;
>          }
>          break;
>      case 0xf0fd: /* ftrv XMTRX,FVn */
>          CHECK_FPU_ENABLED
> -        CHECK_FPSCR_PR_1
> +        CHECK_FPSCR_PR_0
>          {
>              if ((ctx->opcode & 0x0300) != 0x0100) {
>                  goto do_illegal;
>              }
> -            TCGv n = tcg_constant_i32((ctx->opcode >> 10) & 3);
> +            TCGv n = tcg_constant_i32(((ctx->opcode >> 10) & 3) << 2);
>              gen_helper_ftrv(tcg_env, n);
>              return;
>          }
> 
> -- 
> 2.43.0
> 

On Tue, 30 Jun 2026 06:49:13 +0900,
Randy Schifflin wrote:
> 
> Fixes TCG generation for sh4 `fipr` and `ftrv` instructions.
> Updates the current logic for these instructions to check the
> FPSCR register appropriately (according to the sh4 cpu manual, `fipr`
> and `ftrv` are only defined when the FPSCR register PR flag is 0).
> Also fixes the mth/nth-vector operands by multiplying by 4 to convert
> to the correct floating point register offset.
> 
> Signed-off-by: Randy Schifflin <[email protected]>
> ---
>  target/sh4/op_helper.c |  2 +-
>  target/sh4/translate.c | 15 +++++----------
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> index 90c065b217..777380f8e4 100644
> --- a/target/sh4/op_helper.c
> +++ b/target/sh4/op_helper.c
> @@ -488,7 +488,7 @@ void helper_ftrv(CPUSH4State *env, uint32_t n)
>      float32 p;
>  
>      bank_matrix = (env->sr & FPSCR_FR) ? 0 : 16;
> -    bank_vector = (env->sr & FPSCR_FR) ? 16 : 0;
> +    bank_vector = (env->sr & FPSCR_FR) ? 16 + n : n;
>      set_float_exception_flags(0, &env->fp_status);
>      for (i = 0 ; i < 4 ; i++) {
>          r[i] = float32_zero;
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index d38a6bd352..373950fd66 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -377,11 +377,6 @@ static inline void gen_store_fpr64(DisasContext *ctx, TCGv_i64 t, int reg)
>          goto do_illegal;                    \
>      }
>  
> -#define CHECK_FPSCR_PR_1 \
> -    if (!(ctx->tbflags & FPSCR_PR)) {       \
> -        goto do_illegal;                    \
> -    }
> -
>  #define CHECK_SH4A \
>      if (!(ctx->features & SH_FEATURE_SH4A)) { \
>          goto do_illegal;                      \
> @@ -1740,22 +1735,22 @@ static void _decode_opc(DisasContext * ctx)
>          return;
>      case 0xf0ed: /* fipr FVm,FVn */
>          CHECK_FPU_ENABLED
> -        CHECK_FPSCR_PR_1
> +        CHECK_FPSCR_PR_0
>          {
> -            TCGv m = tcg_constant_i32((ctx->opcode >> 8) & 3);
> -            TCGv n = tcg_constant_i32((ctx->opcode >> 10) & 3);
> +            TCGv m = tcg_constant_i32(((ctx->opcode >> 8) & 3) << 2);
> +            TCGv n = tcg_constant_i32(((ctx->opcode >> 10) & 3) << 2);
>              gen_helper_fipr(tcg_env, m, n);
>              return;
>          }
>          break;
>      case 0xf0fd: /* ftrv XMTRX,FVn */
>          CHECK_FPU_ENABLED
> -        CHECK_FPSCR_PR_1
> +        CHECK_FPSCR_PR_0
>          {
>              if ((ctx->opcode & 0x0300) != 0x0100) {
>                  goto do_illegal;
>              }
> -            TCGv n = tcg_constant_i32((ctx->opcode >> 10) & 3);
> +            TCGv n = tcg_constant_i32(((ctx->opcode >> 10) & 3) << 2);
>              gen_helper_ftrv(tcg_env, n);
>              return;
>          }
> 
> -- 
> 2.43.0
> 

On Tue, 30 Jun 2026 06:49:13 +0900,
Randy Schifflin wrote:
> 
> Fixes TCG generation for sh4 `fipr` and `ftrv` instructions.
> Updates the current logic for these instructions to check the
> FPSCR register appropriately (according to the sh4 cpu manual, `fipr`
> and `ftrv` are only defined when the FPSCR register PR flag is 0).
> Also fixes the mth/nth-vector operands by multiplying by 4 to convert
> to the correct floating point register offset.
> 
> Signed-off-by: Randy Schifflin <[email protected]>
> ---
>  target/sh4/op_helper.c |  2 +-
>  target/sh4/translate.c | 15 +++++----------
>  2 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
> index 90c065b217..777380f8e4 100644
> --- a/target/sh4/op_helper.c
> +++ b/target/sh4/op_helper.c
> @@ -488,7 +488,7 @@ void helper_ftrv(CPUSH4State *env, uint32_t n)
>      float32 p;
>  
>      bank_matrix = (env->sr & FPSCR_FR) ? 0 : 16;
> -    bank_vector = (env->sr & FPSCR_FR) ? 16 : 0;
> +    bank_vector = (env->sr & FPSCR_FR) ? 16 + n : n;
>      set_float_exception_flags(0, &env->fp_status);
>      for (i = 0 ; i < 4 ; i++) {
>          r[i] = float32_zero;
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index d38a6bd352..373950fd66 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -377,11 +377,6 @@ static inline void gen_store_fpr64(DisasContext *ctx, TCGv_i64 t, int reg)
>          goto do_illegal;                    \
>      }
>  
> -#define CHECK_FPSCR_PR_1 \
> -    if (!(ctx->tbflags & FPSCR_PR)) {       \
> -        goto do_illegal;                    \
> -    }
> -
>  #define CHECK_SH4A \
>      if (!(ctx->features & SH_FEATURE_SH4A)) { \
>          goto do_illegal;                      \
> @@ -1740,22 +1735,22 @@ static void _decode_opc(DisasContext * ctx)
>          return;
>      case 0xf0ed: /* fipr FVm,FVn */
>          CHECK_FPU_ENABLED
> -        CHECK_FPSCR_PR_1
> +        CHECK_FPSCR_PR_0
>          {
> -            TCGv m = tcg_constant_i32((ctx->opcode >> 8) & 3);
> -            TCGv n = tcg_constant_i32((ctx->opcode >> 10) & 3);
> +            TCGv m = tcg_constant_i32(((ctx->opcode >> 8) & 3) << 2);
> +            TCGv n = tcg_constant_i32(((ctx->opcode >> 10) & 3) << 2);
>              gen_helper_fipr(tcg_env, m, n);
>              return;
>          }
>          break;
>      case 0xf0fd: /* ftrv XMTRX,FVn */
>          CHECK_FPU_ENABLED
> -        CHECK_FPSCR_PR_1
> +        CHECK_FPSCR_PR_0
>          {
>              if ((ctx->opcode & 0x0300) != 0x0100) {
>                  goto do_illegal;
>              }
> -            TCGv n = tcg_constant_i32((ctx->opcode >> 10) & 3);
> +            TCGv n = tcg_constant_i32(((ctx->opcode >> 10) & 3) << 2);
>              gen_helper_ftrv(tcg_env, n);
>              return;
>          }
> 
> -- 
> 2.43.0
> 

Reviewed-by: Yoshinori Sato <[email protected]>

-- 
Yosinori Sato