[Qemu-devel] [PATCH 1/4] target-arm: Add support for PKHxx in thumb2

[Qemu-devel] [PATCH 1/4] target-arm: Add support for PKHxx in thumb2

[Johan Bengtsson]

The PKHxx instructions were not recognized by the thumb2 decoder. The
solution provided in this changeset is identical to the arm-mode
implementation.

Signed-off-by: Johan Bengtsson <teofrastius@gmail.com>
---
 target-arm/translate.c |   63 ++++++++++++++++++++++++++++++++++-------------
 1 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 6fcdd7e..f39efc5 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7601,27 +7601,54 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
             }
         }
         break;
-    case 5: /* Data processing register constant shift.  */
-        if (rn == 15) {
-            tmp = new_tmp();
-            tcg_gen_movi_i32(tmp, 0);
-        } else {
-            tmp = load_reg(s, rn);
-        }
-        tmp2 = load_reg(s, rm);
+    case 5: 
+
         op = (insn >> 21) & 0xf;
-        shiftop = (insn >> 4) & 3;
-        shift = ((insn >> 6) & 3) | ((insn >> 10) & 0x1c);
-        conds = (insn & (1 << 20)) != 0;
-        logic_cc = (conds && thumb2_logic_op(op));
-        gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
-        if (gen_thumb2_data_op(s, op, conds, 0, tmp, tmp2))
-            goto illegal_op;
-        dead_tmp(tmp2);
-        if (rd != 15) {
+        if (op == 6) {
+            /* Halfword pack.  */
+            tmp = load_reg(s, rn);
+            tmp2 = load_reg(s, rm);
+            shift = ((insn >> 10) & 0x1c) | ((insn >> 6) & 0x3);
+            if (insn & (1 << 5)) {
+                /* pkhtb */
+                if (shift == 0)
+                    shift = 31;
+                tcg_gen_sari_i32(tmp2, tmp2, shift);
+                tcg_gen_andi_i32(tmp, tmp, 0xffff0000);
+                tcg_gen_ext16u_i32(tmp2, tmp2);
+            } else {
+                /* pkhbt */
+                if (shift)
+                    tcg_gen_shli_i32(tmp2, tmp2, shift);
+                tcg_gen_ext16u_i32(tmp, tmp);
+                tcg_gen_andi_i32(tmp2, tmp2, 0xffff0000);
+            }
+            tcg_gen_or_i32(tmp, tmp, tmp2);
+            dead_tmp(tmp2);
             store_reg(s, rd, tmp);
         } else {
-            dead_tmp(tmp);
+            /* Data processing register constant shift.  */
+            if (rn == 15) {
+                tmp = new_tmp();
+                tcg_gen_movi_i32(tmp, 0);
+            } else {
+                tmp = load_reg(s, rn);
+            }
+            tmp2 = load_reg(s, rm);
+            
+            shiftop = (insn >> 4) & 3;
+            shift = ((insn >> 6) & 3) | ((insn >> 10) & 0x1c);
+            conds = (insn & (1 << 20)) != 0;
+            logic_cc = (conds && thumb2_logic_op(op));
+            gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
+            if (gen_thumb2_data_op(s, op, conds, 0, tmp, tmp2))
+                goto illegal_op;
+            dead_tmp(tmp2);
+            if (rd != 15) {
+                store_reg(s, rd, tmp);
+            } else {
+                dead_tmp(tmp);
+            }
         }
         break;
     case 13: /* Misc data processing.  */
-- 
1.7.0.4

[Qemu-devel] [PATCH 2/4] target-arm: Fix mixup in decoding of saturating add and sub

[Johan Bengtsson]

The thumb2 decoder contained a mixup between the bit controlling
doubling and the bit controlling if the operation was an add or a sub.

Signed-off-by: Johan Bengtsson <teofrastius@gmail.com>
---
 target-arm/translate.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index f39efc5..b530a53 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7713,9 +7713,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 /* Saturating add/subtract.  */
                 tmp = load_reg(s, rn);
                 tmp2 = load_reg(s, rm);
-                if (op & 2)
-                    gen_helper_double_saturate(tmp, tmp);
                 if (op & 1)
+                    gen_helper_double_saturate(tmp, tmp);
+                if (op & 2)
                     gen_helper_sub_saturate(tmp, tmp2, tmp);
                 else
                     gen_helper_add_saturate(tmp, tmp, tmp2);
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH 2/4] target-arm: Fix mixup in decoding of saturating add and sub

[Peter Maydell]

On 11 October 2010 09:18, Johan Bengtsson <teofrastius@gmail.com> wrote:
> The thumb2 decoder contained a mixup between the bit controlling
> doubling and the bit controlling if the operation was an add or a sub.
>
> Signed-off-by: Johan Bengtsson <teofrastius@gmail.com>

I've confirmed against the ARM ARM that this patch matches
the T1 encodings of QADD, QDADD, QSUB, QDSUB, and
have tested that once the patch is applied qemu gives identical
results to the hardware for execution of these instructions.

Acked-by: Peter Maydell <peter.maydell@linaro.org>

> ---
>  target-arm/translate.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index f39efc5..b530a53 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7713,9 +7713,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                 /* Saturating add/subtract.  */
>                 tmp = load_reg(s, rn);
>                 tmp2 = load_reg(s, rm);
> -                if (op & 2)
> -                    gen_helper_double_saturate(tmp, tmp);
>                 if (op & 1)
> +                    gen_helper_double_saturate(tmp, tmp);
> +                if (op & 2)
>                     gen_helper_sub_saturate(tmp, tmp2, tmp);
>                 else
>                     gen_helper_add_saturate(tmp, tmp, tmp2);
> --
> 1.7.0.4
>
>
>

