From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52066) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9iqH-0001Po-Sz for qemu-devel@nongnu.org; Wed, 23 Nov 2016 20:28:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9iqD-0005Qu-Lk for qemu-devel@nongnu.org; Wed, 23 Nov 2016 20:28:49 -0500 Date: Thu, 24 Nov 2016 12:18:19 +1100 From: David Gibson Message-ID: <20161124011819.GL17795@umbus.fritz.box> References: <1479918105-15616-1-git-send-email-joserz@linux.vnet.ibm.com> <1479918105-15616-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="p7S+EREVcBHk3zUG" Content-Disposition: inline In-Reply-To: <1479918105-15616-2-git-send-email-joserz@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2 1/4] target-ppc: Implement bcdcfsq. 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 --p7S+EREVcBHk3zUG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 23, 2016 at 02:21:42PM -0200, Jose Ricardo Ziviani wrote: > bcdcfsq.: Decimal convert from signed quadword. It is not possible > to convert values less than 10^31-1 or greater than -10^31-1 to be > represented in packed decimal format. You have your less than / greater than the wrong way around in the above. >=20 > Signed-off-by: Jose Ricardo Ziviani > --- > target-ppc/helper.h | 1 + > target-ppc/int_helper.c | 45 +++++++++++++++++++++++++++++++= ++++++ > target-ppc/translate/vmx-impl.inc.c | 7 ++++++ > 3 files changed, 53 insertions(+) >=20 > diff --git a/target-ppc/helper.h b/target-ppc/helper.h > index da00f0a..87f533c 100644 > --- a/target-ppc/helper.h > +++ b/target-ppc/helper.h > @@ -382,6 +382,7 @@ DEF_HELPER_3(bcdcfn, i32, avr, avr, i32) > DEF_HELPER_3(bcdctn, i32, avr, avr, i32) > DEF_HELPER_3(bcdcfz, i32, avr, avr, i32) > DEF_HELPER_3(bcdctz, i32, avr, avr, i32) > +DEF_HELPER_3(bcdcfsq, 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 8886a72..751909c 100644 > --- a/target-ppc/int_helper.c > +++ b/target-ppc/int_helper.c > @@ -2874,6 +2874,51 @@ uint32_t helper_bcdctz(ppc_avr_t *r, ppc_avr_t *b,= uint32_t ps) > return cr; > } > =20 > +uint32_t helper_bcdcfsq(ppc_avr_t *r, ppc_avr_t *b, uint32_t ps) > +{ > + int cr; > + int i; > + int ox_flag =3D 0; > + uint64_t lo_value; > + uint64_t hi_value; > + uint64_t max =3D 0x38d7ea4c68000; In this case it would be clearer what's going on if you gave this constant in decimal. "max" is also not a great name - see below. > + ppc_avr_t ret =3D { .u64 =3D { 0, 0 } }; > + > + if (b->s64[HI_IDX] < 0) { > + lo_value =3D -b->s64[LO_IDX]; > + hi_value =3D ~b->u64[HI_IDX] + !lo_value; > + bcd_put_digit(&ret, 0xD, 0); > + } else { > + lo_value =3D b->u64[LO_IDX]; > + hi_value =3D b->u64[HI_IDX]; > + bcd_put_digit(&ret, bcd_preferred_sgn(0, ps), 0); > + } > + > + if (divu128(&lo_value, &hi_value, max)) { > + ox_flag =3D 1; > + } else if (lo_value >=3D max && hi_value =3D=3D 0) { This isn't right. max =3D=3D 10^15, but in fact the dividend can safely be up to 10^16 - 1. I don't see what checking the remainder against 0 has to do with anything, either. No overflow + (dividend < 10^16) should be sufficient. > + ox_flag =3D 1; > + } > + > + for (i =3D 1; hi_value; hi_value /=3D 10, i++) { > + bcd_put_digit(&ret, hi_value % 10, i); > + } > + > + for (; lo_value; lo_value /=3D 10, i++) { > + bcd_put_digit(&ret, lo_value % 10, i); > + } > + > + cr =3D bcd_cmp_zero(&ret); > + > + if (unlikely(ox_flag)) { > + cr |=3D 1 << CRF_SO; Since you posted I've merged a patch from Nikunj which chanes the meaning of these CRF_* flags to be bit masks instead of shifts. So this will need to become just cr |=3D 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 7143eb3..36141e5 100644 > --- a/target-ppc/translate/vmx-impl.inc.c > +++ b/target-ppc/translate/vmx-impl.inc.c > @@ -989,10 +989,14 @@ GEN_BCD2(bcdcfn) > GEN_BCD2(bcdctn) > GEN_BCD2(bcdcfz) > GEN_BCD2(bcdctz) > +GEN_BCD2(bcdcfsq) > =20 > static void gen_xpnd04_1(DisasContext *ctx) > { > switch (opc4(ctx->opcode)) { > + case 2: > + gen_bcdcfsq(ctx); > + break; > case 4: > gen_bcdctz(ctx); > break; > @@ -1014,6 +1018,9 @@ static void gen_xpnd04_1(DisasContext *ctx) > static void gen_xpnd04_2(DisasContext *ctx) > { > switch (opc4(ctx->opcode)) { > + case 2: > + gen_bcdcfsq(ctx); > + break; > case 4: > gen_bcdctz(ctx); > break; --=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 --p7S+EREVcBHk3zUG Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYNj/YAAoJEGw4ysog2bOSLnkP/RZLon74I/pNeO3jTNhbPzRd ROJtT5Gtv0qY+uQIFm9MQqCs2YeFFPTAtHpccIfj4047Pi4PW9ddJ+Nkk6P/WN3M oUOm7B4U4KwPcAo0oSgn/H06+zjhmkPG6H601NuxqgZuw0FJ7cAUw18MYkXNHehp KmCDjkcsGM/meVlKI9x+Sccdc3YfDi1+nCbzskl46zmz1djpcJl8t/WlJQpRd9UG idkoxDGYhABo9I2/UHvF3DkgOj03yLje7j1uMO4NNnm+FGTt+TOLNksg3ePIRqOD X3kN1qdeGl+F0ArwmsqWyi51X2ZdVQ5LgvkgZ9V6JmdPEk0VBz++O7kGOhoQrIbw oQtDbhA+vnpRmYSYjBNX1be0srEAWzFYpDzg6uUK96JDyieVmcnv06QeiXrB6CvF diLyfIsMfh1TeFwvnu+QUdJPgwciPgux0w+790b4Tp4WpyVeP7Nt1Y5DG5xBuS2D G1Zg9PhZw0/hfrD35LJMlp2W1dhJtLszNucpo+PkwFZJYfQNY6NSCcwNuyQc5cpD r3NiqAVyuBPx46XOZ8rgh8ZgyOoKSVrAJqBuiakEfTjJgvwPVkOU4PK0swGG+Rj1 o7qNHKF9vnWGqJKDmPlP53drkfDCLnwZiTdN5otBLfWzdlFMR0B99DS5VEhKAdRE kLaXn5iEa7CJoLus3NYH =zU63 -----END PGP SIGNATURE----- --p7S+EREVcBHk3zUG--