[Qemu-devel] [PATCH 0/3] target-m68k: add movem, BCD and CAS instructions

[Qemu-devel] [PATCH 0/3] target-m68k: add movem, BCD and CAS instructions

[Laurent Vivier]

This series is another subset of the series I sent in May:
https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg00501.html

This subset contains reworked patches for:
- abcd/nbcd/sbcd: remove inline, delay write back to memory and
  use only 3 digits (and extract it from the bifield patch as
  it was squashed into it)
- movem: delay the update of the registers to the end of the load
  sequence to be able to restart the operation in case of page
  fault, and manage the 68020+ <-> 68000/68010 special case
- cas/cas2: rewrite according Richard's comments (from May),
  and use cmpxchg() in cas. The cas2 sequence is not atomic,
  all advises are welcome!

I've checked it doesn't break coldfire support:
http://wiki.qemu.org/download/coldfire-test-0.1.tar.bz2
but it can't boot a 680x0 processor kernel.

Laurent Vivier (3):
  target-m68k: add abcd/sbcd/nbcd
  target-m68k: implement 680x0 movem
  target-m68k: add cas/cas2 ops

 target-m68k/translate.c | 502 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 485 insertions(+), 17 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/3] target-m68k: add abcd/sbcd/nbcd

[Laurent Vivier]

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target-m68k/translate.c | 237 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 237 insertions(+)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 322b6d0..f9e1d0d 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1342,6 +1342,238 @@ DISAS_INSN(divl)
     set_cc_op(s, CC_OP_FLAGS);
 }
 
