Patchwork [v3] target-arm: fix neon shift helper functions

login
register
mail settings
Submitter Juha.Riihimaki@nokia.com
Date Oct. 26, 2009, 7:01 a.m.
Message ID <1256540467-87779-1-git-send-email-juha.riihimaki@nokia.com>
Download mbox | patch
Permalink /patch/36884/
State New
Headers show

Comments

Juha.Riihimaki@nokia.com - Oct. 26, 2009, 7:01 a.m.
From: Juha Riihimäki <juha.riihimaki@nokia.com>

Current code is broken at least on recent compilers, comparison
between signed and unsigned types yield incorrect code and render
the neon shift helper functions defunct. This is the third revision
of this patch, casting all comparisons with the sizeof operator to
signed ssize_t type to force comparisons to be between signed integral
types.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
---
 target-arm/neon_helper.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)
Laurent Desnogues - Oct. 27, 2009, 7:17 a.m.
On Mon, Oct 26, 2009 at 8:01 AM,  <juha.riihimaki@nokia.com> wrote:
> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>
> Current code is broken at least on recent compilers, comparison
> between signed and unsigned types yield incorrect code and render
> the neon shift helper functions defunct. This is the third revision
> of this patch, casting all comparisons with the sizeof operator to
> signed ssize_t type to force comparisons to be between signed integral
> types.
>
> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>

Acked-by: Laurent Desnogues <laurent.desnogues@gmail.com>


Laurent