[Qemu-devel] [PATCH 4/4] target-arm: Fix problems with VCVT fixpoint conversion

[Johan Bengtsson]

There were two problems with VCVT fixpoint conversion. The most grave was that
the micro-ops sequence generated by the instruction triggered a failed
assertion in tcg. The second problem was that the extraction of the fraction
field from the opcode was erroneous.

Signed-off-by: Johan Bengtsson <teofrastius@gmail.com>
---
 target-arm/translate.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index b530a53..652cac9 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2877,7 +2877,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     VFP_DREG_D(rd, insn);
                 }
 
-                if (op == 15 && (rn == 16 || rn == 17)) {
+                if (op == 15 && ((rn & 0x14) == 0x14)) {
                     /* Integer source.  */
                     rm = ((insn << 1) & 0x1e) | ((insn >> 5) & 1);
                 } else {
@@ -3179,7 +3179,7 @@ static int disas_vfp_insn(CPUState * env, DisasContext *s, uint32_t insn)
                 /* Write back the result.  */
                 if (op == 15 && (rn >= 8 && rn <= 11))
                     ; /* Comparison, do nothing.  */
-                else if (op == 15 && rn > 17)
+                else if (op == 15 && rn > 17 && ((rn & 0x14) != 0x14))
                     /* Integer result.  */
                     gen_mov_vreg_F0(0, rd);
                 else if (op == 15 && rn == 15)
-- 
1.7.0.4

Re: [Qemu-devel] [PATCH 1/4] target-arm: Add support for PKHxx in thumb2

[Peter Maydell]

On 11 October 2010 09:18, Johan Bengtsson <teofrastius@gmail.com> wrote:
> The PKHxx instructions were not recognized by the thumb2 decoder. The
> solution provided in this changeset is identical to the arm-mode
> implementation.
>
> Signed-off-by: Johan Bengtsson <teofrastius@gmail.com>

I've checked against the ARM ARM that the patch is
doing the right thing and also confirmed that with this patch the
qemu implementation now gives the same register results as
running the PKHxx instructions (encoding T1) on A8 hardware.
(Without the patch qemu complains about "Internal resource leak
before 40922058" because the ARM decoder leaks resources on
a great many of the illegal-encoding code paths.)

Acked-by: Peter Maydell <peter.maydell@linaro.org>

> ---
>  target-arm/translate.c |   63 ++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 6fcdd7e..f39efc5 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7601,27 +7601,54 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>             }
>         }
>         break;
> -    case 5: /* Data processing register constant shift.  */
> -        if (rn == 15) {
> -            tmp = new_tmp();
> -            tcg_gen_movi_i32(tmp, 0);
> -        } else {
> -            tmp = load_reg(s, rn);
> -        }
> -        tmp2 = load_reg(s, rm);
> +    case 5:
> +
>         op = (insn >> 21) & 0xf;
> -        shiftop = (insn >> 4) & 3;
> -        shift = ((insn >> 6) & 3) | ((insn >> 10) & 0x1c);
> -        conds = (insn & (1 << 20)) != 0;
> -        logic_cc = (conds && thumb2_logic_op(op));
> -        gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
> -        if (gen_thumb2_data_op(s, op, conds, 0, tmp, tmp2))
> -            goto illegal_op;
> -        dead_tmp(tmp2);
> -        if (rd != 15) {
> +        if (op == 6) {
> +            /* Halfword pack.  */
> +            tmp = load_reg(s, rn);
> +            tmp2 = load_reg(s, rm);
> +            shift = ((insn >> 10) & 0x1c) | ((insn >> 6) & 0x3);
> +            if (insn & (1 << 5)) {
> +                /* pkhtb */
> +                if (shift == 0)
> +                    shift = 31;
> +                tcg_gen_sari_i32(tmp2, tmp2, shift);
> +                tcg_gen_andi_i32(tmp, tmp, 0xffff0000);
> +                tcg_gen_ext16u_i32(tmp2, tmp2);
> +            } else {
> +                /* pkhbt */
> +                if (shift)
> +                    tcg_gen_shli_i32(tmp2, tmp2, shift);
> +                tcg_gen_ext16u_i32(tmp, tmp);
> +                tcg_gen_andi_i32(tmp2, tmp2, 0xffff0000);
> +            }
> +            tcg_gen_or_i32(tmp, tmp, tmp2);
> +            dead_tmp(tmp2);
>             store_reg(s, rd, tmp);
>         } else {
> -            dead_tmp(tmp);
> +            /* Data processing register constant shift.  */
> +            if (rn == 15) {
> +                tmp = new_tmp();
> +                tcg_gen_movi_i32(tmp, 0);
> +            } else {
> +                tmp = load_reg(s, rn);
> +            }
> +            tmp2 = load_reg(s, rm);
> +
> +            shiftop = (insn >> 4) & 3;
> +            shift = ((insn >> 6) & 3) | ((insn >> 10) & 0x1c);
> +            conds = (insn & (1 << 20)) != 0;
> +            logic_cc = (conds && thumb2_logic_op(op));
> +            gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
> +            if (gen_thumb2_data_op(s, op, conds, 0, tmp, tmp2))
> +                goto illegal_op;
> +            dead_tmp(tmp2);
> +            if (rd != 15) {
> +                store_reg(s, rd, tmp);
> +            } else {
> +                dead_tmp(tmp);
> +            }
>         }
>         break;
>     case 13: /* Misc data processing.  */
> --
> 1.7.0.4
>
>
>