| Message ID | 1293822669-3983-1-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On 31 December 2010 19:11, Aurelien Jarno <aurelien@aurel32.net> wrote: > UMAAL should use unsigned multiply instead of signed. Looks like we've both been working on the same thing this evening :-) > This patch fixes this issue by handling UMAAL separately from > UMULL/UMLAL/SMULL/SUMLAL are these instructions are different "SMLAL as". > It also explicitely list instructions in case and catch > inexistant instruction as illegal. "explicitly" and "nonexistent", if you're tweaking the commit msg anyway. (If you want to use the ARM ARM terminology these instruction patterns are "UNDEFINED" rather than "illegal".) > + case 4: > + /* 64 bit mul double accumulate */ Can we list the instruction mnemonic in the comment too? (Saves future readers having to wade through the decode logic...) This case is UMAAL. > + case 8: case 9: case 10: case 11: > + case 12: case 13: case 14: case 15: /* UMULL, UMLAL, SMULL, SMLAL. */ I've run the patch through my random-sequence test and it passes. -- PMM
diff --git a/target-arm/translate.c b/target-arm/translate.c index 8d494ec..eb207a8 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: + case 4: + /* 64 bit mul double accumulate */ + ARCH(6); + tmp = load_reg(s, rs); + tmp2 = load_reg(s, rm); + 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 */ tmp = load_reg(s, rs); tmp2 = load_reg(s, rm); - if (insn & (1 << 22)) + 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;
UMAAL should use unsigned multiply instead of signed. This patch fixes this issue by handling UMAAL separately from UMULL/UMLAL/SMULL/SUMLAL are these instructions are different enough. It also explicitely list instructions in case and catch inexistant instruction as illegal. Also fixes a few style issues. This fixes the issues reported in https://bugs.launchpad.net/qemu/+bug/696015 Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- target-arm/translate.c | 30 +++++++++++++++++++++--------- 1 files changed, 21 insertions(+), 9 deletions(-)