Re: [Qemu-ppc] [Qemu-devel] [PATCH 07/13] target-ppc: implement xxextractuw instruction

[Richard Henderson]

On 12/05/2016 03:25 AM, Nikunj A Dadhania wrote:
> +static void gen_##name(DisasContext *ctx)                       \
> +{                                                               \
> +    TCGv xt, xb;                                                \
> +    TCGv_i32 t0 = tcg_temp_new_i32();                           \
> +    uint8_t uimm = UIMM4(ctx->opcode);                          \
> +                                                                \
> +    if (unlikely(!ctx->vsx_enabled)) {                          \
> +        gen_exception(ctx, POWERPC_EXCP_VSXU);                  \
> +        return;                                                 \
> +    }                                                           \
> +    xt = tcg_const_tl(xT(ctx->opcode));                         \
> +    xb = tcg_const_tl(xB(ctx->opcode));                         \
> +    tcg_gen_movi_i32(t0, uimm);                                 \
> +    gen_helper_##name(cpu_env, xt, xb, t0);                     \
> +    tcg_temp_free(xb);                                          \
> +    tcg_temp_free(xt);                                          \
> +    tcg_temp_free_i32(t0);                                      \

While the ISA manual says that the results are undefined if UIMM > 12, I don't
think you should allow the implementation here to read beyond the end of the XB
register.

What does real hardware do?  I would have hoped for a SIGILL, but I don't
suppose that actually happens.

> +    getVSR(xbn, &xb, env);                                              \
> +    memmove(&xt.u8[8 - es], &xb.u8[index], es);                         \
> +    memset(&xt.u8[8], 0, 8);                                            \
> +    memset(&xt.u8[0], 0, 8 - es);                                       \
> +    putVSR(xtn, &xt, env);                                              \

I think this would be simpler as

  memset(&xt, 0, sizeof(xt));
  memcpy(&xt.u8[8 - es], &xb.u8[index], es);

since xt and xb are local variables and therefore cannot overlap.


r~