[Qemu-devel] [PATCH v2] target-arm: fix UMAAL instruction

[Qemu-devel] [PATCH v2] target-arm: fix UMAAL instruction

[Aurelien Jarno]

UMAAL should use unsigned multiply instead of signed.

This patch fixes this issue by handling UMAAL separately from
UMULL/UMLAL/SMULL/SMLAL as these instructions are different
enough. It also explicitly list instructions in case and catch
nonexistent instruction as illegal. Also fixes a few style issues.

This fixes the issues reported in
https://bugs.launchpad.net/qemu/+bug/696015

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-arm/translate.c |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 8d494ec..2598268 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6637,26 +6637,38 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                             gen_logic_CC(tmp);
                         store_reg(s, rd, tmp);
                         break;
-                    default:
-                        /* 64 bit mul */
+                    case 4:
+                        /* 64 bit mul double accumulate (UMAAL) */
+                        ARCH(6);
                         tmp = load_reg(s, rs);
                         tmp2 = load_reg(s, rm);
-                        if (insn & (1 << 22))
+                        tmp64 = gen_mulu_i64_i32(tmp, tmp2);
+                        gen_addq_lo(s, tmp64, rn);
+                        gen_addq_lo(s, tmp64, rd);
+                        gen_storeq_reg(s, rn, rd, tmp64);
+                        tcg_temp_free_i64(tmp64);
+                        break;
+                    case 8: case 9: case 10: case 11:
+                    case 12: case 13: case 14: case 15:
+                        /* 64 bit mul: UMULL, UMLAL, SMULL, SMLAL. */
+                        tmp = load_reg(s, rs);
+                        tmp2 = load_reg(s, rm);
+                        if (insn & (1 << 22)) {
                             tmp64 = gen_muls_i64_i32(tmp, tmp2);
-                        else
+                        } else {
                             tmp64 = gen_mulu_i64_i32(tmp, tmp2);
-                        if (insn & (1 << 21)) /* mult accumulate */
+                        }
+                        if (insn & (1 << 21)) { /* mult accumulate */
                             gen_addq(s, tmp64, rn, rd);
-                        if (!(insn & (1 << 23))) { /* double accumulate */
-                            ARCH(6);
-                            gen_addq_lo(s, tmp64, rn);
-                            gen_addq_lo(s, tmp64, rd);
                         }
-                        if (insn & (1 << 20))
+                        if (insn & (1 << 20)) {
                             gen_logicq_cc(tmp64);
+                        }
                         gen_storeq_reg(s, rn, rd, tmp64);
                         tcg_temp_free_i64(tmp64);
                         break;
+                    default:
+                        goto illegal_op;
                     }
                 } else {
                     rn = (insn >> 16) & 0xf;
-- 
1.7.2.3

[Qemu-devel] Re: [PATCH v2] target-arm: fix UMAAL instruction

[Peter Maydell]

On 31 December 2010 20:54, Aurelien Jarno <aurelien@aurel32.net> wrote:
> UMAAL should use unsigned multiply instead of signed.
>
> This patch fixes this issue by handling UMAAL separately from
> UMULL/UMLAL/SMULL/SMLAL as these instructions are different
> enough. It also explicitly list instructions in case and catch
> nonexistent instruction as illegal. Also fixes a few style issues.
>
> This fixes the issues reported in
> https://bugs.launchpad.net/qemu/+bug/696015
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

Thanks for the tweaks, looks good to me.

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

-- PMM

[Qemu-devel] Re: [PATCH v2] target-arm: fix UMAAL instruction

[Aurelien Jarno]

On Fri, Dec 31, 2010 at 09:09:04PM +0000, Peter Maydell wrote:
> On 31 December 2010 20:54, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > UMAAL should use unsigned multiply instead of signed.
> >
> > This patch fixes this issue by handling UMAAL separately from
> > UMULL/UMLAL/SMULL/SMLAL as these instructions are different
> > enough. It also explicitly list instructions in case and catch
> > nonexistent instruction as illegal. Also fixes a few style issues.
> >
> > This fixes the issues reported in
> > https://bugs.launchpad.net/qemu/+bug/696015
> >
> > Cc: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Thanks for the tweaks, looks good to me.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 

Thanks for the review, committed.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net