+static void bcd_add(TCGv src, TCGv dest)
+{
+    TCGv t0, t1;
+
+    /* t1 = (src + 0x066) + dest
+     *    = result with some possible exceding 0x6
+     */
+
+    t0 = tcg_const_i32(0x066);
+    tcg_gen_add_i32(t0, t0, src);
+
+    t1 = tcg_temp_new();
+    tcg_gen_add_i32(t1, t0, dest);
+
+    /* we will remove exceding 0x6 where there is no carry */
+
+    /* t0 = (src + 0x0066) ^ dest
+     *    = t1 without carries
+     */
+
+    tcg_gen_xor_i32(t0, t0, dest);
+
+    /* extract the carries
+     * t0 = t0 ^ t1
+     *    = only the carries
+     */
+
+    tcg_gen_xor_i32(t0, t0, t1);
+
+    /* generate 0x1 where there is no carry */
+
+    tcg_gen_not_i32(t0, t0);
+    tcg_gen_andi_i32(t0, t0, 0x110);
+
+    /* for each 0x10, generate a 0x6 */
+
+    tcg_gen_shri_i32(dest, t0, 2);
+    tcg_gen_shri_i32(t0, t0, 3);
+    tcg_gen_or_i32(dest, dest, t0);
+    tcg_temp_free(t0);
+
+    /* remove the exceding 0x6
+     * for digits that have not generated a carry
+     */
+
+    tcg_gen_sub_i32(dest, t1, dest);
+    tcg_temp_free(t1);
+}
+
+static void bcd_neg(TCGv dest, TCGv src)
+{
+    TCGv t0, t1;
+
+    /* compute the 10's complement
+     *
+     *    bcd_add(0xf99 - (src + X), 0x0001)
+     *
+     *        t1 = 0xF99 - src - X)
+     *        t2 = t1 + 0x066
+     *        t3 = t2 + 0x001
+     *        t4 = t2 ^ 0x001
+     *        t5 = t3 ^ t4
+     *        t6 = ~t5 & 0x110
+     *        t7 = (t6 >> 2) | (t6 >> 3)
+     *        return t3 - t7
+     *
+     * reduced in:
+     *        t2 = 0xFFF + (-src)
+     *        t3 = (-src)
+     *        t4 = t2  ^ (X ^ 1)
+     *        t5 = (t3 - X) ^ t4
+     *        t6 = ~t5 & 0x110
+     *        t7 = (t6 >> 2) | (t6 >> 3)
+     *        return (t3 - X) - t7
+     *
+     */
+
+    tcg_gen_neg_i32(src, src);
+
+    t0 = tcg_temp_new();
+    tcg_gen_addi_i32(t0, src, 0xfff);
+    tcg_gen_xori_i32(t0, t0, 1);
+    tcg_gen_xor_i32(t0, t0, QREG_CC_X);
+    tcg_gen_sub_i32(src, src, QREG_CC_X);
+    tcg_gen_xor_i32(t0, t0, src);
+    tcg_gen_not_i32(t0, t0);
+    tcg_gen_andi_i32(t0, t0, 0x110);
+
+    t1 = tcg_temp_new();
+    tcg_gen_shri_i32(t1, t0, 2);
+    tcg_gen_shri_i32(t0, t0, 3);
+    tcg_gen_or_i32(t0, t0, t1);
+    tcg_temp_free(t1);
+
+    tcg_gen_sub_i32(dest, src, t0);
+    tcg_temp_free(t0);
+}
+
+static void bcd_flags(TCGv val)
+{
+    tcg_gen_andi_i32(QREG_CC_C, val, 0x0ff);
+    tcg_gen_or_i32(QREG_CC_Z, QREG_CC_Z, QREG_CC_C);
+
+    tcg_gen_movi_i32(QREG_CC_X, 0);
+    tcg_gen_andi_i32(val, val, 0xf00);
+    tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_C, val, QREG_CC_X);
+
+    tcg_gen_mov_i32(QREG_CC_X, QREG_CC_C);
+}
+
+DISAS_INSN(abcd_reg)
+{
+    TCGv src;
+    TCGv dest;
+
+    gen_flush_flags(s); /* !Z is sticky */
+
+    src = gen_extend(DREG(insn, 0), OS_BYTE, 0);
+    dest = gen_extend(DREG(insn, 9), OS_BYTE, 0);
+    bcd_add(src, dest);
+    gen_partset_reg(OS_BYTE, DREG(insn, 9), dest);
+
+    bcd_flags(dest);
+}
+
+DISAS_INSN(abcd_mem)
+{
+    TCGv src;
+    TCGv addr_src;
+    TCGv dest;
+    TCGv addr_dest;
+
+    gen_flush_flags(s); /* !Z is sticky */
+
+    addr_src = tcg_temp_new();
+    tcg_gen_mov_i32(addr_src, AREG(insn, 0));
+
+    tcg_gen_subi_i32(addr_src, addr_src, opsize_bytes(OS_BYTE));
+    src = gen_load(s, OS_BYTE, addr_src, 0);
+
+    addr_dest = tcg_temp_new();
+    if (REG(insn, 0) == REG(insn, 9)) {
+        tcg_gen_mov_i32(addr_dest, addr_src);
+    } else {
+        tcg_gen_mov_i32(addr_dest, AREG(insn, 9));
+    }
+    tcg_gen_subi_i32(addr_dest, addr_dest, opsize_bytes(OS_BYTE));
+    dest = gen_load(s, OS_BYTE, addr_dest, 0);
+
+    bcd_add(src, dest);
+
+    gen_store(s, OS_BYTE, addr_dest, dest);
+
+    tcg_gen_mov_i32(AREG(insn, 0), addr_src);
+    tcg_temp_free(addr_src);
+    tcg_gen_mov_i32(AREG(insn, 9), addr_dest);
+    tcg_temp_free(addr_dest);
+
+    bcd_flags(dest);
+}
+
+DISAS_INSN(sbcd_reg)
+{
+    TCGv src;
+    TCGv dest;
+    TCGv tmp;
+
+    gen_flush_flags(s); /* !Z is sticky */
+
+    src = gen_extend(DREG(insn, 0), OS_BYTE, 0);
+    dest = gen_extend(DREG(insn, 9), OS_BYTE, 0);
+
+    tmp = tcg_temp_new();
+    bcd_neg(tmp, src);
+    bcd_add(tmp, dest);
+    tcg_temp_free(tmp);
+
+    gen_partset_reg(OS_BYTE, DREG(insn, 9), dest);
+
+    bcd_flags(dest);
+
+    set_cc_op(s, CC_OP_FLAGS);
+}
+
+DISAS_INSN(sbcd_mem)
+{
+    TCGv src;
+    TCGv addr_src;
+    TCGv dest;
+    TCGv addr_dest;
+    TCGv tmp;
+
+    gen_flush_flags(s); /* !Z is sticky */
+
+    addr_src = AREG(insn, 0);
+    tcg_gen_subi_i32(addr_src, addr_src, opsize_bytes(OS_BYTE));
+    src = gen_load(s, OS_BYTE, addr_src, 0);
+
+    addr_dest = AREG(insn, 9);
+    tcg_gen_subi_i32(addr_dest, addr_dest, opsize_bytes(OS_BYTE));
+    dest = gen_load(s, OS_BYTE, addr_dest, 0);
+
+    tmp = tcg_temp_new();
+    bcd_neg(tmp, src);
+    bcd_add(tmp, dest);
+    tcg_temp_free(tmp);
+
+    gen_store(s, OS_BYTE, addr_dest, dest);
+
+    bcd_flags(dest);
+
+    set_cc_op(s, CC_OP_FLAGS);
+}
+
+DISAS_INSN(nbcd)
+{
+    TCGv dest;
+    TCGv addr;
+
+    gen_flush_flags(s); /* !Z is sticky */
+
+    SRC_EA(env, dest, OS_BYTE, 0, &addr);
+
+    bcd_neg(dest, dest);
+
+    DEST_EA(env, insn, OS_BYTE, dest, &addr);
+
+    bcd_flags(dest);
+
+    set_cc_op(s, CC_OP_FLAGS);
+}
+
 DISAS_INSN(addsub)
 {
     TCGv reg;
@@ -3655,6 +3887,7 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(not,       4600, ff00, M68000);
     INSN(undef,     46c0, ffc0, M68000);
     INSN(move_to_sr, 46c0, ffc0, CF_ISA_A);
+    INSN(nbcd,      4800, ffc0, M68000);
     INSN(linkl,     4808, fff8, M68000);
     BASE(pea,       4840, ffc0);
     BASE(swap,      4840, fff8);
@@ -3706,6 +3939,8 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(mvzs,      7100, f100, CF_ISA_B);
     BASE(or,        8000, f000);
     BASE(divw,      80c0, f0c0);
+    INSN(sbcd_reg,  8100, f1f8, M68000);
+    INSN(sbcd_mem,  8108, f1f8, M68000);
     BASE(addsub,    9000, f000);
     INSN(undef,     90c0, f0c0, CF_ISA_A);
     INSN(subx_reg,  9180, f1f8, CF_ISA_A);
@@ -3743,6 +3978,8 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(exg_aa,    c148, f1f8, M68000);
     INSN(exg_da,    c188, f1f8, M68000);
     BASE(mulw,      c0c0, f0c0);
+    INSN(abcd_reg,  c100, f1f8, M68000);
+    INSN(abcd_mem,  c108, f1f8, M68000);
     BASE(addsub,    d000, f000);
     INSN(undef,     d0c0, f0c0, CF_ISA_A);
     INSN(addx_reg,      d180, f1f8, CF_ISA_A);
-- 
2.7.4

[Qemu-devel] [PATCH 2/3] target-m68k: implement 680x0 movem

[Laurent Vivier]

680x0 movem can load/store words and long words
and can use more addressing modes.
Coldfire can only use long words with (Ax) and (d16,Ax)
addressing modes.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target-m68k/translate.c | 97 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 80 insertions(+), 17 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index f9e1d0d..159a17d 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1691,14 +1691,25 @@ static void gen_push(DisasContext *s, TCGv val)
     tcg_gen_mov_i32(QREG_SP, tmp);
 }
 
