Re: [Qemu-devel] [PATCH 4/9] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x

[Anton Blanchard <anton@ozlabs.org>]

Hi Mark,

> Following on from this I've just gone through the load/store
> operations once again and spotted two things:
> 
> 
> 1) VSX_LOAD_SCALAR_DS has an extra get_cpu_vsrh() which can be removed
> 
> diff --git a/target/ppc/translate/vsx-impl.inc.c
> b/target/ppc/translate/vsx-impl.inc.c index 11d9b75d01..004ea56c4f
> 100644 --- a/target/ppc/translate/vsx-impl.inc.c
> +++ b/target/ppc/translate/vsx-impl.inc.c
> @@ -329,7 +329,6 @@ static void gen_##name(DisasContext
> *ctx)                         \
> return;
> \ }                                                             \ xth
> = tcg_temp_new_i64();                                     \
> -    get_cpu_vsrh(xth, rD(ctx->opcode) + 32);                      \
>      gen_set_access_type(ctx, ACCESS_INT);                         \
>      EA = tcg_temp_new();                                          \
>      gen_addr_imm_index(ctx, EA, 0x03);                            \

Looks good. I also noticed we had two stores that needed to be fixed:

VSX_LOAD_SCALAR_DS(stxsd, st64_i64)
VSX_LOAD_SCALAR_DS(stxssp, st32fs)

> 2) VSX_VECTOR_LOAD_STORE is confusing and should be split into
> separate VSX_VECTOR_LOAD and VSX_VECTOR_STORE macros

Good idea. I also removed (what I assume) are redundant set_cpu_vsr*
and get_cpu_vsr* calls.

> Does that sound reasonable? I'm also thinking that we should consider
> adding a CC to stable for patches 4, 5 and 9 in this series since
> these are genuine regressions.

Fine with me. If David agrees, I'm not sure if he can rebase them or
if I can send them manually if they have been already committed.

Thanks,
Anton