Re: [Qemu-devel] [PATCH v2 3/6] target/mips: Fix HI[ac] and LO[ac] 32-bit truncation with MIPS64 DSP ASE

[Aleksandar Markovic]

> From: Fredrik Noring <address@hidden>
> Sent: Wednesday, November 7, 2018 8:18 PM
> To: Aleksandar Markovic; Aurelien Jarno; Philippe Mathieu-Daudé; Richard 
> Henderson
> Cc: Jürgen Urban; Maciej W. Rozycki; Jia Liu; address@hidden
> Subject: [PATCH v2 3/6] target/mips: Fix HI[ac] and LO[ac] 32-bit truncation 
> with > MIPS64 DSP ASE
> 
> This change removes the 32-bit truncation of the HI[ac] and LO[ac]
> special purpose registers when ac range from 1 to 3 for the instructions
> MFHI, MFLO, MTHI and MTLO. The "MIPS Architecture for Programmers Volume
> IV-e: MIPS DSP Module for MIPS64 Architecture" manual specifies that all
> 64 bits are copied in all cases:
> 
>    MFHI: GPR[rd]63..0 <- HI[ac]63..0
>    MFLO: GPR[rd]63..0 <- LO[ac]63..0
>    MTHI: HI[ac]63..0 <- GPR[rs]63..0
>    MTLO: LO[ac]63..0 <- GPR[rs]63..0
> 
> Fixes: 4133498f8e53 ("Use correct acc value to index cpu_HI/cpu_LO rather 
> than using > a fix number")
> Cc: Jia Liu <address@hidden>
> Reported-by: Maciej W. Rozycki <address@hidden>
> Signed-off-by: Fredrik Noring <address@hidden>
> ---
>  target/mips/translate.c | 36 ++++--------------------------------
>  1 file changed, 4 insertions(+), 32 deletions(-)
> 

Hi, Fredrik.

Thanks for this follow-up patch. I will consider it, even if I perhaps don't 
include it for 3.1. Let's see if Jia has some comments.

Aleksandar

> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 3ddd70043a..19ae7d2f1c 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -4409,49 +4409,21 @@ static void gen_HILO(DisasContext *ctx, uint32_t opc, 
> int > acc, int reg)
> 
>      switch (opc) {
>      case OPC_MFHI:
> -#if defined(TARGET_MIPS64)
> -        if (acc != 0) {
> -            tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[acc]);
> -        } else
> -#endif
> -        {
> -            tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[acc]);
> -        }
> +        tcg_gen_mov_tl(cpu_gpr[reg], cpu_HI[acc]);
>          break;
>      case OPC_MFLO:
> -#if defined(TARGET_MIPS64)
> -        if (acc != 0) {
> -            tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_LO[acc]);
> -        } else
> -#endif
> -        {
> -            tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[acc]);
> -        }
> +        tcg_gen_mov_tl(cpu_gpr[reg], cpu_LO[acc]);
>          break;
>      case OPC_MTHI:
>          if (reg != 0) {
> -#if defined(TARGET_MIPS64)
> -            if (acc != 0) {
> -                tcg_gen_ext32s_tl(cpu_HI[acc], cpu_gpr[reg]);
> -            } else
> -#endif
> -            {
> -                tcg_gen_mov_tl(cpu_HI[acc], cpu_gpr[reg]);
> -            }
> +            tcg_gen_mov_tl(cpu_HI[acc], cpu_gpr[reg]);
>          } else {
>              tcg_gen_movi_tl(cpu_HI[acc], 0);
>          }
>          break;
>      case OPC_MTLO:
>          if (reg != 0) {
> -#if defined(TARGET_MIPS64)
> -            if (acc != 0) {
> -                tcg_gen_ext32s_tl(cpu_LO[acc], cpu_gpr[reg]);
> -            } else
> -#endif
> -            {
> -                tcg_gen_mov_tl(cpu_LO[acc], cpu_gpr[reg]);
> -            }
> +            tcg_gen_mov_tl(cpu_LO[acc], cpu_gpr[reg]);
>          } else {
>              tcg_gen_movi_tl(cpu_LO[acc], 0);
>          }
> --
> 2.18.1
> 
>