Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations

[Aurelien Jarno]

On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
> The recent transition to always have the DCR helper functions take 32 bit
> values broke the PPC64 target, as tlong became 64 bits there.
> 
> This patch moves all translate.c callers to a _tl function that simply
> calls the uint32_t functions. That way we don't need to mess with TCG
> trying to pass registers as uint32_t variables to functions.
> 
> Fixes PPC64 build with --enable-debug-tcg
> 
> Signed-off-by: Alexander Graf <address@hidden>
> Reported-by: Stefan Weil <address@hidden>
> ---
>  target-ppc/cpu.h       |    2 ++
>  target-ppc/helper.h    |    4 ++--
>  target-ppc/op_helper.c |   10 ++++++++++
>  target-ppc/translate.c |   12 ++++++------
>  4 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index d15bba1..60a8b68 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, 
> target_ulong rs);
>  void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
>  #endif /* !defined(CONFIG_USER_ONLY) */
>  void ppc_store_msr (CPUPPCState *env, target_ulong value);
> +void helper_store_dcr (uint32_t dcrn, uint32_t val);
> +uint32_t helper_load_dcr (uint32_t dcrn);
>  
>  void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, 
> ...));
>  
> diff --git a/target-ppc/helper.h b/target-ppc/helper.h
> index 40d4ced..86f0af7 100644
> --- a/target-ppc/helper.h
> +++ b/target-ppc/helper.h
> @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
>  DEF_HELPER_2(divs, tl, tl, tl)
>  DEF_HELPER_2(divso, tl, tl, tl)
>  
> -DEF_HELPER_1(load_dcr, i32, i32);
> -DEF_HELPER_2(store_dcr, void, i32, i32)
> +DEF_HELPER_1(load_dcr_tl, tl, tl);
> +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
>  
>  DEF_HELPER_1(load_dump_spr, void, i32)
>  DEF_HELPER_1(store_dump_spr, void, i32)
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index cea27f2..6c375d3 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
>      return val;
>  }
>  
> +target_ulong helper_load_dcr_tl (target_ulong dcrn)
> +{
> +    return (uint32_t)helper_load_dcr((uint32_t)dcrn);
> +}
> +
>  void helper_store_dcr (uint32_t dcrn, uint32_t val)
>  {
>      if (unlikely(env->dcr_env == NULL)) {
> @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
>      }
>  }
>  
> +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
> +{
> +    helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
> +}
> +

I do wonder why we need to keep the old helper_load_dcr() and
helper_store_dcr() instead of modifying them directly. They doesn't seems
to be used elsewhere.

>  #if !defined(CONFIG_USER_ONLY)
>  void helper_40x_rfci (void)
>  {
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index d4e81ce..d83d196 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -5565,7 +5565,7 @@ static void gen_mfdcr(DisasContext *ctx)
>      /* NIP cannot be restored if the memory exception comes from an helper */
>      gen_update_nip(ctx, ctx->nip - 4);
>      dcrn = tcg_const_tl(SPR(ctx->opcode));
> -    gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], dcrn);
> +    gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], dcrn);
>      tcg_temp_free(dcrn);
>  #endif
>  }
> @@ -5584,7 +5584,7 @@ static void gen_mtdcr(DisasContext *ctx)
>      /* NIP cannot be restored if the memory exception comes from an helper */
>      gen_update_nip(ctx, ctx->nip - 4);
>      dcrn = tcg_const_tl(SPR(ctx->opcode));
> -    gen_helper_store_dcr(dcrn, cpu_gpr[rS(ctx->opcode)]);
> +    gen_helper_store_dcr_tl(dcrn, cpu_gpr[rS(ctx->opcode)]);
>      tcg_temp_free(dcrn);
>  #endif
>  }
> @@ -5602,7 +5602,7 @@ static void gen_mfdcrx(DisasContext *ctx)
>      }
>      /* NIP cannot be restored if the memory exception comes from an helper */
>      gen_update_nip(ctx, ctx->nip - 4);
> -    gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> +    gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], 
> cpu_gpr[rA(ctx->opcode)]);
>      /* Note: Rc update flag set leads to undefined state of Rc0 */
>  #endif
>  }
> @@ -5620,7 +5620,7 @@ static void gen_mtdcrx(DisasContext *ctx)
>      }
>      /* NIP cannot be restored if the memory exception comes from an helper */
>      gen_update_nip(ctx, ctx->nip - 4);
> -    gen_helper_store_dcr(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> +    gen_helper_store_dcr_tl(cpu_gpr[rA(ctx->opcode)], 
> cpu_gpr[rS(ctx->opcode)]);
>      /* Note: Rc update flag set leads to undefined state of Rc0 */
>  #endif
>  }
> @@ -5630,7 +5630,7 @@ static void gen_mfdcrux(DisasContext *ctx)
>  {
>      /* NIP cannot be restored if the memory exception comes from an helper */
>      gen_update_nip(ctx, ctx->nip - 4);
> -    gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> +    gen_helper_load_dcr_tl(cpu_gpr[rD(ctx->opcode)], 
> cpu_gpr[rA(ctx->opcode)]);
>      /* Note: Rc update flag set leads to undefined state of Rc0 */
>  }
>  
> @@ -5639,7 +5639,7 @@ static void gen_mtdcrux(DisasContext *ctx)
>  {
>      /* NIP cannot be restored if the memory exception comes from an helper */
>      gen_update_nip(ctx, ctx->nip - 4);
> -    gen_helper_store_dcr(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> +    gen_helper_store_dcr_tl(cpu_gpr[rA(ctx->opcode)], 
> cpu_gpr[rS(ctx->opcode)]);
>      /* Note: Rc update flag set leads to undefined state of Rc0 */
>  }
>  
> -- 
> 1.6.0.2
> 
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net