Patchwork [v2,06/10] target-arm: fix neon vsri, vshl and vsli ops

login
register
mail settings
Submitter Juha.Riihimaki@nokia.com
Date Oct. 24, 2009, 12:19 p.m.
Message ID <1256386749-85299-7-git-send-email-juha.riihimaki@nokia.com>
Download mbox | patch
Permalink /patch/36836/
State New
Headers show

Comments

Juha.Riihimaki@nokia.com - Oct. 24, 2009, 12:19 p.m.
From: Juha Riihimäki <juha.riihimaki@nokia.com>

Shift by immediate value is incorrectly overwritten by a temporary
variable in the processing of NEON vsri, vshl and vsli instructions.
This patch has been revised to also include a fix for the special
case where the code would previously try to shift an integer value
over 31 bits left/right.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
---
 target-arm/translate.c |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)
Laurent Desnogues - Oct. 24, 2009, 5:44 p.m.
On Sat, Oct 24, 2009 at 2:19 PM,  <juha.riihimaki@nokia.com> wrote:
> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>
> Shift by immediate value is incorrectly overwritten by a temporary
> variable in the processing of NEON vsri, vshl and vsli instructions.
> This patch has been revised to also include a fix for the special
> case where the code would previously try to shift an integer value
> over 31 bits left/right.
>
> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>

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


Laurent

> ---
>  target-arm/translate.c |   32 ++++++++++++++++++--------------
>  1 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index d1e2ed2..9e924d4 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4098,7 +4098,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>     int pairwise;
>     int u;
>     int n;
> -    uint32_t imm;
> +    uint32_t imm, mask;
>     TCGv tmp, tmp2, tmp3, tmp4, tmp5;
>     TCGv_i64 tmp64;
>
> @@ -4626,31 +4626,35 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                             switch (size) {
>                             case 0:
>                                 if (op == 4)
> -                                    imm = 0xff >> -shift;
> +                                    mask = 0xff >> -shift;
>                                 else
> -                                    imm = (uint8_t)(0xff << shift);
> -                                imm |= imm << 8;
> -                                imm |= imm << 16;
> +                                    mask = (uint8_t)(0xff << shift);
> +                                mask |= mask << 8;
> +                                mask |= mask << 16;
>                                 break;
>                             case 1:
>                                 if (op == 4)
> -                                    imm = 0xffff >> -shift;
> +                                    mask = 0xffff >> -shift;
>                                 else
> -                                    imm = (uint16_t)(0xffff << shift);
> -                                imm |= imm << 16;
> +                                    mask = (uint16_t)(0xffff << shift);
> +                                mask |= mask << 16;
>                                 break;
>                             case 2:
> -                                if (op == 4)
> -                                    imm = 0xffffffffu >> -shift;
> -                                else
> -                                    imm = 0xffffffffu << shift;
> +                                if (shift < -31 || shift > 31) {
> +                                    mask = 0;
> +                                } else {
> +                                    if (op == 4)
> +                                        mask = 0xffffffffu >> -shift;
> +                                    else
> +                                        mask = 0xffffffffu << shift;
> +                                }
>                                 break;
>                             default:
>                                 abort();
>                             }
>                             tmp2 = neon_load_reg(rd, pass);
> -                            tcg_gen_andi_i32(tmp, tmp, imm);
> -                            tcg_gen_andi_i32(tmp2, tmp2, ~imm);
> +                            tcg_gen_andi_i32(tmp, tmp, mask);
> +                            tcg_gen_andi_i32(tmp2, tmp2, ~mask);
>                             tcg_gen_or_i32(tmp, tmp, tmp2);
>                             dead_tmp(tmp2);
>                         }
> --
> 1.6.5
>
>
>
>

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index d1e2ed2..9e924d4 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4098,7 +4098,7 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
     int pairwise;
     int u;
     int n;
-    uint32_t imm;
+    uint32_t imm, mask;
     TCGv tmp, tmp2, tmp3, tmp4, tmp5;
     TCGv_i64 tmp64;
 
@@ -4626,31 +4626,35 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                             switch (size) {
                             case 0:
                                 if (op == 4)
-                                    imm = 0xff >> -shift;
+                                    mask = 0xff >> -shift;
                                 else
-                                    imm = (uint8_t)(0xff << shift);
-                                imm |= imm << 8;
-                                imm |= imm << 16;
+                                    mask = (uint8_t)(0xff << shift);
+                                mask |= mask << 8;
+                                mask |= mask << 16;
                                 break;
                             case 1:
                                 if (op == 4)
-                                    imm = 0xffff >> -shift;
+                                    mask = 0xffff >> -shift;
                                 else
-                                    imm = (uint16_t)(0xffff << shift);
-                                imm |= imm << 16;
+                                    mask = (uint16_t)(0xffff << shift);
+                                mask |= mask << 16;
                                 break;
                             case 2:
-                                if (op == 4)
-                                    imm = 0xffffffffu >> -shift;
-                                else
-                                    imm = 0xffffffffu << shift;
+                                if (shift < -31 || shift > 31) {
+                                    mask = 0;
+                                } else {
+                                    if (op == 4)
+                                        mask = 0xffffffffu >> -shift;
+                                    else
+                                        mask = 0xffffffffu << shift;
+                                }
                                 break;
                             default:
                                 abort();
                             }
                             tmp2 = neon_load_reg(rd, pass);
-                            tcg_gen_andi_i32(tmp, tmp, imm);
-                            tcg_gen_andi_i32(tmp2, tmp2, ~imm);
+                            tcg_gen_andi_i32(tmp, tmp, mask);
+                            tcg_gen_andi_i32(tmp2, tmp2, ~mask);
                             tcg_gen_or_i32(tmp, tmp, tmp2);
                             dead_tmp(tmp2);
                         }