On Mon, Dec 12, 2016 at 09:31:11AM +0530, Nikunj Dadhania wrote: > On 12 December 2016 at 06:00, David Gibson <da...@gibson.dropbear.id.au> > wrote: > > On Fri, Dec 09, 2016 at 05:47:24PM +0530, Nikunj A Dadhania wrote: > >> xxextractuw: VSX Vector Extract Unsigned Word > >> > >> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> > >> --- > >> target-ppc/helper.h | 1 + > >> target-ppc/int_helper.c | 21 +++++++++++++++++++++ > >> target-ppc/translate/vsx-impl.inc.c | 27 +++++++++++++++++++++++++++ > >> target-ppc/translate/vsx-ops.inc.c | 5 +++++ > >> 4 files changed, 54 insertions(+) > >> > >> diff --git a/target-ppc/helper.h b/target-ppc/helper.h > >> index 4707db4..8b30420 100644 > >> --- a/target-ppc/helper.h > >> +++ b/target-ppc/helper.h > >> @@ -540,6 +540,7 @@ DEF_HELPER_2(xvrspip, void, env, i32) > >> DEF_HELPER_2(xvrspiz, void, env, i32) > >> DEF_HELPER_2(xxperm, void, env, i32) > >> DEF_HELPER_2(xxpermr, void, env, i32) > >> +DEF_HELPER_4(xxextractuw, void, env, tl, tl, i32) > >> > >> DEF_HELPER_2(efscfsi, i32, env, i32) > >> DEF_HELPER_2(efscfui, i32, env, i32) > >> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c > >> index 7989b1f..e3f66ac 100644 > >> --- a/target-ppc/int_helper.c > >> +++ b/target-ppc/int_helper.c > >> @@ -2033,6 +2033,27 @@ VEXTRACT(uw, u32) > >> VEXTRACT(d, u64) > >> #undef VEXTRACT > >> > >> +void helper_xxextractuw(CPUPPCState *env, target_ulong xtn, > >> + target_ulong xbn, uint32_t index) > >> +{ > >> + ppc_vsr_t xt, xb; > >> + size_t es = sizeof(uint32_t); > >> + uint32_t ext_index; > >> + > >> + getVSR(xbn, &xb, env); > >> + memset(&xt, 0, sizeof(xt)); > >> + > >> +#if defined(HOST_WORDS_BIGENDIAN) > >> + ext_index = index; > >> + memcpy(&xt.u8[8 - es], &xb.u8[ext_index], es); > >> +#else > >> + ext_index = (16 - index) - es; > >> + memcpy(&xt.u8[8], &xb.u8[ext_index], es); > > > > Hm. So, IIUC, ext_index is the byte element - in IBM numbering - to > > start copying from. But I thought that when we have an LE host, the > > IBM byte element ordering is reversed from the actual order in host > > memory, so we'd need &xb.u8[16 - ext_index - es] > > I am not getting you, I am getting index from user. So in case of BE host: > > ext_index = index; > > LE Host: > > ext_index = (16 - index) - es; > > I am already doing that. Am I missing something.
Duh, sorry, apparently I'm blind and missed that logic. > >> +#endif > >> + > >> + putVSR(xtn, &xt, env); > >> +} > >> + > >> #define VEXT_SIGNED(name, element, mask, cast, recast) \ > >> void helper_##name(ppc_avr_t *r, ppc_avr_t *b) \ > >> { \ > >> diff --git a/target-ppc/translate/vsx-impl.inc.c > >> b/target-ppc/translate/vsx-impl.inc.c > >> index 2a17c35..1c40a35 100644 > >> --- a/target-ppc/translate/vsx-impl.inc.c > >> +++ b/target-ppc/translate/vsx-impl.inc.c > >> @@ -1180,6 +1180,33 @@ static void gen_xxsldwi(DisasContext *ctx) > >> tcg_temp_free_i64(xtl); > >> } > >> > >> +#define VSX_EXTRACT(name) \ > >> +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; \ > >> + } \ > >> + if (uimm > 12) { \ > > > > Throughout the helper you use es == sizeof(uint32_t), but here you > > hardcode the assumption of 4 bytes, seems a bit inconsistent. > > > >> + tcg_gen_movi_i64(cpu_vsrh(xT(ctx->opcode)), 0); \ > >> + tcg_gen_movi_i64(cpu_vsrl(xT(ctx->opcode)), 0); \ > >> + return; \ > > > > So, I know the architecture says it is undefined. But since you're > > testing for the bogus case anyway, why not turn this into an > > exception. That seems like it would be more helpful for debugging the > > guest than just setting the result to zero. Or is this done to match > > actual hardware behaviour? > > I havent had a change to run on the real hardware, but on the system > simulator, it happily > returns extracted content even if UIMM > 12. Hm. Returns what exactly? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature