From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40594) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSJ35-0002eJ-Jb for qemu-devel@nongnu.org; Wed, 27 Jul 2016 03:14:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSJ2z-0000cT-Dd for qemu-devel@nongnu.org; Wed, 27 Jul 2016 03:14:35 -0400 Date: Wed, 27 Jul 2016 17:02:39 +1000 From: David Gibson Message-ID: <20160727070239.GU17429@voom.fritz.box> References: <1469561218-3067-1-git-send-email-nikunj@linux.vnet.ibm.com> <1469561218-3067-6-git-send-email-nikunj@linux.vnet.ibm.com> <20160727060522.GK17429@voom.fritz.box> <8760rrbni2.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> <20160727064823.GS17429@voom.fritz.box> <87wpk7a7qg.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="nk27jClX7wGPY2hW" Content-Disposition: inline In-Reply-To: <87wpk7a7qg.fsf@abhimanyu.i-did-not-set--mail-host-address--so-tickle-me> Subject: Re: [Qemu-devel] [PATCH RFC v0 5/6] target-ppc: add vsrv instruction 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, benh@kernel.crashing.org, Vivek Andrew Sha --nk27jClX7wGPY2hW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 27, 2016 at 12:27:27PM +0530, Nikunj A Dadhania wrote: > David Gibson writes: >=20 > > [ Unknown signature status ] > > On Wed, Jul 27, 2016 at 12:01:33PM +0530, Nikunj A Dadhania wrote: > >> David Gibson writes: > >>=20 > >> > [ Unknown signature status ] > >> > On Wed, Jul 27, 2016 at 12:56:57AM +0530, Nikunj A Dadhania wrote: > >> >> From: Vivek Andrew Sha > >> >>=20 > >> >> Adds Vector Shift Right Variable instruction. > >> >>=20 > >> >> Signed-off-by: Vivek Andrew Sha > >> >> Signed-off-by: Nikunj A Dadhania > >> >> --- > >> >> target-ppc/helper.h | 1 + > >> >> target-ppc/int_helper.c | 17 +++++++++++++++++ > >> >> target-ppc/translate.c | 2 ++ > >> >> 3 files changed, 20 insertions(+) > >> >>=20 > >> >> diff --git a/target-ppc/helper.h b/target-ppc/helper.h > >> >> index 9703f85..8eada2f 100644 > >> >> --- a/target-ppc/helper.h > >> >> +++ b/target-ppc/helper.h > >> >> @@ -211,6 +211,7 @@ DEF_HELPER_3(vslw, void, avr, avr, avr) > >> >> DEF_HELPER_3(vsld, void, avr, avr, avr) > >> >> DEF_HELPER_3(vslo, void, avr, avr, avr) > >> >> DEF_HELPER_3(vsro, void, avr, avr, avr) > >> >> +DEF_HELPER_3(vsrv, void, avr, avr, avr) > >> >> DEF_HELPER_3(vslv, void, avr, avr, avr) > >> >> DEF_HELPER_3(vaddcuw, void, avr, avr, avr) > >> >> DEF_HELPER_3(vsubcuw, void, avr, avr, avr) > >> >> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c > >> >> index 412398f..f4776f0 100644 > >> >> --- a/target-ppc/int_helper.c > >> >> +++ b/target-ppc/int_helper.c > >> >> @@ -1722,6 +1722,23 @@ void helper_vslv(ppc_avr_t *r, ppc_avr_t *a,= ppc_avr_t *b) > >> >> } > >> >> } > >> >> =20 > >> >> +void helper_vsrv(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b) > >> >> +{ > >> >> + int i; > >> >> + unsigned int shift, bytes, src[ARRAY_SIZE(r->u8) + 1]; > >> >> + > >> >> + src[0] =3D 0; > >> >> + for (i =3D 0; i < ARRAY_SIZE(r->u8); i++) { > >> >> + src[i + 1] =3D a->u8[i]; > >> >> + } > >> >> + > >> >> + for (i =3D 0; i < ARRAY_SIZE(r->u8); i++) { > >> >> + shift =3D b->u8[i] & 0x7; /* extract shift v= alue */ > >> >> + bytes =3D (src[i] << 8) + src[i + 1]; /* extract adjac= ent bytes */ > >> > > >> > I think you should be able to construct bytes on the fly without > >> > pre-generating teh whole of src, as you already did for vslv. > >>=20 > >> Had done that, but that introduces a bug like this, for eg: > >>=20 > >> vslv vra,vra,vrb > >>=20 > >> So modified vra->u8[i] is used during subsequent operation as input. > >>=20 > >> Assuming I take care or special casing "0": > >>=20 > >> bytes =3D ((vra->u8[i - 1] << 8) | (vra->u8[i]))=20 > >> vra->u8[i] =3D (bytes >> shift) & 0xFF; > >>=20 > >> when i =3D 1, bytes will ((vra->u8[0] << 8) | (vra->u8[1])). But vra-= >u8[0], > >> was changed in the previous operation. > > > > Ah, good point. > > > >> Thats the reason src[] is needed > > > > It's probably possible to avoid generating all of src by instead > > generating the bytes one loop iteration ahead, but that sounds fiddly, > > so the current approach is fine for now. >=20 > Or do the operation in the reverse: starting from size to 0. Let me try > that. Good idea, that should work. --=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 --nk27jClX7wGPY2hW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXmFyPAAoJEGw4ysog2bOSIocQAMHi+l21fbz5de8AJHd8Zcy1 u8hdiy/5ZhbXa3DgLDsFsfdkYJPah+LaVOyvYwrMhu5dmZ336SAXmaUtm4G3Gxbp Y+gbjegnTwCtozQGzsB+q7TtSrJmm5E6TWiSSl0cQWbQqgv9nJ+5axgpbrTgJ6To HcbFVz2POfvOf6kxbx3ebIRE+4YAgda98+PKvBkXt1A+58RT1D+1gzZsrY5mLL2R rJIFZajgGNrykgE0Mx1oHORdneIGF0viOtNeo0SNLLfCdH1By0ml7bsLqSkUZ9Mu QhmNBSj2TNlMDztECMwpDP1ChG1xy7Kxqq2Gn2v8PrfA6y4hNLzPGBJFr/CB5tq0 ocg8+UekAoQqWwVl7hnSV2jxYeCE5DhoKa90jpF6AXs82PrmH0H90lMO20Kpqk// TfzJDn9a43jLt0A20t38urackONTqVxug69bwIv0eKrRu5AIOIiPhWLx9zqBblEd XCYZvWubMAJvQEqAFig6FQZK/uOKi+AcZRIFTuuFWYhtNYVR84bVCGkuJakt0+Qb UHofNUH31hh57Uruj+c4DQff3qI78vT7yJuh0NsLa5nlto1zlL7Ja1DNo+k8Ojfs +/lp+5uOfDSL+3fD5syjAAgjUmLwDBQwSrdAh+jyrBU03xLaNRBbPpdEhS0ck4q4 m0olX9FTOSdlQSBlZf5z =1/Yb -----END PGP SIGNATURE----- --nk27jClX7wGPY2hW--