From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=54405 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Pp2V8-0004BR-8H
	for qemu-devel@nongnu.org; Mon, 14 Feb 2011 12:46:19 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Pp2V7-000068-05
	for qemu-devel@nongnu.org; Mon, 14 Feb 2011 12:46:18 -0500
Received: from mail-iw0-f173.google.com ([209.85.214.173]:55852)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Pp2V6-00005y-NN
	for qemu-devel@nongnu.org; Mon, 14 Feb 2011 12:46:16 -0500
Received: by iwc10 with SMTP id 10so5487360iwc.4
	for <qemu-devel@nongnu.org>; Mon, 14 Feb 2011 09:46:15 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <1297437062-6118-5-git-send-email-christophe.lyon@st.com>
References: <1297437062-6118-1-git-send-email-christophe.lyon@st.com>
	<1297437062-6118-5-git-send-email-christophe.lyon@st.com>
Date: Mon, 14 Feb 2011 17:46:15 +0000
Message-ID: <AANLkTimbqACSVJGjnd6wmHb3x2LDW=dBqFrT63xi7gbi@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH 4/6] target-arm: fix saturated values for
	Neon right shifts.
From: Peter Maydell <peter.maydell@linaro.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: christophe.lyon@st.com
Cc: qemu-devel@nongnu.org

On 11 February 2011 15:11,  <christophe.lyon@st.com> wrote:

> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -903,7 +903,7 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64=
_t val, uint64_t shiftop)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 dest =3D src1 << tmp; \
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((dest >> tmp) !=3D src1) { \
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SET_QC(); \
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dest =3D src1 >> 31; \
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dest =3D (uint32_t)(1 << (size=
of(src1) * 8 - 1)) - (src1 > 0 ? 1 : 0); \
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 } \
> =C2=A0 =C2=A0 }} while (0)

This gives the right values but is a pretty long expression (among
other things it is more than 80 chars and breaks the QEMU coding
style). I'd rather do the same as the existing qshl_s* helper:

            dest =3D (uint32_t)(1 << (sizeof(src1) * 8 - 1)); \
            if (src1 > 0) { \
                dest--; \
            } \

> =C2=A0NEON_VOP_ENV(qrshl_s8, neon_s8, 4)
> @@ -924,7 +924,11 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint3=
2_t valop, uint32_t shiftop)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 dest =3D val << shift;
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((dest >> shift) !=3D val) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 SET_QC();
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dest =3D (uint32_t)(1 << (size=
of(val) * 8 - 1)) - (val > 0 ? 1 : 0);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (val < 0) {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dest =3D INT32_M=
IN;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dest =3D INT32_M=
AX;
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}

Again, right answers but the way most of the rest of the code
forces a 32 bit value to signed saturation is
   dest =3D (val >> 31) ^ ~SIGNBIT;

-- PMM