From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1N221P-0005gp-0Y for qemu-devel@nongnu.org; Sun, 25 Oct 2009 08:16:31 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1N221M-0005gV-JU for qemu-devel@nongnu.org; Sun, 25 Oct 2009 08:16:29 -0400 Received: from [199.232.76.173] (port=42068 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1N221M-0005gS-F3 for qemu-devel@nongnu.org; Sun, 25 Oct 2009 08:16:28 -0400 Received: from fg-out-1718.google.com ([72.14.220.158]:56854) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1N221M-0000z0-4M for qemu-devel@nongnu.org; Sun, 25 Oct 2009 08:16:28 -0400 Received: by fg-out-1718.google.com with SMTP id d23so3901983fga.10 for ; Sun, 25 Oct 2009 05:16:27 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1256386749-85299-11-git-send-email-juha.riihimaki@nokia.com> References: <1256386749-85299-1-git-send-email-juha.riihimaki@nokia.com> <1256386749-85299-11-git-send-email-juha.riihimaki@nokia.com> Date: Sun, 25 Oct 2009 13:16:26 +0100 Message-ID: <761ea48b0910250516r45d6e5e2k411ef2e3e766e523@mail.gmail.com> Subject: Re: [Qemu-devel] [PATCH v2 10/10] target-arm: fix neon shift helper functions From: Laurent Desnogues Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: juha.riihimaki@nokia.com Cc: qemu-devel@nongnu.org On Sat, Oct 24, 2009 at 1:19 PM, wrote: > From: Juha Riihim=E4ki > > Current code is broken at least on gcc 4.2, the result of a comparison > "-1 >=3D sizeof(type) * 8" results true and causes wrong code path to > be taken. The fix has been revised to use a type cast instead of > abs() function and extra checks. > > Signed-off-by: Juha Riihim=E4ki > --- > =A0target-arm/neon_helper.c | =A0 12 ++++++------ > =A01 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c > index f32ecd6..440b2ec 100644 > --- a/target-arm/neon_helper.c > +++ b/target-arm/neon_helper.c > @@ -392,7 +392,7 @@ NEON_VOP(abd_u32, neon_u32, 1) > =A0#define NEON_FN(dest, src1, src2) do { \ > =A0 =A0 int8_t tmp; \ > =A0 =A0 tmp =3D (int8_t)src2; \ > - =A0 =A0if (tmp >=3D sizeof(src1) * 8 || tmp <=3D -sizeof(src1) * 8) { \ > + =A0 =A0if (tmp >=3D (int)sizeof(src1) * 8 || tmp <=3D -sizeof(src1) * 8= ) { \ You should also cast the sizeof in the second comparison. With gcc 4.4.1, "1 <=3D -sizeof(uint32_t) * 8" is true. Laurent > =A0 =A0 =A0 =A0 dest =3D 0; \ > =A0 =A0 } else if (tmp < 0) { \ > =A0 =A0 =A0 =A0 dest =3D src1 >> -tmp; \ > @@ -420,7 +420,7 @@ uint64_t HELPER(neon_shl_u64)(uint64_t val, uint64_t = shiftop) > =A0#define NEON_FN(dest, src1, src2) do { \ > =A0 =A0 int8_t tmp; \ > =A0 =A0 tmp =3D (int8_t)src2; \ > - =A0 =A0if (tmp >=3D sizeof(src1) * 8) { \ > + =A0 =A0if (tmp >=3D (int)sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 dest =3D 0; \ > =A0 =A0 } else if (tmp <=3D -sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 dest =3D src1 >> (sizeof(src1) * 8 - 1); \ > @@ -453,7 +453,7 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_= t shiftop) > =A0#define NEON_FN(dest, src1, src2) do { \ > =A0 =A0 int8_t tmp; \ > =A0 =A0 tmp =3D (int8_t)src2; \ > - =A0 =A0if (tmp >=3D sizeof(src1) * 8) { \ > + =A0 =A0if (tmp >=3D (int)sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 dest =3D 0; \ > =A0 =A0 } else if (tmp < -sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 dest =3D src1 >> (sizeof(src1) * 8 - 1); \ > @@ -494,7 +494,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64= _t shiftop) > =A0#define NEON_FN(dest, src1, src2) do { \ > =A0 =A0 int8_t tmp; \ > =A0 =A0 tmp =3D (int8_t)src2; \ > - =A0 =A0if (tmp >=3D sizeof(src1) * 8 || tmp < -sizeof(src1) * 8) { \ > + =A0 =A0if (tmp >=3D (int)sizeof(src1) * 8 || tmp < -sizeof(src1) * 8) {= \ > =A0 =A0 =A0 =A0 dest =3D 0; \ > =A0 =A0 } else if (tmp =3D=3D -sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 dest =3D src1 >> (tmp - 1); \ > @@ -528,7 +528,7 @@ uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t= shiftop) > =A0#define NEON_FN(dest, src1, src2) do { \ > =A0 =A0 int8_t tmp; \ > =A0 =A0 tmp =3D (int8_t)src2; \ > - =A0 =A0if (tmp >=3D sizeof(src1) * 8) { \ > + =A0 =A0if (tmp >=3D (int)sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 if (src1) { \ > =A0 =A0 =A0 =A0 =A0 =A0 SET_QC(); \ > =A0 =A0 =A0 =A0 =A0 =A0 dest =3D ~0; \ > @@ -579,7 +579,7 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_= t val, uint64_t shiftop) > =A0#define NEON_FN(dest, src1, src2) do { \ > =A0 =A0 int8_t tmp; \ > =A0 =A0 tmp =3D (int8_t)src2; \ > - =A0 =A0if (tmp >=3D sizeof(src1) * 8) { \ > + =A0 =A0if (tmp >=3D (int)sizeof(src1) * 8) { \ > =A0 =A0 =A0 =A0 if (src1) \ > =A0 =A0 =A0 =A0 =A0 =A0 SET_QC(); \ > =A0 =A0 =A0 =A0 dest =3D src1 >> 31; \ > -- > 1.6.5 > > > >