From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQUfV-00044d-UE for qemu-devel@nongnu.org; Fri, 22 Jul 2016 03:14:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQUfR-00064E-LQ for qemu-devel@nongnu.org; Fri, 22 Jul 2016 03:14:45 -0400 Date: Fri, 22 Jul 2016 17:12:29 +1000 From: David Gibson Message-ID: <20160722071229.GW15941@voom.fritz.box> References: <1468861517-2508-1-git-send-email-nikunj@linux.vnet.ibm.com> <1468861517-2508-6-git-send-email-nikunj@linux.vnet.ibm.com> <20160722045137.GO15941@voom.fritz.box> <87d1m69r69.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> <20160722060948.GT15941@voom.fritz.box> <874m7i9n7k.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="p609JBPwWeYlfsbE" Content-Disposition: inline In-Reply-To: <874m7i9n7k.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> Subject: Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com, benh@kernel.crashing.org --p609JBPwWeYlfsbE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 22, 2016 at 12:24:55PM +0530, Nikunj A Dadhania wrote: > David Gibson writes: >=20 > > [ Unknown signature status ] > > On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote: > >> David Gibson writes: > >>=20 > >> > [ Unknown signature status ] > >> > On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote: > >> >> Adding following instructions: > >> >>=20 > >> >> moduw: Modulo Unsigned Word > >> >> modsw: Modulo Signed Word > >> >>=20 > >> >> Signed-off-by: Nikunj A Dadhania > >> > > >> > As rth has already mentioned this many branches probably means this > >> > wants a helper. > >> > > >> >> --- > >> >> target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++= ++++++++++ > >> >> 1 file changed, 48 insertions(+) > >> >>=20 > >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c > >> >> index d44f7af..487dd94 100644 > >> >> --- a/target-ppc/translate.c > >> >> +++ b/target-ppc/translate.c > >> >> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0); > >> >> GEN_DIVE(divdeo, divde, 1); > >> >> #endif > >> >> =20 > >> >> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, = TCGv arg1, > >> >> + TCGv arg2, int sign) > >> >> +{ > >> >> + TCGLabel *l1 =3D gen_new_label(); > >> >> + TCGLabel *l2 =3D gen_new_label(); > >> >> + TCGv_i32 t0 =3D tcg_temp_local_new_i32(); > >> >> + TCGv_i32 t1 =3D tcg_temp_local_new_i32(); > >> >> + TCGv_i32 t2 =3D tcg_temp_local_new_i32(); > >> >> + > >> >> + tcg_gen_trunc_tl_i32(t0, arg1); > >> >> + tcg_gen_trunc_tl_i32(t1, arg2); > >> >> + tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1); > >>=20 > >> Result for: > >> % 0 and ... > >>=20 > >> >> + if (sign) { > >> >> + TCGLabel *l3 =3D gen_new_label(); > >> >> + tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3); > >> >> + tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1); > >> >> + gen_set_label(l3); > >> > > >> > It's not really clear to be what the logic above is doing. > >>=20 > >> ... For signed case > >> 0x8000_0000 % -1 > >>=20 > >> Is undefined, addressing those cases. > > > > Do you mean the tcg operations have undefined results or that the ppc > > instructions have undefined results? >=20 > TCG side, I haven't tried. >=20 > > If the latter, then why do you care about those cases? >=20 > Thats how divd is implemented as well, i didn't want to break that. I am > looking at doing both div and mod as helpers. >=20 > >> >> + tcg_gen_rem_i32(t2, t0, t1); > >> >> + } else { > >> >> + tcg_gen_remu_i32(t2, t0, t1); > >> >> + } > >> >> + tcg_gen_br(l2); > >> >> + gen_set_label(l1); > >> >> + if (sign) { > >> >> + tcg_gen_sari_i32(t2, t0, 31); > >> > > >> > AFAICT this sets t2 to either 0 or -1 depending on the sign of t0, > >> > which seems like an odd thing to do. > >>=20 > >> Extending the sign later ... > > > > Right, so after sign extension you have a 64-bit 0 or -1. Still not > > seeing what that 0 or -1 result is useful for. >=20 > Oh ok, i got why you got confused. I am re-writing all of it though, but > for understanding: >=20 > if (divisor =3D=3D 0) > goto l1; >=20 > if (signed) { > if (divisor =3D=3D -1 && dividend =3D=3D INT_MIN) > goto l1; > compute_signed_rem(t2, t0, t1); > } else { > compute_unsigned_rem(t2, t0, t1); =20 > } > goto l2; /* jump to setting extending result and return */ >=20 > l1: /* in case of invalid input set values */ > if (signed) > t2 =3D -1 or 0; > else > t2 =3D 0; Ok, so why do you ever need different result values in the case of invalid input? Why is always returning 0 not good enough? > l2: > set (ret, t2) >=20 > Regards > Nikunj >=20 >=20 --=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 --p609JBPwWeYlfsbE Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXkcddAAoJEGw4ysog2bOSUNoP/1sinUzqKnyXOKEERUU41UUL AX5gec2bx2jDZrzEKPGdH2v1EUAWoQQc9Sbdpb6ZvjjV0TzMY2YIT8M/zdYt81Fy WL1SRqIV3Lfe5UZx2WN+9m7n4ccy6XULV3K3wtRUycOR4yABY4NQWW+k2RaAocgp iE40yMzWQ3eplBunFzTJ8Qk9vOMxMeWINwKqi+zlY2N8Lvghz+oli+BkB+qvwDmz w9iTZL3/qD/1wVKiXhS52VLK13ZcRbdWiw8F0dyqpV3bU7Khsug3xQ/1EWBrCQ/a kmcex75YOt8OI2I7I+o5v9r0dlIojZ/WBx3mIq+7lyS2avTr+j25fn3hiW4K/k3D 6/G/cr7uc9NLflvlvKSLOlejj1c8mrF2he3Z748yBsJbsI6Rk7cEbhsij3lKa1ec M5BpKSGavzIx24I351864xyT+EU2sKEGnmYprhYA+KF2YjVWLcr7eq/1FO5Faj4N yR6+3IvZz0abNDYnmS8YWInChX70dNSiDomwITWcuk1uxVQMjkqhxwwSkWWn5Vtk /U5Y/Fec6wGcTmbtTx65k+uN0S2zry6xEOvngedaoOHT2tUrO8IddSYjxRfdWJKC F+aDvmGGTxfYRydXLCArmZx2zJvpsVZSV6d4G25TBdbxp1cX6WUR/kXNYYgGfk9u kmWW2g9ATf5/BtsyYg2C =m7/+ -----END PGP SIGNATURE----- --p609JBPwWeYlfsbE--