From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37185) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQU6D-0005e7-1d for qemu-devel@nongnu.org; Fri, 22 Jul 2016 02:38:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQU68-0006kS-Qz for qemu-devel@nongnu.org; Fri, 22 Jul 2016 02:38:15 -0400 Date: Fri, 22 Jul 2016 16:09:48 +1000 From: David Gibson Message-ID: <20160722060948.GT15941@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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="dLXnlYbDJNCwF3YM" Content-Disposition: inline In-Reply-To: <87d1m69r69.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 --dLXnlYbDJNCwF3YM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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, TCG= v 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? If the latter, then why do you care about those cases? > >> + 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 > >> + } else { > >> + tcg_gen_movi_i32(t2, 0); > >> + } > >> + gen_set_label(l2); > >> + tcg_gen_extu_i32_tl(ret, t2); >=20 > ... Here. >=20 > Regards > Nikunj >=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 --dLXnlYbDJNCwF3YM Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXkbisAAoJEGw4ysog2bOSrDkP/RnyXKT2554puxn9Y+9+UAlp uE68sJ69AN33oQ7Zm9WQrKPWU48oLProOaSk2Ejaro2vjl9T++zGCBkzW+XsQPHJ fP2Kw0iPfO5oQtRATAZSyOuQvrJyDJPnQvlHKn4A9CLmXl7HuI8hNbxIwV5D7Pg2 W+vhzIzRNUfb+7OCbHROj36YOVJHLIs6HOrbA3yH8ZmxGeEPVMDUZvy9YeDCj3Mv Fb7OIIrLFPZZnqdENYkHW8+uGdElsHt9QeYZRLBBjvFs7w9FYXv2rYoTjMSOfIuv AokR5IT4lTd+Q0gJRW7qe3ZFq55FsYkGYRfZwYAoeFOSAObPvsJT1KXn8yQrGXE0 agTIxo0lfH6iKSNz92q7v0dLhKRSRCOEbl8CA7IphDvlkAixb2t18ZeY059jfAUk d70M/kQ+9CShEzAagmdjtSzytpt28fgeU07HDhF4OWGUbiMyreGkQQyuIJFSB91N x4340leDsFUZb8gtZBEK23nillO2QvVYJX4+gXEAxDqqprBrKmvYd+sUmHH3VG33 slsUFw0HOC222pkYJTHbiR4OCf/JghHUovbJlARO6USPZmr8DnwSitJxOohBF9q8 BKhrn9luL/2bxFeDvaptnt7QKNKJ798JY8V4RgksE2VHORorHwx54Xj6RbNVh7Tr VCTOlmPkTIACTpO3Q6NT =yXCR -----END PGP SIGNATURE----- --dLXnlYbDJNCwF3YM--