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~