Re: [Qemu-devel] [V6 PATCH 15/18] target-ppc: Move To/From VSR Instructions

[Tom Musta]

On 1/10/2014 3:29 PM, Richard Henderson wrote:
> On 01/10/2014 11:07 AM, Tom Musta wrote:
>> +#define MV_VSR(name, tcgop1, tcgop2, target, source)            \
>> +static void gen_##name(DisasContext *ctx)                       \
>> +{                                                               \
>> +    if (xS(ctx->opcode) < 32) {                                 \
>> +        if (unlikely(!ctx->fpu_enabled)) {                      \
>> +            gen_exception(ctx, POWERPC_EXCP_FPU);               \
>> +            return;                                             \
>> +        }                                                       \
>> +    } else {                                                    \
>> +        if (unlikely(!ctx->altivec_enabled)) {                  \
>> +            gen_exception(ctx, POWERPC_EXCP_VPU);               \
>> +            return;                                             \
>> +        }                                                       \
>> +    }                                                           \
>> +    TCGv_i64 tmp = tcg_temp_new_i64();                          \
>> +    tcg_gen_##tcgop1(tmp, source);                              \
>> +    tcg_gen_##tcgop2(target, tmp);                              \
>> +    tcg_temp_free_i64(tmp);                                     \
>> +}
>> +
>> +
>> +MV_VSR(mfvsrwz, ext32u_i64, trunc_i64_tl, cpu_gpr[rA(ctx->opcode)], \
>> +       cpu_vsrh(xS(ctx->opcode)))
>> +MV_VSR(mtvsrwa, extu_tl_i64, ext32s_i64, cpu_vsrh(xT(ctx->opcode)), \
>> +       cpu_gpr[rA(ctx->opcode)])
>> +MV_VSR(mtvsrwz, extu_tl_i64, ext32u_i64, cpu_vsrh(xT(ctx->opcode)), \
>> +       cpu_gpr[rA(ctx->opcode)])
>> +#if defined(TARGET_PPC64)
>> +MV_VSR(mfvsrd, mov_i64, mov_i64, cpu_gpr[rA(ctx->opcode)], \
>> +       cpu_vsrh(xS(ctx->opcode)))
>> +MV_VSR(mtvsrd, mov_i64, mov_i64, cpu_vsrh(xT(ctx->opcode)), \
>> +       cpu_gpr[rA(ctx->opcode)])
>> +#endif
> 
> Better to do this in one step:
> 
> mfcsrwz:      tcg_gen_ext32u_tl
> mtvsrwa:      tcg_gen_ext_tl_i64
> mtvsrwz:      tcg_gen_extu_tl_i64
> m[tf]vsrd:    tcg_gen_mov_i64
> 
> 
> r~
> 

Richard:

As always, thanks for reviewing my patches.

I agree on m[tf]vsrd because these are 64-bit instructions and therefore both
source and target are always i64s.

However, the word versions are a bit more tricky.  The VSR operand is always
an i64.  However, the GPR operand is either an i32 (on 32-bit implementations)
or a part of an i64.  I could not find single TCG operations to handle
these cases.  Specifically, here is what I think doesn't work with your
suggestions:

(1) Using tcg_gen_ext32u_tl for mfvsrwz does not compile on 32-bit PPC -- the
ext32u_tl operation is equivalent to mov_i32 but the source operand (VSRH) is
an i64.

(2) Using tcg_gen_ext_tl_i64 for mtvsrwa compiles but is incorrect on 64-bit PPC
-- the ext_tl_i64 translates to mov_i64.  The instruction semantic is to sign
extend the lower 32 bits of the 64-bit source GPR.

(3) Similarly, using tcg_gen_extu_tl_i64 for mtvsrwz is incorrect for 64-bit
PPC -- this is a mov_i64 and hence does not zero out the upper 32 bits of the
target VSRH.

I can recode the "d" versions to eliminate the extraneous tcg op.