| Message ID | 20190218143049.17142-34-david@gibson.dropbear.id.au |
|---|---|
| State | Accepted |
| Commit | 9bb0048ec6f8f3bcc144b2c5769d9301e824f946 |
| Headers | show |
| Series | None | expand |
On Mon, 18 Feb 2019 at 14:31, David Gibson <david@gibson.dropbear.id.au> wrote: > > From: Richard Henderson <richard.henderson@linaro.org> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Message-Id: <20190215100058.20015-8-mark.cave-ayland@ilande.co.uk> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> So this is a commit from 18 months back, but I happened to notice an oddity in it while grepping the code base for something else: > diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c > index 944fc0608a..0e8cecb00a 100644 > --- a/target/ppc/translate/vsx-impl.inc.c > +++ b/target/ppc/translate/vsx-impl.inc.c > @@ -1359,38 +1359,24 @@ static void gen_xxsel(DisasContext * ctx) > > static void gen_xxspltw(DisasContext *ctx) > { > - TCGv_i64 b, b2; > - TCGv_i64 vsr; > + int rt = xT(ctx->opcode); > + int rb = xB(ctx->opcode); > + int uim = UIM(ctx->opcode); > + int tofs, bofs; [...] > - tcg_gen_shli_i64(b2, b, 32); > - tcg_gen_or_i64(vsr, b, b2); > - set_cpu_vsrh(xT(ctx->opcode), vsr); > - set_cpu_vsrl(xT(ctx->opcode), vsr); > + tofs = vsr_full_offset(rt); > + bofs = vsr_full_offset(rb); > + bofs += uim << MO_32; > +#ifndef HOST_WORDS_BIG_ENDIAN > + bofs ^= 8 | 4; > +#endif The ifdef is HOST_WORDS_BIGENDIAN without the third underscore, so this XOR operation will be done on both little and big-endian hosts. Should the ifndef line be fixed, or is this actually a case where the xor really should be done on all hosts so we should drop the ifndef/endif lines? > > - tcg_temp_free_i64(vsr); > - tcg_temp_free_i64(b); > - tcg_temp_free_i64(b2); > + tcg_gen_gvec_dup_mem(MO_32, tofs, bofs, 16, 16); > } thanks -- PMM
On 11/6/20 10:47 AM, Peter Maydell wrote: >> +#ifndef HOST_WORDS_BIG_ENDIAN >> + bofs ^= 8 | 4; >> +#endif > > The ifdef is HOST_WORDS_BIGENDIAN without the > third underscore, so this XOR operation will be > done on both little and big-endian hosts. Ho hum. > Should the ifndef line be fixed... This. I once had a patch set that changed all of our endian tests from defined/undef to true/false, so that we could detect errors like this. Perhaps I'll try to recreate it next dev cycle... r~
diff --git a/target/ppc/translate/vsx-impl.inc.c b/target/ppc/translate/vsx-impl.inc.c index 944fc0608a..0e8cecb00a 100644 --- a/target/ppc/translate/vsx-impl.inc.c +++ b/target/ppc/translate/vsx-impl.inc.c @@ -1359,38 +1359,24 @@ static void gen_xxsel(DisasContext * ctx) static void gen_xxspltw(DisasContext *ctx) { - TCGv_i64 b, b2; - TCGv_i64 vsr; + int rt = xT(ctx->opcode); + int rb = xB(ctx->opcode); + int uim = UIM(ctx->opcode); + int tofs, bofs; if (unlikely(!ctx->vsx_enabled)) { gen_exception(ctx, POWERPC_EXCP_VSXU); return; } - vsr = tcg_temp_new_i64(); - if (UIM(ctx->opcode) & 2) { - get_cpu_vsrl(vsr, xB(ctx->opcode)); - } else { - get_cpu_vsrh(vsr, xB(ctx->opcode)); - } - - b = tcg_temp_new_i64(); - b2 = tcg_temp_new_i64(); - - if (UIM(ctx->opcode) & 1) { - tcg_gen_ext32u_i64(b, vsr); - } else { - tcg_gen_shri_i64(b, vsr, 32); - } - - tcg_gen_shli_i64(b2, b, 32); - tcg_gen_or_i64(vsr, b, b2); - set_cpu_vsrh(xT(ctx->opcode), vsr); - set_cpu_vsrl(xT(ctx->opcode), vsr); + tofs = vsr_full_offset(rt); + bofs = vsr_full_offset(rb); + bofs += uim << MO_32; +#ifndef HOST_WORDS_BIG_ENDIAN + bofs ^= 8 | 4; +#endif - tcg_temp_free_i64(vsr); - tcg_temp_free_i64(b); - tcg_temp_free_i64(b2); + tcg_gen_gvec_dup_mem(MO_32, tofs, bofs, 16, 16); } #define pattern(x) (((x) & 0xff) * (~(uint64_t)0 / 0xff))