Re: [Qemu-devel] [PATCH 17/40] Add m{f,t}vscr instructions.

[Aurelien Jarno]

On Tue, Dec 30, 2008 at 07:09:59PM -0800, Nathan Froyd wrote:
> 
> Signed-off-by: Nathan Froyd <address@hidden>
> ---
>  target-ppc/translate.c |   37 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 4e1bd23..5c13ed2 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6178,6 +6178,43 @@ GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x00000001, 
> PPC_ALTIVEC)
>      tcg_temp_free_ptr(rd);
>  }
>  
> +static always_inline uint32_t gen_vscr_offset (int r)
> +{
> +#if defined(WORDS_BIGENDIAN)
> +    return offsetof(CPUPPCState, avr[r].u32[3]);
> +#else
> +    return offsetof(CPUPPCState, avr[r].u32[0]);
> +#endif
> +}
> +
> +GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC)
> +{
> +    TCGv_i32 t;
> +    if (unlikely(!ctx->altivec_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_VPU);
> +        return;
> +    }
> +    tcg_gen_movi_i64(cpu_avrh[rD(ctx->opcode)], 0);
> +    tcg_gen_movi_i64(cpu_avrl[rD(ctx->opcode)], 0);
> +    t = tcg_temp_new_i32();
> +    tcg_gen_ld_i32(t, cpu_env, offsetof(CPUState, vscr));
> +    tcg_gen_st_i32(t, cpu_env, gen_vscr_offset(rD(ctx->opcode)));
> +    tcg_temp_free_i32(t);
> +}

TCG doesn't like when a variable is accessed through both a TCG register
and a TCG load/store in the same TB. This is most probably the problem
you are seeing with mfvscr/mtvscr.

I suggest you to replace the movi_i64 to cpu_avrl and the st_i32 by a
extu_i32_i64 to cpu_avrl.

> +GEN_HANDLER(mtvscr, 0x04, 0x2, 0x19, 0x03ff0000, PPC_ALTIVEC)
> +{
> +    TCGv_i32 t;
> +    if (unlikely(!ctx->altivec_enabled)) {
> +        gen_exception(ctx, POWERPC_EXCP_VPU);
> +        return;
> +    }
> +    t = tcg_temp_new_i32();
> +    tcg_gen_ld_i32(t, cpu_env, gen_vscr_offset(rB(ctx->opcode)));
> +    tcg_gen_st_i32(t, cpu_env, offsetof(CPUState, vscr));
> +    tcg_temp_free_i32(t);
> +}

The same way, the ld_i32 can be replaced by a trunc_i64_i32 on cpu_avrl.
This way you can get rid of gen_vscr_offset().

>  /* Logical operations */
>  #define GEN_VX_LOGICAL(name, tcg_op, opc2, opc3)                        \
>  GEN_HANDLER(name, 0x04, opc2, opc3, 0x00000000, PPC_ALTIVEC)            \

-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net