+static TCGv mreg(int reg)
+{
+    if (reg < 8) {
+        /* Dx */
+        return cpu_dregs[reg];
+    }
+    /* Ax */
+    return cpu_aregs[reg & 7];
+}
+
 DISAS_INSN(movem)
 {
     TCGv addr;
     int i;
     uint16_t mask;
-    TCGv reg;
     TCGv tmp;
-    int is_load;
+    int is_load = (insn & 0x0400) != 0;
+    int opsize = (insn & 0x40) != 0 ? OS_LONG : OS_WORD;
+    TCGv incr;
 
     mask = read_im16(env, s);
     tmp = gen_lea(env, s, insn, OS_LONG);
@@ -1706,25 +1717,75 @@ DISAS_INSN(movem)
         gen_addr_fault(s);
         return;
     }
+
     addr = tcg_temp_new();
     tcg_gen_mov_i32(addr, tmp);
-    is_load = ((insn & 0x0400) != 0);
-    for (i = 0; i < 16; i++, mask >>= 1) {
-        if (mask & 1) {
-            if (i < 8)
-                reg = DREG(i, 0);
-            else
-                reg = AREG(i, 0);
-            if (is_load) {
-                tmp = gen_load(s, OS_LONG, addr, 0);
-                tcg_gen_mov_i32(reg, tmp);
-            } else {
-                gen_store(s, OS_LONG, addr, reg);
+    incr = tcg_const_i32(opsize_bytes(opsize));
+
+    if (is_load) {
+        /* memory to register */
+        uint16_t mask2 = mask;
+        TCGv r[16];
+        for (i = 0; i < 16; i++, mask >>= 1) {
+            if (mask & 1) {
+                r[i] = gen_load(s, opsize, addr, 1);
+                tcg_gen_add_i32(addr, addr, incr);
+            }
+        }
+        for (i = 0; i < 16; i++, mask2 >>= 1) {
+            if (mask2 & 1) {
+                tcg_gen_mov_i32(mreg(i), r[i]);
+                tcg_temp_free(r[i]);
+            }
+        }
+        if ((insn & 070) == 030) {
+            /* movem (An)+,X */
+            tcg_gen_mov_i32(AREG(insn, 0), addr);
+        }
+
+    } else {
+        /* register to memory */
+
+        if ((insn & 070) == 040) {
+            /* movem X,-(An) */
+
+            for (i = 15; i >= 0; i--, mask >>= 1) {
+                if (mask & 1) {
+                    if ((insn & 7) + 8 == i &&
+                        m68k_feature(s->env, M68K_FEATURE_EXT_FULL)) {
+                        /* M68020+: if the addressing register is the
+                         * register moved to memory, the value written
+                         * is the initial value decremented by the size of
+                         * the operation
+                         * M68000/M68010: the value is the initial value
+                         */
+                        TCGv tmp = tcg_temp_new();
+                        tcg_gen_sub_i32(tmp, mreg(i), incr);
+                        gen_store(s, opsize, addr, tmp);
+                        tcg_temp_free(tmp);
+                    } else {
+                        gen_store(s, opsize, addr, mreg(i));
+                    }
+                    if (mask != 1) {
+                        tcg_gen_sub_i32(addr, addr, incr);
+                    }
+                }
+            }
+            tcg_gen_mov_i32(AREG(insn, 0), addr);
+        } else {
+            /* movem X,(An)+ is not allowed */
+
+            for (i = 0; i < 16; i++, mask >>= 1) {
+                if (mask & 1) {
+                    gen_store(s, opsize, addr, mreg(i));
+                    tcg_gen_add_i32(addr, addr, incr);
+                }
             }
-            if (mask != 1)
-                tcg_gen_addi_i32(addr, addr, 4);
         }
     }
+
+    tcg_temp_free(incr);
+    tcg_temp_free(addr);
 }
 
 DISAS_INSN(bitop_im)
@@ -3892,7 +3953,9 @@ void register_m68k_insns (CPUM68KState *env)
     BASE(pea,       4840, ffc0);
     BASE(swap,      4840, fff8);
     INSN(bkpt,      4848, fff8, BKPT);
-    BASE(movem,     48c0, fbc0);
+    INSN(movem,     48d0, fbf8, CF_ISA_A);
+    INSN(movem,     48e8, fbf8, CF_ISA_A);
+    INSN(movem,     4880, fb80, M68000);
     BASE(ext,       4880, fff8);
     BASE(ext,       48c0, fff8);
     BASE(ext,       49c0, fff8);
-- 
2.7.4

[Qemu-devel] [PATCH 3/3] target-m68k: add cas/cas2 ops

[Laurent Vivier]

Implement CAS using cmpxchg.
Implement CAS2, but the sequence is not atomic.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 target-m68k/translate.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 159a17d..4e66284 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1905,6 +1905,172 @@ DISAS_INSN(arith_im)
     tcg_temp_free(dest);
 }
 
+DISAS_INSN(cas)
+{
+    int opsize;
+    TCGv addr;
+    uint16_t ext;
+    TCGv load;
+    TCGv cmp;
+    TCGMemOp opc;
+
+    switch ((insn >> 9) & 3) {
+    case 1:
+        opsize = OS_BYTE;
+        opc = MO_UB;
+        break;
+    case 2:
+        opsize = OS_WORD;
+        opc = MO_TEUW;
+        break;
+    case 3:
+        opsize = OS_LONG;
+        opc = MO_TEUL;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    opc |= MO_ALIGN;
+
+    ext = read_im16(env, s);
+
+    /* cas Dc,Du,<EA> */
+
+    addr = gen_lea(env, s, insn, opsize);
+    if (IS_NULL_QREG(addr)) {
+        gen_addr_fault(s);
+        return;
+    }
+
+    cmp = gen_extend(DREG(ext, 0), opsize, 0);
+
+    /* if  <EA> == Dc then
+     *     <EA> = Du
+     *     Dc = <EA> (because <EA> == Dc)
+     * else
+     *     Dc = <EA>
+     */
+
+    load = tcg_temp_new();
+    tcg_gen_atomic_cmpxchg_i32(load, addr, cmp, DREG(ext, 6),
+                               IS_USER(s), opc);
+    gen_partset_reg(opsize, DREG(ext, 0), load);
+
+    gen_update_cc_cmp(s, load, cmp, opsize);
+    tcg_temp_free(load);
+}
+
+DISAS_INSN(cas2)
+{
+    int opsize;
+    uint16_t ext1, ext2;
+    TCGv addr1, addr2;
+    TCGv load1, load2;
+    TCGv cmp1, cmp2;
+    TCGv dest, src;
+    TCGv store;
+
+    switch ((insn >> 9) & 3) {
+    case 1:
+        opsize = OS_BYTE;
+        break;
+    case 2:
+        opsize = OS_WORD;
+        break;
+    case 3:
+        opsize = OS_LONG;
+        break;
+    default:
+        abort();
+    }
+
+    /* cas2 Dc1:Dc2,Du1:Du2,(Rn1):(Rn2) */
+
+    ext1 = read_im16(env, s);
+
+    if (ext1 & 0x8000) {
+        /* Address Register */
+        addr1 = AREG(ext1, 12);
+    } else {
+        /* Data Register */
+        addr1 = DREG(ext1, 12);
+    }
+
+    ext2 = read_im16(env, s);
+    if (ext2 & 0x8000) {
+        /* Address Register */
+        addr2 = AREG(ext2, 12);
+    } else {
+        /* Data Register */
+        addr2 = DREG(ext2, 12);
+    }
+
+    /* FIXME: this sequence is not atomic */
+
+    /* if (R1) == Dc1 && (R2) == Dc2 then
+     *     (R1) = Du1
+     *     (R2) = Du2
+     *     Dc1 = (R1) (because Dc1 == (R1))
+     *     Dc2 = (R2) (because Dc2 == (R2))
+     * else
+     *     Dc1 = (R1)
+     *     Dc2 = (R2)
+     */
+
+    load1 = gen_load(s, opsize, addr1, 0);
+    load2 = gen_load(s, opsize, addr2, 0);
+
+    cmp1 = gen_extend(DREG(ext1, 0), opsize, 0);
+    cmp2 = gen_extend(DREG(ext2, 0), opsize, 0);
+
+    /* (R1) - Dc1 -> cc
+     *     if Z
+     *         (R2) - Dc2 -> cc
+     *
+     * with CC_OP_CMP, set dest and src accordingly
+     *    dest - src -> cc
+     */
+
+    dest = tcg_temp_new();
+    tcg_gen_movcond_i32(TCG_COND_EQ, dest,
+                        load1, cmp1,
+                        load2, load1);
+    src = tcg_temp_new();
+    tcg_gen_movcond_i32(TCG_COND_EQ, src,
+                        load1, cmp1,
+                        cmp2, cmp1);
+
+    /* if src == dest then
+     *     (R1) = Du1
+     *     (R2) = Du2
+     */
+
+    store = tcg_temp_new();
+    tcg_gen_movcond_i32(TCG_COND_EQ, store,
+                        dest, src,
+                        DREG(ext1, 6), load1);
+    gen_store(s, opsize, addr1, store);
+
+    tcg_gen_movcond_i32(TCG_COND_EQ, store,
+                        dest, src,
+                        DREG(ext2, 6), load2);
+    gen_store(s, opsize, addr2, store);
+    tcg_temp_free(store);
+
+    /* Dc1 = (R1)
+     * Dc2 = (R2)
+     */
+
+    gen_partset_reg(opsize, DREG(ext1, 0), load1);
+    gen_partset_reg(opsize, DREG(ext2, 0), load2);
+
+    tcg_gen_mov_i32(QREG_CC_N, dest);
+    tcg_temp_free(dest);
+    tcg_gen_mov_i32(QREG_CC_V, src);
+    tcg_temp_free(src);
+    set_cc_op(s, CC_OP_CMPB + opsize);
+}
+
 DISAS_INSN(byterev)
 {
     TCGv reg;
@@ -3920,6 +4086,8 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(arith_im,  0680, fff8, CF_ISA_A);
     INSN(arith_im,  0c00, ff38, CF_ISA_A);
     INSN(arith_im,  0c00, ff00, M68000);
+    INSN(cas,       08c0, f9c0, CAS);
+    INSN(cas2,      08fc, f9ff, CAS);
     BASE(bitop_im,  0800, ffc0);
     BASE(bitop_im,  0840, ffc0);
     BASE(bitop_im,  0880, ffc0);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/3] target-m68k: add abcd/sbcd/nbcd

[Richard Henderson]

On 10/31/2016 03:20 AM, Laurent Vivier wrote:
> +    tcg_gen_subi_i32(addr_src, addr_src, opsize_bytes(OS_BYTE));
> +    src = gen_load(s, OS_BYTE, addr_src, 0);
> +
> +    addr_dest = tcg_temp_new();
> +    if (REG(insn, 0) == REG(insn, 9)) {
> +        tcg_gen_mov_i32(addr_dest, addr_src);
> +    } else {
> +        tcg_gen_mov_i32(addr_dest, AREG(insn, 9));
> +    }
> +    tcg_gen_subi_i32(addr_dest, addr_dest, opsize_bytes(OS_BYTE));
> +    dest = gen_load(s, OS_BYTE, addr_dest, 0);

Write backs before loads.


> +    bcd_neg(tmp, src);
> +    bcd_add(tmp, dest);

Didn't we simplify the subtraction algorithm to make neg?  Why not un-simplify 
it and let nbcd provide a zero (and then let the tcg optimizer do what it can).


r~

Re: [Qemu-devel] [PATCH 2/3] target-m68k: implement 680x0 movem

[Richard Henderson]

On 10/31/2016 03:20 AM, Laurent Vivier wrote:
> +        uint16_t mask2 = mask;
> +        TCGv r[16];
> +        for (i = 0; i < 16; i++, mask >>= 1) {
> +            if (mask & 1) {
> +                r[i] = gen_load(s, opsize, addr, 1);
> +                tcg_gen_add_i32(addr, addr, incr);
> +            }
> +        }
> +        for (i = 0; i < 16; i++, mask2 >>= 1) {
> +            if (mask2 & 1) {
> +                tcg_gen_mov_i32(mreg(i), r[i]);
> +                tcg_temp_free(r[i]);
> +            }
> +        }

While it's not wrong, why not examine mask non-destructively.

   if ((mask >> i) & 1)


r~

Re: [Qemu-devel] [PATCH 3/3] target-m68k: add cas/cas2 ops

[Richard Henderson]

On 10/31/2016 03:20 AM, Laurent Vivier wrote:
> +    /* FIXME: this sequence is not atomic */
> +
> +    /* if (R1) == Dc1 && (R2) == Dc2 then
> +     *     (R1) = Du1
> +     *     (R2) = Du2
> +     *     Dc1 = (R1) (because Dc1 == (R1))
> +     *     Dc2 = (R2) (because Dc2 == (R2))
> +     * else
> +     *     Dc1 = (R1)
> +     *     Dc2 = (R2)
> +     */

We'll need to handle this in a helper, I think.  And there is an interesting 
subcase that's worth handling: When (R1) and (R2) are consecutive, i.e. we're 
performing a "normal" 64-bit cmpxchg, rather than two disjoint 32-bit cmpxchgs.

While we could pack the Dc and Du data into uint64_t, getting the flags exactly 
right probably requires more help so let's do everything in the helper, 
including updating the registers.  Like so:

void HELPER(cas2l)(CPUM68KState *env, uint32_t regs, uint32_t a1, uint32_t a2)
{
     uint32_t Dc1 = extract32(regs, 9, 3);
     uint32_t Dc2 = extract32(regs, 6, 3);
     uint32_t Du1 = extract32(regs, 3, 3);
     uint32_t Du2 = extract32(regs, 0, 3);
     uint32_t c1 = env->dregs[Dc1];
     uint32_t c2 = env->dregs[Dc2];
     uint32_t u1 = env->dregs[Du1];
     uint32_t u2 = env->dregs[Du2];
     uint32_t l1, l2;
     uint64_t c, u, l;
     uintptr_t ra = GETPC();
     int mmu_idx = cpu_mmu_index(env, 0);
     TCGMemOpIdx oi;

     if (parallel_cpus) {
         /* We're executing in a parallel context -- must be atomic.  */
         if ((a1 & 7) == 0 && a2 == a1 + 4) {
             c = deposit64(c2, 32, 32, c1);
             u = deposit64(u2, 32, 32, u1);
             oi = make_memop_idx(MO_BEQ, mmu_idx);
             l = helper_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
             l1 = l >> 32;
             l2 = l;
         } else if ((a2 & 7) == 0 && a1 == a2 + 4) {
             c = deposit64(c1, 32, 32, c2);
             u = deposit64(u1, 32, 32, u2);
             oi = make_memop_idx(MO_BEQ, mmu_idx);
             l = helper_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
             l2 = l >> 32;
             l1 = l;
         } else {
             /* Tell the main loop we need to serialize this insn.  */
             cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
         }
     } else {
         /* We're executing in a serial context -- no need to be atomic.  */
         oi = make_memop_idx(MO_BEUL, mmu_idx);
         l1 = helper_be_ldul_mmu(env, a1, oi, ra);
         l2 = helper_be_ldul_mmu(env, a2, oi, ra);
         if (l1 == c1 && l2 == c2) {
             helper_be_stl_mmu(env, a1, u1, oi, ra);
             helper_be_stl_mmu(env, a2, u2, oi, ra);
         }
     }

     if (c1 != l1) {
         env->cc_n = c1;
         env->cc_v = l1;
     } else {
         env->cc_n = c2;
         env->cc_v = l2;
     }
     env->cc_op = CC_OP_CMPL;
     env->dregs[Dc1] = l1;
     env->dregs[Dc2] = l2;
}

where cas2w would be similar (but could probably drop the special case for aX = 
aY + 2, because surely anyone sane would have just used cas.l in the first place).


r~