From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1N8g-00012Y-FX for qemu-devel@nongnu.org; Mon, 31 Oct 2016 20:41:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1N8c-000445-9h for qemu-devel@nongnu.org; Mon, 31 Oct 2016 20:41:18 -0400 Date: Tue, 1 Nov 2016 11:30:33 +1100 From: David Gibson Message-ID: <20161101003033.GB15969@umbus.fritz.box> References: <1477935289-5010-1-git-send-email-joserz@linux.vnet.ibm.com> <1477935289-5010-2-git-send-email-joserz@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="i0/AhcQY5QxfSsSZ" Content-Disposition: inline In-Reply-To: <1477935289-5010-2-git-send-email-joserz@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v3 1/4] target-ppc: Implement bcdcfn. instruction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jose Ricardo Ziviani Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, nikunj@linux.vnet.ibm.com, bharata@linux.vnet.ibm.com --i0/AhcQY5QxfSsSZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 31, 2016 at 03:34:46PM -0200, Jose Ricardo Ziviani wrote: > bcdcfn. converts from National numeric format to BCD. National format > uses a byte to represent a digit where the most significant nibble is > always 0x3 and the least sign. nibbles is the digit itself. >=20 > Signed-off-by: Jose Ricardo Ziviani Sorry, couple more tweaks I've thought of. > --- > target-ppc/helper.h | 1 + > target-ppc/int_helper.c | 54 ++++++++++++++++++++++++++ > target-ppc/translate/vmx-impl.inc.c | 77 +++++++++++++++++++++++++++++++= ++++++ > target-ppc/translate/vmx-ops.inc.c | 4 +- > 4 files changed, 134 insertions(+), 2 deletions(-) >=20 > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > index 3916b2e..3b23eed 100644 > --- a/target-ppc/helper.h > +++ b/target-ppc/helper.h > @@ -371,6 +371,7 @@ DEF_HELPER_4(vpermxor, void, avr, avr, avr, avr) > =20 > DEF_HELPER_4(bcdadd, i32, avr, avr, avr, i32) > DEF_HELPER_4(bcdsub, i32, avr, avr, avr, i32) > +DEF_HELPER_3(bcdcfn, i32, avr, avr, i32) > =20 > DEF_HELPER_2(xsadddp, void, env, i32) > DEF_HELPER_2(xssubdp, void, env, i32) > diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c > index dca4798..dc7a429 100644 > --- a/target-ppc/int_helper.c > +++ b/target-ppc/int_helper.c > @@ -2429,6 +2429,8 @@ void helper_vsubecuq(ppc_avr_t *r, ppc_avr_t *a, pp= c_avr_t *b, ppc_avr_t *c) > #define BCD_NEG_PREF 0xD > #define BCD_NEG_ALT 0xB > #define BCD_PLUS_ALT_2 0xE > +#define NATIONAL_PLUS 0x2B > +#define NATIONAL_NEG 0x2D > =20 > #if defined(HOST_WORDS_BIGENDIAN) > #define BCD_DIG_BYTE(n) (15 - (n/2)) > @@ -2495,6 +2497,15 @@ static void bcd_put_digit(ppc_avr_t *bcd, uint8_t = digit, int n) > } > } > =20 > +static uint16_t get_national_digit(ppc_avr_t *reg, int n) > +{ > +#if defined(HOST_WORDS_BIGENDIAN) > + return reg->u16[8 - n]; > +#else > + return reg->u16[n]; > +#endif > +} > + > static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b) > { > int i; > @@ -2625,6 +2636,49 @@ uint32_t helper_bcdsub(ppc_avr_t *r, ppc_avr_t *a= , ppc_avr_t *b, uint32_t ps) > return helper_bcdadd(r, a, &bcopy, ps); > } > =20 > +uint32_t helper_bcdcfn(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > +{ > + int i; > + int neq_flag =3D 0; > + int cr =3D 0; > + uint16_t national =3D 0; > + uint16_t sgnb =3D get_national_digit(b, 0); > + ppc_avr_t ret =3D { .u64 =3D { 0, 0 } }; > + int invalid =3D (sgnb !=3D NATIONAL_PLUS && sgnb !=3D NATIONAL_NEG); > + > + for (i =3D 1; i < 8; i++) { > + national =3D get_national_digit(b, i); > + > + neq_flag +=3D (national !=3D 0x30); Specifically adding up the non-zero digits is a bit clunky. You're already converting the value to BCD, so you could... > + if (unlikely(national < 0x30 || national > 0x39)) { > + invalid =3D 1; > + break; > + } > + > + bcd_put_digit(&ret, national & 0xf, i); > + } > + > + if (sgnb =3D=3D NATIONAL_PLUS) { > + bcd_put_digit(&ret, (ps =3D=3D 0) ? BCD_PLUS_PREF_1 : BCD_PLUS_P= REF_2, 0); > + } else { > + bcd_put_digit(&ret, BCD_NEG_PREF, 0); > + } > + > + if (neq_flag) { =2E.. just check if the BCD output is zero here. I think that will amount to ((ret.u64[1] >> 4) !=3D 0), but it's probably worth adding a bcd_is_zero() helper for this. > + cr =3D (sgnb =3D=3D NATIONAL_PLUS) ? 1 << CRF_GT : 1 << CRF_LT; > + } else { > + cr =3D 1 << CRF_EQ; > + } > + > + if (unlikely(invalid)) { > + cr =3D 1 << CRF_SO; > + } > + > + *r =3D ret; > + > + return cr; > +} > + > void helper_vsbox(ppc_avr_t *r, ppc_avr_t *a) > { > int i; > diff --git a/target-ppc/translate/vmx-impl.inc.c b/target-ppc/translate/v= mx-impl.inc.c > index fc612d9..38fea3f 100644 > --- a/target-ppc/translate/vmx-impl.inc.c > +++ b/target-ppc/translate/vmx-impl.inc.c > @@ -945,8 +945,83 @@ static void gen_##op(DisasContext *ctx) \ > tcg_temp_free_i32(ps); \ > } > =20 > +#define GEN_BCD2(op) \ > +static void gen_##op(DisasContext *ctx) \ > +{ \ > + TCGv_ptr rd, rb; \ > + TCGv_i32 ps; \ > + \ > + if (unlikely(!ctx->altivec_enabled)) { \ > + gen_exception(ctx, POWERPC_EXCP_VPU); \ > + return; \ > + } \ > + \ > + rb =3D gen_avr_ptr(rB(ctx->opcode)); \ > + rd =3D gen_avr_ptr(rD(ctx->opcode)); \ > + \ > + ps =3D tcg_const_i32((ctx->opcode & 0x200) !=3D 0); \ > + \ > + gen_helper_##op(cpu_crf[6], rd, rb, ps); \ > + \ > + tcg_temp_free_ptr(rb); \ > + tcg_temp_free_ptr(rd); \ > + tcg_temp_free_i32(ps); \ > +} > + > GEN_BCD(bcdadd) > GEN_BCD(bcdsub) > +GEN_BCD2(bcdcfn) > + > +static void gen_xpnd04_1(DisasContext *ctx) > +{ > + switch (opc4(ctx->opcode)) { > + case 0: > + break; /* bcdctsq. */ > + case 2: > + break; /* bcdcfsq. */ > + case 4: > + break; /* bcdctz. */ > + case 5: > + break; /* bcdctn. */ > + case 6: > + break; /* bcdcfz. */ > + case 7: > + gen_bcdcfn(ctx); > + break; > + case 31: > + break; /* bcdsetsgn. */ > + default: > + gen_invalid(ctx); > + break; More important change here. I think you need to leave out the cases you haven't implemented yet, and have them all cause an invalid instruction exception. That's not correct to the ISA, obviously, but neither is treating them as a no-op. Better to have the guest get a fault, so that you know something has gone wrong. > + } > +} > + > +static void gen_xpnd04_2(DisasContext *ctx) > +{ > + switch (opc4(ctx->opcode)) { > + case 0: > + break; /* bcdctsq. */ > + case 2: > + break; /* bcdcfsq. */ > + case 4: > + break; /* bcdctz. */ > + case 6: > + break; /* bcdcfz. */ > + case 7: > + gen_bcdcfn(ctx); > + break; > + case 31: > + break; /* bcdsetsgn. */ > + default: > + gen_invalid(ctx); > + break; > + } > +} > + > +GEN_VXFORM_DUAL(vsubcuw, PPC_ALTIVEC, PPC_NONE, \ > + xpnd04_1, PPC_NONE, PPC2_ISA300) > +GEN_VXFORM_DUAL(vsubsws, PPC_ALTIVEC, PPC_NONE, \ > + xpnd04_2, PPC_NONE, PPC2_ISA300) > =20 > GEN_VXFORM_DUAL(vsububm, PPC_ALTIVEC, PPC_NONE, \ > bcdadd, PPC_NONE, PPC2_ALTIVEC_207) > @@ -1023,3 +1098,5 @@ GEN_VXFORM_DUAL(vsldoi, PPC_ALTIVEC, PPC_NONE, > #undef GEN_VXFORM_NOA > #undef GEN_VXFORM_UIMM > #undef GEN_VAFORM_PAIRED > + > +#undef GEN_BCD2 > diff --git a/target-ppc/translate/vmx-ops.inc.c b/target-ppc/translate/vm= x-ops.inc.c > index cc7ed7e..637b43c 100644 > --- a/target-ppc/translate/vmx-ops.inc.c > +++ b/target-ppc/translate/vmx-ops.inc.c > @@ -122,7 +122,7 @@ GEN_VXFORM_300(vslv, 2, 29), > GEN_VXFORM(vslo, 6, 16), > GEN_VXFORM(vsro, 6, 17), > GEN_VXFORM(vaddcuw, 0, 6), > -GEN_VXFORM(vsubcuw, 0, 22), > +GEN_VXFORM_DUAL(vsubcuw, xpnd04_1, 0, 22, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM_DUAL(vaddubs, vmul10uq, 0, 8, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM_DUAL(vadduhs, vmul10euq, 0, 9, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM(vadduws, 0, 10), > @@ -134,7 +134,7 @@ GEN_VXFORM_DUAL(vsubuhs, bcdsub, 0, 25, PPC_ALTIVEC, = PPC_NONE), > GEN_VXFORM(vsubuws, 0, 26), > GEN_VXFORM(vsubsbs, 0, 28), > GEN_VXFORM(vsubshs, 0, 29), > -GEN_VXFORM(vsubsws, 0, 30), > +GEN_VXFORM_DUAL(vsubsws, xpnd04_2, 0, 30, PPC_ALTIVEC, PPC_NONE), > GEN_VXFORM_207(vadduqm, 0, 4), > GEN_VXFORM_207(vaddcuq, 0, 5), > GEN_VXFORM_DUAL(vaddeuqm, vaddecuq, 30, 0xFF, PPC_NONE, PPC2_ALTIVEC_207= ), --=20 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 --i0/AhcQY5QxfSsSZ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYF+ImAAoJEGw4ysog2bOSe80QAMY3eLN5RfmEXs7INv3tuVwK R/EaP+r4MitJv44Usa742k8ef4e/1jq+JOuUABrALF9c+Iq0u9WMrxg+JR0vFjUF tDNlpCF8Tl38S4C9dPQZu3frak5Gu8LciR6Jhv97LdCXugBUCZ+l2jRmcGdS+GeM dHeJywA1IDMwmL5zzRwQK94famqOXYqko9Sbh9HDuj/FDwciSMuMhOHTEhu8cn7H LH5CNamqJ4bBawM+OpdwuRjgDiUIGetfbCZvhRyxpUeTT2k+8kqvKWXW93Gu+P6m +Q9edaBv0Ve5d6K+fp6IgbxnQq1eZgj0IxNkCDlN9/jjwbilzeGsUMl+AlTw3Dvw Nzs02++N0mfGRnhynI2kAauVk1x9GVzyGsdPDmTG9LA0rC/thOP9MBRFEQmjomXB hZVxMlxP/OKZhDnqWuO4HQAd4470xMX+D5msILTY/W7glOBnj4pzYUOjIyswzcmG +rX4do08gqQop4XzGoTt4OWu55yTci0LxllZfMKI8MINP0fCHs7Lk92Rva/uHbWn 6xGQIy2S8RrM12OvCXOlFui4l7w06A+Yw/x0+AVRc42mV94yK1u7bKsJaZNwhmL2 JIxHTecZxpcAOzzLfVAb9WbTlgv8/hf/fezkRX770y64jc7nmYSGEamGBrRkKr2F rk7SoX4Y1VI6pMmTkK8F =2/jV -----END PGP SIGNATURE----- --i0/AhcQY5QxfSsSZ--