All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Richard Henderson <rth@twiddle.net>, qemu-devel@nongnu.org
Cc: bruno@clisp.org, aurelien@aurel32.net
Subject: Re: [Qemu-devel] [PATCH 07/11] target/sh4: Unify cpu_fregs into FREG
Date: Wed, 5 Jul 2017 22:55:38 -0300	[thread overview]
Message-ID: <09234b5f-c837-3cb0-ea27-449591753b42@amsat.org> (raw)
In-Reply-To: <20170706002401.10507-8-rth@twiddle.net>

On 07/05/2017 09:23 PM, Richard Henderson wrote:
> We were treating FREG as an index and REG as a TCGv.
> Making FREG return a TCGv is both less confusing and
> a step toward cleaner banking of cpu_fregs.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   target/sh4/translate.c | 123 +++++++++++++++++++++----------------------------
>   1 file changed, 52 insertions(+), 71 deletions(-)
> 
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 20e24d5..e4fd6f2 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -382,10 +382,11 @@ static inline void gen_store_fpr64 (TCGv_i64 t, int reg)
>   #define REG(x)     ctx->gregs[x]
>   #define ALTREG(x)  ctx->altregs[x]
>   
> -#define FREG(x) (ctx->tbflags & FPSCR_FR ? (x) ^ 0x10 : (x))
> +#define FREG(x) cpu_fregs[ctx->tbflags & FPSCR_FR ? (x) ^ 0x10 : (x)]
>   #define XHACK(x) ((((x) & 1 ) << 4) | ((x) & 0xe))
> -#define XREG(x) (ctx->tbflags & FPSCR_FR ? XHACK(x) ^ 0x10 : XHACK(x))
> -#define DREG(x) FREG(x) /* Assumes lsb of (x) is always 0 */
> +#define XREG(x) FREG(XHACK(x))
> +/* Assumes lsb of (x) is always 0 */
> +#define DREG(x) (ctx->tbflags & FPSCR_FR ? (x) ^ 0x10 : (x))
>   
>   #define CHECK_NOT_DELAY_SLOT \
>       if (ctx->envflags & DELAY_SLOT_MASK) {                           \
> @@ -1005,56 +1006,51 @@ static void _decode_opc(DisasContext * ctx)
>   	CHECK_FPU_ENABLED
>           if (ctx->tbflags & FPSCR_SZ) {
>   	    TCGv_i64 fp = tcg_temp_new_i64();
> -	    gen_load_fpr64(fp, XREG(B7_4));
> -	    gen_store_fpr64(fp, XREG(B11_8));
> +	    gen_load_fpr64(fp, XHACK(B7_4));
> +	    gen_store_fpr64(fp, XHACK(B11_8));
>   	    tcg_temp_free_i64(fp);
>   	} else {
> -	    tcg_gen_mov_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B7_4)]);
> +	    tcg_gen_mov_i32(FREG(B11_8), FREG(B7_4));
>   	}
>   	return;
>       case 0xf00a: /* fmov {F,D,X}Rm,@Rn - FPSCR: Nothing */
>   	CHECK_FPU_ENABLED
>           if (ctx->tbflags & FPSCR_SZ) {
>   	    TCGv addr_hi = tcg_temp_new();
> -	    int fr = XREG(B7_4);
> +	    int fr = XHACK(B7_4);
>   	    tcg_gen_addi_i32(addr_hi, REG(B11_8), 4);
> -            tcg_gen_qemu_st_i32(cpu_fregs[fr], REG(B11_8),
> -                                ctx->memidx, MO_TEUL);
> -            tcg_gen_qemu_st_i32(cpu_fregs[fr+1], addr_hi,
> -                                ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_st_i32(FREG(fr), REG(B11_8), ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_st_i32(FREG(fr + 1), addr_hi, ctx->memidx, MO_TEUL);
>   	    tcg_temp_free(addr_hi);
>   	} else {
> -            tcg_gen_qemu_st_i32(cpu_fregs[FREG(B7_4)], REG(B11_8),
> -                                ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_st_i32(FREG(B7_4), REG(B11_8), ctx->memidx, MO_TEUL);
>   	}
>   	return;
>       case 0xf008: /* fmov @Rm,{F,D,X}Rn - FPSCR: Nothing */
>   	CHECK_FPU_ENABLED
>           if (ctx->tbflags & FPSCR_SZ) {
>   	    TCGv addr_hi = tcg_temp_new();
> -	    int fr = XREG(B11_8);
> +	    int fr = XHACK(B11_8);
>   	    tcg_gen_addi_i32(addr_hi, REG(B7_4), 4);
> -            tcg_gen_qemu_ld_i32(cpu_fregs[fr], REG(B7_4), ctx->memidx, MO_TEUL);
> -            tcg_gen_qemu_ld_i32(cpu_fregs[fr+1], addr_hi, ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_ld_i32(FREG(fr), REG(B7_4), ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_ld_i32(FREG(fr + 1), addr_hi, ctx->memidx, MO_TEUL);
>   	    tcg_temp_free(addr_hi);
>   	} else {
> -            tcg_gen_qemu_ld_i32(cpu_fregs[FREG(B11_8)], REG(B7_4),
> -                                ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_ld_i32(FREG(B11_8), REG(B7_4), ctx->memidx, MO_TEUL);
>   	}
>   	return;
>       case 0xf009: /* fmov @Rm+,{F,D,X}Rn - FPSCR: Nothing */
>   	CHECK_FPU_ENABLED
>           if (ctx->tbflags & FPSCR_SZ) {
>   	    TCGv addr_hi = tcg_temp_new();
> -	    int fr = XREG(B11_8);
> +	    int fr = XHACK(B11_8);
>   	    tcg_gen_addi_i32(addr_hi, REG(B7_4), 4);
> -            tcg_gen_qemu_ld_i32(cpu_fregs[fr], REG(B7_4), ctx->memidx, MO_TEUL);
> -            tcg_gen_qemu_ld_i32(cpu_fregs[fr+1], addr_hi, ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_ld_i32(FREG(fr), REG(B7_4), ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_ld_i32(FREG(fr + 1), addr_hi, ctx->memidx, MO_TEUL);
>   	    tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 8);
>   	    tcg_temp_free(addr_hi);
>   	} else {
> -            tcg_gen_qemu_ld_i32(cpu_fregs[FREG(B11_8)], REG(B7_4),
> -                                ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_ld_i32(FREG(B11_8), REG(B7_4), ctx->memidx, MO_TEUL);
>   	    tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 4);
>   	}
>   	return;
> @@ -1063,13 +1059,12 @@ static void _decode_opc(DisasContext * ctx)
>           TCGv addr = tcg_temp_new_i32();
>           tcg_gen_subi_i32(addr, REG(B11_8), 4);
>           if (ctx->tbflags & FPSCR_SZ) {
> -	    int fr = XREG(B7_4);
> -            tcg_gen_qemu_st_i32(cpu_fregs[fr+1], addr, ctx->memidx, MO_TEUL);
> +	    int fr = XHACK(B7_4);
> +            tcg_gen_qemu_st_i32(FREG(fr + 1), addr, ctx->memidx, MO_TEUL);
>   	    tcg_gen_subi_i32(addr, addr, 4);
> -            tcg_gen_qemu_st_i32(cpu_fregs[fr], addr, ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_st_i32(FREG(fr), addr, ctx->memidx, MO_TEUL);
>   	} else {
> -            tcg_gen_qemu_st_i32(cpu_fregs[FREG(B7_4)], addr,
> -                                ctx->memidx, MO_TEUL);
> +            tcg_gen_qemu_st_i32(FREG(B7_4), addr, ctx->memidx, MO_TEUL);
>   	}
>           tcg_gen_mov_i32(REG(B11_8), addr);
>           tcg_temp_free(addr);
> @@ -1080,15 +1075,12 @@ static void _decode_opc(DisasContext * ctx)
>   	    TCGv addr = tcg_temp_new_i32();
>   	    tcg_gen_add_i32(addr, REG(B7_4), REG(0));
>               if (ctx->tbflags & FPSCR_SZ) {
> -		int fr = XREG(B11_8);
> -                tcg_gen_qemu_ld_i32(cpu_fregs[fr], addr,
> -                                    ctx->memidx, MO_TEUL);
> +		int fr = XHACK(B11_8);
> +                tcg_gen_qemu_ld_i32(FREG(fr), addr, ctx->memidx, MO_TEUL);
>   		tcg_gen_addi_i32(addr, addr, 4);
> -                tcg_gen_qemu_ld_i32(cpu_fregs[fr+1], addr,
> -                                    ctx->memidx, MO_TEUL);
> +                tcg_gen_qemu_ld_i32(FREG(fr + 1), addr, ctx->memidx, MO_TEUL);
>   	    } else {
> -                tcg_gen_qemu_ld_i32(cpu_fregs[FREG(B11_8)], addr,
> -                                    ctx->memidx, MO_TEUL);
> +                tcg_gen_qemu_ld_i32(FREG(B11_8), addr, ctx->memidx, MO_TEUL);
>   	    }
>   	    tcg_temp_free(addr);
>   	}
> @@ -1099,15 +1091,12 @@ static void _decode_opc(DisasContext * ctx)
>   	    TCGv addr = tcg_temp_new();
>   	    tcg_gen_add_i32(addr, REG(B11_8), REG(0));
>               if (ctx->tbflags & FPSCR_SZ) {
> -		int fr = XREG(B7_4);
> -                tcg_gen_qemu_ld_i32(cpu_fregs[fr], addr,
> -                                    ctx->memidx, MO_TEUL);
> +		int fr = XHACK(B7_4);
> +                tcg_gen_qemu_ld_i32(FREG(fr), addr, ctx->memidx, MO_TEUL);
>   		tcg_gen_addi_i32(addr, addr, 4);
> -                tcg_gen_qemu_ld_i32(cpu_fregs[fr+1], addr,
> -                                    ctx->memidx, MO_TEUL);
> +                tcg_gen_qemu_ld_i32(FREG(fr + 1), addr, ctx->memidx, MO_TEUL);
>   	    } else {
> -                tcg_gen_qemu_st_i32(cpu_fregs[FREG(B7_4)], addr,
> -                                    ctx->memidx, MO_TEUL);
> +                tcg_gen_qemu_st_i32(FREG(B7_4), addr, ctx->memidx, MO_TEUL);
>   	    }
>   	    tcg_temp_free(addr);
>   	}
> @@ -1155,32 +1144,26 @@ static void _decode_opc(DisasContext * ctx)
>   	    } else {
>                   switch (ctx->opcode & 0xf00f) {
>                   case 0xf000:		/* fadd Rm,Rn */
> -                    gen_helper_fadd_FT(cpu_fregs[FREG(B11_8)], cpu_env,
> -                                       cpu_fregs[FREG(B11_8)],
> -                                       cpu_fregs[FREG(B7_4)]);
> +                    gen_helper_fadd_FT(FREG(B11_8), cpu_env,
> +                                       FREG(B11_8), FREG(B7_4));
>                       break;
>                   case 0xf001:		/* fsub Rm,Rn */
> -                    gen_helper_fsub_FT(cpu_fregs[FREG(B11_8)], cpu_env,
> -                                       cpu_fregs[FREG(B11_8)],
> -                                       cpu_fregs[FREG(B7_4)]);
> +                    gen_helper_fsub_FT(FREG(B11_8), cpu_env,
> +                                       FREG(B11_8), FREG(B7_4));
>                       break;
>                   case 0xf002:		/* fmul Rm,Rn */
> -                    gen_helper_fmul_FT(cpu_fregs[FREG(B11_8)], cpu_env,
> -                                       cpu_fregs[FREG(B11_8)],
> -                                       cpu_fregs[FREG(B7_4)]);
> +                    gen_helper_fmul_FT(FREG(B11_8), cpu_env,
> +                                       FREG(B11_8), FREG(B7_4));
>                       break;
>                   case 0xf003:		/* fdiv Rm,Rn */
> -                    gen_helper_fdiv_FT(cpu_fregs[FREG(B11_8)], cpu_env,
> -                                       cpu_fregs[FREG(B11_8)],
> -                                       cpu_fregs[FREG(B7_4)]);
> +                    gen_helper_fdiv_FT(FREG(B11_8), cpu_env,
> +                                       FREG(B11_8), FREG(B7_4));
>                       break;
>                   case 0xf004:		/* fcmp/eq Rm,Rn */
> -                    gen_helper_fcmp_eq_FT(cpu_env, cpu_fregs[FREG(B11_8)],
> -                                          cpu_fregs[FREG(B7_4)]);
> +                    gen_helper_fcmp_eq_FT(cpu_env, FREG(B11_8), FREG(B7_4));
>                       return;
>                   case 0xf005:		/* fcmp/gt Rm,Rn */
> -                    gen_helper_fcmp_gt_FT(cpu_env, cpu_fregs[FREG(B11_8)],
> -                                          cpu_fregs[FREG(B7_4)]);
> +                    gen_helper_fcmp_gt_FT(cpu_env, FREG(B11_8), FREG(B7_4));
>                       return;
>                   }
>   	    }
> @@ -1192,9 +1175,8 @@ static void _decode_opc(DisasContext * ctx)
>               if (ctx->tbflags & FPSCR_PR) {
>                   break; /* illegal instruction */
>               } else {
> -                gen_helper_fmac_FT(cpu_fregs[FREG(B11_8)], cpu_env,
> -                                   cpu_fregs[FREG(0)], cpu_fregs[FREG(B7_4)],
> -                                   cpu_fregs[FREG(B11_8)]);
> +                gen_helper_fmac_FT(FREG(B11_8), cpu_env,
> +                                   FREG(0), FREG(B7_4), FREG(B11_8));
>                   return;
>               }
>           }
> @@ -1732,11 +1714,11 @@ static void _decode_opc(DisasContext * ctx)
>           return;
>       case 0xf00d: /* fsts FPUL,FRn - FPSCR: Nothing */
>   	CHECK_FPU_ENABLED
> -	tcg_gen_mov_i32(cpu_fregs[FREG(B11_8)], cpu_fpul);
> +	tcg_gen_mov_i32(FREG(B11_8), cpu_fpul);
>   	return;
>       case 0xf01d: /* flds FRm,FPUL - FPSCR: Nothing */
>   	CHECK_FPU_ENABLED
> -	tcg_gen_mov_i32(cpu_fpul, cpu_fregs[FREG(B11_8)]);
> +	tcg_gen_mov_i32(cpu_fpul, FREG(B11_8));
>   	return;
>       case 0xf02d: /* float FPUL,FRn/DRn - FPSCR: R[PR,Enable.I]/W[Cause,Flag] */
>   	CHECK_FPU_ENABLED
> @@ -1750,7 +1732,7 @@ static void _decode_opc(DisasContext * ctx)
>   	    tcg_temp_free_i64(fp);
>   	}
>   	else {
> -            gen_helper_float_FT(cpu_fregs[FREG(B11_8)], cpu_env, cpu_fpul);
> +            gen_helper_float_FT(FREG(B11_8), cpu_env, cpu_fpul);
>   	}
>   	return;
>       case 0xf03d: /* ftrc FRm/DRm,FPUL - FPSCR: R[PR,Enable.V]/W[Cause,Flag] */
> @@ -1765,13 +1747,13 @@ static void _decode_opc(DisasContext * ctx)
>   	    tcg_temp_free_i64(fp);
>   	}
>   	else {
> -            gen_helper_ftrc_FT(cpu_fpul, cpu_env, cpu_fregs[FREG(B11_8)]);
> +            gen_helper_ftrc_FT(cpu_fpul, cpu_env, FREG(B11_8));
>   	}
>   	return;
>       case 0xf04d: /* fneg FRn/DRn - FPSCR: Nothing */
>   	CHECK_FPU_ENABLED
>   	{
> -	    gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
> +	    gen_helper_fneg_T(FREG(B11_8), FREG(B11_8));
>   	}
>   	return;
>       case 0xf05d: /* fabs FRn/DRn */
> @@ -1785,7 +1767,7 @@ static void _decode_opc(DisasContext * ctx)
>   	    gen_store_fpr64(fp, DREG(B11_8));
>   	    tcg_temp_free_i64(fp);
>   	} else {
> -	    gen_helper_fabs_FT(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
> +	    gen_helper_fabs_FT(FREG(B11_8), FREG(B11_8));
>   	}
>   	return;
>       case 0xf06d: /* fsqrt FRn */
> @@ -1799,8 +1781,7 @@ static void _decode_opc(DisasContext * ctx)
>   	    gen_store_fpr64(fp, DREG(B11_8));
>   	    tcg_temp_free_i64(fp);
>   	} else {
> -            gen_helper_fsqrt_FT(cpu_fregs[FREG(B11_8)], cpu_env,
> -                                cpu_fregs[FREG(B11_8)]);
> +            gen_helper_fsqrt_FT(FREG(B11_8), cpu_env, FREG(B11_8));
>   	}
>   	return;
>       case 0xf07d: /* fsrra FRn */
> @@ -1809,13 +1790,13 @@ static void _decode_opc(DisasContext * ctx)
>       case 0xf08d: /* fldi0 FRn - FPSCR: R[PR] */
>   	CHECK_FPU_ENABLED
>           if (!(ctx->tbflags & FPSCR_PR)) {
> -	    tcg_gen_movi_i32(cpu_fregs[FREG(B11_8)], 0);
> +	    tcg_gen_movi_i32(FREG(B11_8), 0);
>   	}
>   	return;
>       case 0xf09d: /* fldi1 FRn - FPSCR: R[PR] */
>   	CHECK_FPU_ENABLED
>           if (!(ctx->tbflags & FPSCR_PR)) {
> -	    tcg_gen_movi_i32(cpu_fregs[FREG(B11_8)], 0x3f800000);
> +	    tcg_gen_movi_i32(FREG(B11_8), 0x3f800000);
>   	}
>   	return;
>       case 0xf0ad: /* fcnvsd FPUL,DRn */
> 

  reply	other threads:[~2017-07-06  1:55 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-06  0:23 [Qemu-devel] [PATCH 00/11] target/sh4 improvments Richard Henderson
2017-07-06  0:23 ` [Qemu-devel] [PATCH 01/11] target/sh4: Use cmpxchg for movco Richard Henderson
2017-07-06 15:25   ` Richard Henderson
2017-07-06  0:23 ` [Qemu-devel] [PATCH 02/11] target/sh4: Consolidate end-of-TB tests Richard Henderson
2017-07-06 15:17   ` Aurelien Jarno
2017-07-06  0:23 ` [Qemu-devel] [PATCH 03/11] target/sh4: Handle user-space atomics Richard Henderson
2017-07-06 15:50   ` Aurelien Jarno
2017-07-06  0:23 ` [Qemu-devel] [PATCH 04/11] target/sh4: Recognize common gUSA sequences Richard Henderson
2017-07-06  0:23 ` [Qemu-devel] [PATCH 05/11] linux-user/sh4: Notice gUSA regions during signal delivery Richard Henderson
2017-07-06  1:09   ` Laurent Vivier
2017-07-06  8:10     ` John Paul Adrian Glaubitz
2017-07-06  8:35       ` Laurent Vivier
2017-07-06  9:07         ` John Paul Adrian Glaubitz
2017-07-06  9:13         ` John Paul Adrian Glaubitz
2017-07-06  9:19           ` Laurent Vivier
2017-07-06 11:07     ` John Paul Adrian Glaubitz
2017-07-06 12:09   ` Laurent Vivier
2017-07-06  0:23 ` [Qemu-devel] [PATCH 06/11] target/sh4: Hoist register bank selection Richard Henderson
2017-07-06  0:23 ` [Qemu-devel] [PATCH 07/11] target/sh4: Unify cpu_fregs into FREG Richard Henderson
2017-07-06  1:55   ` Philippe Mathieu-Daudé [this message]
2017-07-06  0:23 ` [Qemu-devel] [PATCH 08/11] target/sh4: Pass DisasContext to fpr64 routines Richard Henderson
2017-07-06  0:23 ` [Qemu-devel] [PATCH 09/11] target/sh4: Avoid a potential translator crash for malformed FPR64 Richard Henderson
2017-07-06  0:24 ` [Qemu-devel] [PATCH 10/11] target/sh4: Hoist fp bank selection Richard Henderson
2017-07-06  0:24 ` [Qemu-devel] [PATCH 11/11] target/sh4: Eliminate DREG macro Richard Henderson
2017-07-06  1:15 ` [Qemu-devel] [PATCH 00/11] target/sh4 improvments Laurent Vivier
2017-07-06 14:55 ` Aurelien Jarno

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=09234b5f-c837-3cb0-ea27-449591753b42@amsat.org \
    --to=f4bug@amsat.org \
    --cc=aurelien@aurel32.net \
    --cc=bruno@clisp.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.