> ---
>  target-arm/neon_helper.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index f32ecd6..5e6452b 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -392,7 +392,8 @@ NEON_VOP(abd_u32, neon_u32, 1)
>  #define NEON_FN(dest, src1, src2) do { \
>     int8_t tmp; \
>     tmp = (int8_t)src2; \
> -    if (tmp >= sizeof(src1) * 8 || tmp <= -sizeof(src1) * 8) { \
> +    if (tmp >= (ssize_t)sizeof(src1) * 8 || \
> +        tmp <= -(ssize_t)sizeof(src1) * 8) { \
>         dest = 0; \
>     } else if (tmp < 0) { \
>         dest = src1 >> -tmp; \
> @@ -420,9 +421,9 @@ uint64_t HELPER(neon_shl_u64)(uint64_t val, uint64_t shiftop)
>  #define NEON_FN(dest, src1, src2) do { \
>     int8_t tmp; \
>     tmp = (int8_t)src2; \
> -    if (tmp >= sizeof(src1) * 8) { \
> +    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
>         dest = 0; \
> -    } else if (tmp <= -sizeof(src1) * 8) { \
> +    } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
>         dest = src1 >> (sizeof(src1) * 8 - 1); \
>     } else if (tmp < 0) { \
>         dest = src1 >> -tmp; \
> @@ -453,11 +454,11 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
>  #define NEON_FN(dest, src1, src2) do { \
>     int8_t tmp; \
>     tmp = (int8_t)src2; \
> -    if (tmp >= sizeof(src1) * 8) { \
> +    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
>         dest = 0; \
> -    } else if (tmp < -sizeof(src1) * 8) { \
> +    } else if (tmp < -(ssize_t)sizeof(src1) * 8) { \
>         dest = src1 >> (sizeof(src1) * 8 - 1); \
> -    } else if (tmp == -sizeof(src1) * 8) { \
> +    } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
>         dest = src1 >> (tmp - 1); \
>         dest++; \
>         dest >>= 1; \
> @@ -494,9 +495,10 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
>  #define NEON_FN(dest, src1, src2) do { \
>     int8_t tmp; \
>     tmp = (int8_t)src2; \
> -    if (tmp >= sizeof(src1) * 8 || tmp < -sizeof(src1) * 8) { \
> +    if (tmp >= (ssize_t)sizeof(src1) * 8 || \
> +        tmp < -(ssize_t)sizeof(src1) * 8) { \
>         dest = 0; \
> -    } else if (tmp == -sizeof(src1) * 8) { \
> +    } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
>         dest = src1 >> (tmp - 1); \
>     } else if (tmp < 0) { \
>         dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
> @@ -528,14 +530,14 @@ uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
>  #define NEON_FN(dest, src1, src2) do { \
>     int8_t tmp; \
>     tmp = (int8_t)src2; \
> -    if (tmp >= sizeof(src1) * 8) { \
> +    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
>         if (src1) { \
>             SET_QC(); \
>             dest = ~0; \
>         } else { \
>             dest = 0; \
>         } \
> -    } else if (tmp <= -sizeof(src1) * 8) { \
> +    } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
>         dest = 0; \
>     } else if (tmp < 0) { \
>         dest = src1 >> -tmp; \
> @@ -579,11 +581,11 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
>  #define NEON_FN(dest, src1, src2) do { \
>     int8_t tmp; \
>     tmp = (int8_t)src2; \
> -    if (tmp >= sizeof(src1) * 8) { \
> +    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
>         if (src1) \
>             SET_QC(); \
>         dest = src1 >> 31; \
> -    } else if (tmp <= -sizeof(src1) * 8) { \
> +    } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
>         dest = src1 >> 31; \
>     } else if (tmp < 0) { \
>         dest = src1 >> -tmp; \
> --
> 1.6.5
>
>
>
>

Patch

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index f32ecd6..5e6452b 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -392,7 +392,8 @@  NEON_VOP(abd_u32, neon_u32, 1)
 #define NEON_FN(dest, src1, src2) do { \
     int8_t tmp; \
     tmp = (int8_t)src2; \
-    if (tmp >= sizeof(src1) * 8 || tmp <= -sizeof(src1) * 8) { \
+    if (tmp >= (ssize_t)sizeof(src1) * 8 || \
+        tmp <= -(ssize_t)sizeof(src1) * 8) { \
         dest = 0; \
     } else if (tmp < 0) { \
         dest = src1 >> -tmp; \
@@ -420,9 +421,9 @@  uint64_t HELPER(neon_shl_u64)(uint64_t val, uint64_t shiftop)
 #define NEON_FN(dest, src1, src2) do { \
     int8_t tmp; \
     tmp = (int8_t)src2; \
-    if (tmp >= sizeof(src1) * 8) { \
+    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
         dest = 0; \
-    } else if (tmp <= -sizeof(src1) * 8) { \
+    } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
         dest = src1 >> (sizeof(src1) * 8 - 1); \
     } else if (tmp < 0) { \
         dest = src1 >> -tmp; \
@@ -453,11 +454,11 @@  uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
 #define NEON_FN(dest, src1, src2) do { \
     int8_t tmp; \
     tmp = (int8_t)src2; \
-    if (tmp >= sizeof(src1) * 8) { \
+    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
         dest = 0; \
-    } else if (tmp < -sizeof(src1) * 8) { \
+    } else if (tmp < -(ssize_t)sizeof(src1) * 8) { \
         dest = src1 >> (sizeof(src1) * 8 - 1); \
-    } else if (tmp == -sizeof(src1) * 8) { \
+    } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
         dest = src1 >> (tmp - 1); \
         dest++; \
         dest >>= 1; \
@@ -494,9 +495,10 @@  uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
 #define NEON_FN(dest, src1, src2) do { \
     int8_t tmp; \
     tmp = (int8_t)src2; \
-    if (tmp >= sizeof(src1) * 8 || tmp < -sizeof(src1) * 8) { \
+    if (tmp >= (ssize_t)sizeof(src1) * 8 || \
+        tmp < -(ssize_t)sizeof(src1) * 8) { \
         dest = 0; \
-    } else if (tmp == -sizeof(src1) * 8) { \
+    } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
         dest = src1 >> (tmp - 1); \
     } else if (tmp < 0) { \
         dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
@@ -528,14 +530,14 @@  uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
 #define NEON_FN(dest, src1, src2) do { \
     int8_t tmp; \
     tmp = (int8_t)src2; \
-    if (tmp >= sizeof(src1) * 8) { \
+    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
         if (src1) { \
             SET_QC(); \
             dest = ~0; \
         } else { \
             dest = 0; \
         } \
-    } else if (tmp <= -sizeof(src1) * 8) { \
+    } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
         dest = 0; \
     } else if (tmp < 0) { \
         dest = src1 >> -tmp; \
@@ -579,11 +581,11 @@  uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
 #define NEON_FN(dest, src1, src2) do { \
     int8_t tmp; \
     tmp = (int8_t)src2; \
-    if (tmp >= sizeof(src1) * 8) { \
+    if (tmp >= (ssize_t)sizeof(src1) * 8) { \
         if (src1) \
             SET_QC(); \
         dest = src1 >> 31; \
-    } else if (tmp <= -sizeof(src1) * 8) { \
+    } else if (tmp <= -(ssize_t)sizeof(src1) * 8) { \
         dest = src1 >> 31; \
     } else if (tmp < 0) { \
         dest = src1 >> -tmp; \