From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37838) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSIqq-00079B-MF for qemu-devel@nongnu.org; Wed, 27 Jul 2016 03:01:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSIqm-0005tX-Em for qemu-devel@nongnu.org; Wed, 27 Jul 2016 03:01:55 -0400 Date: Wed, 27 Jul 2016 16:56:37 +1000 From: David Gibson Message-ID: <20160727065637.GT17429@voom.fritz.box> References: <1469534318-5549-1-git-send-email-nikunj@linux.vnet.ibm.com> <1469534318-5549-8-git-send-email-nikunj@linux.vnet.ibm.com> <20160727051904.GE17429@voom.fritz.box> <87eg6fbo5w.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> <20160727062913.GP17429@voom.fritz.box> <87zip3a8hn.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="dpEE41+QZ7HQEdc/" Content-Disposition: inline In-Reply-To: <87zip3a8hn.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> Subject: Re: [Qemu-devel] [PATCH v4 07/15] target-ppc: implement branch-less divw[o][.] List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania Cc: qemu-ppc@nongnu.org, rth@twiddle.net, qemu-devel@nongnu.org, bharata@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com --dpEE41+QZ7HQEdc/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 27, 2016 at 12:11:08PM +0530, Nikunj A Dadhania wrote: > David Gibson writes: >=20 > > [ Unknown signature status ] > > On Wed, Jul 27, 2016 at 11:47:15AM +0530, Nikunj A Dadhania wrote: > >> David Gibson writes: > >>=20 > >> > [ Unknown signature status ] > >> > On Tue, Jul 26, 2016 at 05:28:30PM +0530, Nikunj A Dadhania wrote: > >> >> While implementing modulo instructions figured out that the > >> >> implementation uses many branches. Change the logic to achieve the > >> >> branch-less code. Undefined value is set to dividend in case of inv= alid > >> >> input. > >> >>=20 > >> >> Signed-off-by: Nikunj A Dadhania > >> >> --- > >> >> target-ppc/translate.c | 48 +++++++++++++++++++++++---------------= ---------- > >> >> 1 file changed, 23 insertions(+), 25 deletions(-) > >> >>=20 > >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c > >> >> index 7c7328f..69d9ae0 100644 > >> >> --- a/target-ppc/translate.c > >> >> +++ b/target-ppc/translate.c > >> >> @@ -1049,41 +1049,39 @@ static void gen_addpcis(DisasContext *ctx) > >> >> static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, = TCGv arg1, > >> >> TCGv arg2, int sign, int comp= ute_ov) > >> >> { > >> >> - 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 t0 =3D tcg_temp_new_i32(); > >> >> + TCGv_i32 t1 =3D tcg_temp_new_i32(); > >> >> + TCGv_i32 t2 =3D tcg_temp_new_i32(); > >> >> + TCGv_i32 t3 =3D tcg_temp_new_i32(); > >> >> =20 > >> >> tcg_gen_trunc_tl_i32(t0, arg1); > >> >> tcg_gen_trunc_tl_i32(t1, arg2); > >> >> - tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1); > >> >> - 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); > >> >> - tcg_gen_div_i32(t0, t0, t1); > >> >> - } else { > >> >> - tcg_gen_divu_i32(t0, t0, t1); > >> >> - } > >> >> - if (compute_ov) { > >> >> - tcg_gen_movi_tl(cpu_ov, 0); > >> >> - } > >> >> - tcg_gen_br(l2); > >> >> - gen_set_label(l1); > >> >> if (sign) { > >> >> - tcg_gen_sari_i32(t0, t0, 31); > >> >> + tcg_gen_setcondi_i32(TCG_COND_EQ, t2, t0, INT_MIN); > >> >> + tcg_gen_setcondi_i32(TCG_COND_EQ, t3, t1, -1); > >> >> + tcg_gen_and_i32(t2, t2, t3); > >> >> + tcg_gen_setcondi_i32(TCG_COND_EQ, t3, t1, 0); > >> >> + tcg_gen_or_i32(t2, t2, t3); > >> >> + tcg_gen_movi_i32(t3, 0); > >> >> + tcg_gen_movcond_i32(TCG_COND_NE, t1, t2, t3, t2, t1); > >> >> + tcg_gen_div_i32(t3, t0, t1); > >> >> + tcg_gen_extu_i32_tl(ret, t3); > >> > > >> > Should this be a signed extend, given it's a signed divide? > >>=20 > >> Don't think so, as the instruction is 32-bit, caller will only look at > >> the 32bit and div_i32 is signed, it will take care of extending sign > >> till 32-boundary. > > > > Hrm. I thought most 32-bit arithmetic operations on Power actually > > set the underlying 64-bit registers to a sign extended version of the > > 32-bit result. >=20 > I think, when I want to operate on it as 64-bit, i will need signed > extension. rth can give more info on this. >=20 > Retained the behaviour as per the previous code as well: Ah, yes RT[0:31] <- undefined according to the ISA, so I guess this is ok (though ideally we'd double check what the actual hardware does and match that, just in case something relies on it). >=20 > tcg_gen_trunc_tl_i32(t0, arg1); > tcg_gen_trunc_tl_i32(t1, arg2); > tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1); > 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); > tcg_gen_div_i32(t0, t0, t1); > } else { > tcg_gen_divu_i32(t0, t0, t1); > } > if (compute_ov) { > tcg_gen_movi_tl(cpu_ov, 0); > } > tcg_gen_br(l2); > gen_set_label(l1); > if (sign) { > tcg_gen_sari_i32(t0, t0, 31); > } else { > tcg_gen_movi_i32(t0, 0); > } > if (compute_ov) { > tcg_gen_movi_tl(cpu_ov, 1); > tcg_gen_movi_tl(cpu_so, 1); > } > gen_set_label(l2); > tcg_gen_extu_i32_tl(ret, t0); <<<<<<<<<<<<<<<<<<<= <<<< > tcg_temp_free_i32(t0); > tcg_temp_free_i32(t1); >=20 > IMO, thats correct. >=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 --dpEE41+QZ7HQEdc/ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXmFslAAoJEGw4ysog2bOSKkwP/1Et0GFqvehG7kjDC9cLd1pd 06ZzR/LdrzNGCdgXDrU3SUqOXIXr/EmElIkphroLS6+nmr8iq3qFRZmz4ckZGpw5 zD00tNDdIV+lkP8izN+apPuGMUOmXG+O+LIMK8v/psvPLzp7eIBKoENHxuTH2zXI aIJ8rpU9d0boEJwGvZusxnvDTLjfUN1X02XEYagA7qH4AXwFVH90lQVLvPH5jEQy MkECjSCxjhDak2bz/mDIGYhctHHyPhSqTt6JW4qPsP5m05OEUIcJBx0mgvuUeI1F UMN8qjM62va5BqcCosLzxFnkqRhMr7stxIy8N8ovpMMqj2CePqBklijlciihy4Qf I89W0LeOS+DsiJ9YYRZq07gAiSAqchObiUKSsNVupBZZ48rPIDbeDzQMBBbAw+jr UXTgYFxcrncdb42R8BbZmVbSzwSBINAm/u2dubJBjVDBxM4bHj9Fu8W/VlHf4hUP l0A5VjMJF8DGqZXfzR5ExEk0zXBzuxd6bg/+c+jSkkliqWsikNfR5MVnacggueOD yBdwaXHKXpk33oOSAVSzsEEKJMzt8SrreAj0HmaMaM+YjiHLNK7meYnBy6joldj1 aH9vj5o03SO3zlbFXKbS3JYg0Joh4irxRaJvkSQPyg+XCVCm26Cq5k70c0Oxo44Z 2uknPX5tFac1pUComCRM =U1RO -----END PGP SIGNATURE----- --dpEE41+QZ7HQEdc/--