[Qemu-devel] [PATCH 0/5] target-m68k patches

[Qemu-devel] [PATCH 0/5] target-m68k patches

[Richard Henderson]

I believe the first two patches have already been posted.
No changes there, but included here for completeness.

Next is a proposed shift patch, with the changes I believe
I mentioned wanting during review.

Finally, implementations of the bitfield instructions.  At least
the register portion depends on the generic tcg (s)extract primitives
that I have queued for 2.9.

The full tree is at

  git://github.com/rth7680/qemu.git tgt-m68k


r~


Laurent Vivier (1):
  target-m68k: implement 680x0 movem

Richard Henderson (4):
  target-m68k: Do not cpu_abort on undefined insns
  target-m68k: Inline shifts
  target-m68k: Implement bitfield ops for registers
  target-m68k: Implement bitfield ops for memory

 target-m68k/cpu.h       |   1 +
 target-m68k/helper.c    | 235 ++++++++++++----
 target-m68k/helper.h    |  10 +-
 target-m68k/translate.c | 702 ++++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 840 insertions(+), 108 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/5] target-m68k: implement 680x0 movem

[Richard Henderson]

From: Laurent Vivier <laurent@vivier.eu>

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>
Message-Id: <1478121319-31986-3-git-send-email-laurent@vivier.eu>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-m68k/translate.c | 130 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 107 insertions(+), 23 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 9b4ad52..60c8b4b 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1430,40 +1430,122 @@ 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;
+    TCGv addr, incr, tmp, r[16];
+    int is_load = (insn & 0x0400) != 0;
+    int opsize = (insn & 0x40) != 0 ? OS_LONG : OS_WORD;
+    uint16_t mask = read_im16(env, s);
+    int mode = extract32(insn, 3, 3);
+    int reg0 = REG(insn, 0);
     int i;
-    uint16_t mask;
-    TCGv reg;
-    TCGv tmp;
-    int is_load;
 
-    mask = read_im16(env, s);
-    tmp = gen_lea(env, s, insn, OS_LONG);
-    if (IS_NULL_QREG(tmp)) {
+    tmp = cpu_aregs[reg0];
+
+    switch (mode) {
+    case 0: /* data register direct */
+    case 1: /* addr register direct */
+    do_addr_fault:
         gen_addr_fault(s);
         return;
+
+    case 2: /* indirect */
+        break;
+
+    case 3: /* indirect post-increment */
+        if (!is_load) {
+            /* post-increment is not allowed */
+            goto do_addr_fault;
+        }
+        break;
+
+    case 4: /* indirect pre-decrement */
+        if (is_load) {
+            /* pre-decrement is not allowed */
+            goto do_addr_fault;
+        }
+        /* We want a bare copy of the address reg, without any pre-decrement
+           adjustment, as gen_lea would provide.  */
+        break;
+
+    default:
+        tmp = gen_lea_mode(env, s, mode, reg0, opsize);
+        if (IS_NULL_QREG(tmp)) {
+            goto do_addr_fault;
+        }
+        break;
     }
+
     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 */
+        for (i = 0; i < 16; i++) {
+            if (mask & (1 << i)) {
+                r[i] = gen_load(s, opsize, addr, 1);
+                tcg_gen_add_i32(addr, addr, incr);
+            }
+        }
+        for (i = 0; i < 16; i++) {
+            if (mask & (1 << i)) {
+                tcg_gen_mov_i32(mreg(i), r[i]);
+                tcg_temp_free(r[i]);
+            }
+        }
+        if (mode == 3) {
+            /* post-increment: movem (An)+,X */
+            tcg_gen_mov_i32(AREG(insn, 0), addr);
+        }
+    } else {
+        /* register to memory */
+        if (mode == 4) {
+            /* pre-decrement: movem X,-(An) */
+            for (i = 15; i >= 0; i--) {
+                if ((mask << i) & 0x8000) {
+                    tcg_gen_sub_i32(addr, addr, incr);
+                    if (reg0 + 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, regardless of how many actual
+                         * stores have been performed until this point.
+                         * M68000/M68010: the value is the initial value.
+                         */
+                        TCGv tmp = tcg_temp_new();
+                        tcg_gen_sub_i32(tmp, cpu_aregs[reg0], incr);
+                        gen_store(s, opsize, addr, tmp);
+                        tcg_temp_free(tmp);
+                    } else {
+                        gen_store(s, opsize, addr, mreg(i));
+                    }
+                }
+            }
+            tcg_gen_mov_i32(cpu_aregs[reg0], addr);
+        } else {
+            for (i = 0; i < 16; i++) {
+                if (mask & (1 << i)) {
+                    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)
@@ -3452,7 +3534,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 2/5] target-m68k: Do not cpu_abort on undefined insns

[Richard Henderson]

Report this properly via exception and, importantly, allow
the disassembler the chance to tell us what insn is not handled.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-m68k/translate.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 60c8b4b..2f956d9 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1214,10 +1214,12 @@ DISAS_INSN(undef_fpu)
 
 DISAS_INSN(undef)
 {
-    M68kCPU *cpu = m68k_env_get_cpu(env);
-
+    /* ??? This is both instructions that are as yet unimplemented
+       for the 680x0 series, as well as those that are implemented
+       but actually illegal for CPU32 or pre-68020.  */
+    qemu_log_mask(LOG_UNIMP, "Illegal instruction: %04x @ %08x",
+                  insn, s->pc - 2);
     gen_exception(s, s->pc - 2, EXCP_UNSUPPORTED);
-    cpu_abort(CPU(cpu), "Illegal instruction: %04x @ %08x", insn, s->pc - 2);
 }
 
 DISAS_INSN(mulw)
-- 
2.7.4

[Qemu-devel] [PATCH 3/5] target-m68k: Inline shifts

[Richard Henderson]

Also manage word and byte operands and fix the computation of
overflow in the case of M68000 arithmetic shifts.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-m68k/helper.c    |  52 ------------
 target-m68k/helper.h    |   3 -
 target-m68k/translate.c | 216 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 190 insertions(+), 81 deletions(-)

diff --git a/target-m68k/helper.c b/target-m68k/helper.c
index 7aed9ff..f750d3d 100644
--- a/target-m68k/helper.c
+++ b/target-m68k/helper.c
@@ -284,58 +284,6 @@ void HELPER(set_sr)(CPUM68KState *env, uint32_t val)
     m68k_switch_sp(env);
 }
 
-uint32_t HELPER(shl_cc)(CPUM68KState *env, uint32_t val, uint32_t shift)
-{
-    uint64_t result;
-
-    shift &= 63;
-    result = (uint64_t)val << shift;
-
-    env->cc_c = (result >> 32) & 1;
-    env->cc_n = result;
-    env->cc_z = result;
-    env->cc_v = 0;
-    env->cc_x = shift ? env->cc_c : env->cc_x;
-
-    return result;
-}
-
-uint32_t HELPER(shr_cc)(CPUM68KState *env, uint32_t val, uint32_t shift)
-{
-    uint64_t temp;
-    uint32_t result;
-
-    shift &= 63;
-    temp = (uint64_t)val << 32 >> shift;
-    result = temp >> 32;
-
-    env->cc_c = (temp >> 31) & 1;
-    env->cc_n = result;
-    env->cc_z = result;
-    env->cc_v = 0;
-    env->cc_x = shift ? env->cc_c : env->cc_x;
-
-    return result;
-}
-
-uint32_t HELPER(sar_cc)(CPUM68KState *env, uint32_t val, uint32_t shift)
-{
-    uint64_t temp;
-    uint32_t result;
-
-    shift &= 63;
-    temp = (int64_t)val << 32 >> shift;
-    result = temp >> 32;
-
-    env->cc_c = (temp >> 31) & 1;
-    env->cc_n = result;
-    env->cc_z = result;
-    env->cc_v = result ^ val;
-    env->cc_x = shift ? env->cc_c : env->cc_x;
-
-    return result;
-}
-
 /* FPU helpers.  */
 uint32_t HELPER(f64_to_i32)(CPUM68KState *env, float64 val)
 {
diff --git a/target-m68k/helper.h b/target-m68k/helper.h
index 6180dc5..d863e55 100644
--- a/target-m68k/helper.h
+++ b/target-m68k/helper.h
@@ -7,9 +7,6 @@ DEF_HELPER_4(divul, void, env, int, int, i32)
 DEF_HELPER_4(divsl, void, env, int, int, s32)
 DEF_HELPER_4(divull, void, env, int, int, i32)
 DEF_HELPER_4(divsll, void, env, int, int, s32)
-DEF_HELPER_3(shl_cc, i32, env, i32, i32)
-DEF_HELPER_3(shr_cc, i32, env, i32, i32)
-DEF_HELPER_3(sar_cc, i32, env, i32, i32)
 DEF_HELPER_2(set_sr, void, env, i32)
 DEF_HELPER_3(movec, void, env, i32, i32)
 
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 2f956d9..b538d74 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2519,48 +2519,205 @@ DISAS_INSN(addx_mem)
     gen_store(s, opsize, addr_dest, QREG_CC_N);
 }
 
-/* TODO: This could be implemented without helper functions.  */
-DISAS_INSN(shift_im)
-{
-    TCGv reg;
-    int tmp;
-    TCGv shift;
+static inline void shift_im(DisasContext *s, uint16_t insn, int opsize)
+{
+    int count = (insn >> 9) & 7;
+    int logical = insn & 8;
+    int left = insn & 0x100;
+    int bits = opsize_bytes(opsize) * 8;
+    TCGv reg = gen_extend(DREG(insn, 0), opsize, !logical);
+
+    if (count == 0) {
+        count = 8;
+    }
+
+    tcg_gen_movi_i32(QREG_CC_V, 0);
+    if (left) {
+        tcg_gen_shri_i32(QREG_CC_C, reg, bits - count);
+        tcg_gen_shli_i32(QREG_CC_N, reg, count);
+
+        /* Note that ColdFire always clears V (done above),
+           while M68000 sets if the most significant bit is changed at
+           any time during the shift operation */
+        if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+            /* if shift count >= bits, V is (reg != 0) */
+            if (count >= bits) {
+                tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, reg, QREG_CC_V);
+            } else {
+                TCGv t0 = tcg_temp_new();
+                tcg_gen_sari_i32(QREG_CC_V, reg, bits - 1);
+                tcg_gen_sari_i32(t0, t0, bits - count);
+                tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, QREG_CC_V, t0);
+                tcg_temp_free(t0);
+            }
+            tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
+        }
+    } else {
+        tcg_gen_shri_i32(QREG_CC_C, reg, count - 1);
+        if (logical) {
+            tcg_gen_shri_i32(QREG_CC_N, reg, count);
+        } else {
+            tcg_gen_sari_i32(QREG_CC_N, reg, count);
+        }
+    }
 
+    gen_ext(QREG_CC_N, QREG_CC_N, opsize, 1);
+    tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 1);
+    tcg_gen_mov_i32(QREG_CC_Z, QREG_CC_N);
+    tcg_gen_mov_i32(QREG_CC_X, QREG_CC_C);
+
+    gen_partset_reg(opsize, DREG(insn, 0), QREG_CC_N);
     set_cc_op(s, CC_OP_FLAGS);
+}
 
-    reg = DREG(insn, 0);
-    tmp = (insn >> 9) & 7;
-    if (tmp == 0)
-        tmp = 8;
-    shift = tcg_const_i32(tmp);
-    /* No need to flush flags becuse we know we will set C flag.  */
-    if (insn & 0x100) {
-        gen_helper_shl_cc(reg, cpu_env, reg, shift);
+static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
+{
+    int logical = insn & 8;
+    int left = insn & 0x100;
+    int bits = opsize_bytes(opsize) * 8;
+    TCGv reg = gen_extend(DREG(insn, 0), opsize, !logical);
+    TCGv s32;
+    TCGv_i64 t64, s64;
+
+    t64 = tcg_temp_new_i64();
+    s64 = tcg_temp_new_i64();
+    s32 = tcg_temp_new();
+
+    /* Note that m68k truncates the shift count modulo 64, not 32.
+       In addition, a 64-bit shift makes it easy to find "the last
+       bit shifted out", for the carry flag.  */
+    tcg_gen_andi_i32(s32, DREG(insn, 9), 63);
+    tcg_gen_extu_i32_i64(s64, s32);
+    tcg_gen_extu_i32_i64(t64, reg);
+
+    /* Optimistically set V=0.  Also used as a zero source below.  */
+    tcg_gen_movi_i32(QREG_CC_V, 0);
+    if (left) {
+        tcg_gen_shl_i64(t64, t64, s64);
+
+        tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
+
+        /* Note that C=0 if shift count is 0, and we get that for free.  */
+        tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 1);
+
+        /* X = C, but only if the shift count was non-zero.  */
+        tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_X, s32, QREG_CC_V,
+                            QREG_CC_C, QREG_CC_X);
+
+        /* M68000 sets V if the most significant bit is changed at
+         * any time during the shift operation.  Do this via creating
+         * an extension of the sign bit, comparing, and discarding
+         * the bits below the sign bit.  I.e.
+         *     int64_t s = (intN_t)reg;
+         *     int64_t t = (int64_t)(intN_t)reg << count;
+         *     V = ((s ^ t) & (-1 << (bits - 1))) != 0
+         */
+        if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+            /* Sign extend the input to 64 bits; re-do the shift.  */
+            tcg_gen_ext_i32_i64(t64, reg);
+            tcg_gen_shl_i64(s64, t64, s64);
+            /* Clear all bits that are unchanged.  */
+            tcg_gen_xor_i64(t64, t64, s64);
+            /* Ignore the bits below the sign bit.  */
+            tcg_gen_andi_i64(t64, t64, -1ULL << (bits - 1));
+            /* If any bits remain set, we have overflow.  */
+            tcg_gen_setcondi_i64(TCG_COND_NE, t64, t64, 0);
+            tcg_gen_extrl_i64_i32(QREG_CC_V, t64);
+            tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
+        }
     } else {
-        if (insn & 8) {
-            gen_helper_shr_cc(reg, cpu_env, reg, shift);
+        tcg_gen_shli_i64(t64, t64, 32);
+        if (logical) {
+            tcg_gen_shr_i64(t64, t64, s64);
         } else {
-            gen_helper_sar_cc(reg, cpu_env, reg, shift);
+            tcg_gen_sar_i64(t64, t64, s64);
         }
+        tcg_gen_extr_i64_i32(QREG_CC_C, QREG_CC_N, t64);
+
+        /* Note that C=0 if shift count is 0, and we get that for free.  */
+        tcg_gen_shri_i32(QREG_CC_C, QREG_CC_C, 31);
+
+        /* X = C, but only if the shift count was non-zero.  */
+        tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_X, s32, QREG_CC_V,
+                            QREG_CC_C, QREG_CC_X);
     }
+    gen_ext(QREG_CC_N, QREG_CC_N, opsize, 1);
+    tcg_gen_mov_i32(QREG_CC_Z, QREG_CC_N);
+
+    tcg_temp_free(s32);
+    tcg_temp_free_i64(s64);
+    tcg_temp_free_i64(t64);
+
+    /* Write back the result.  */
+    gen_partset_reg(opsize, DREG(insn, 0), QREG_CC_N);
+    set_cc_op(s, CC_OP_FLAGS);
+}
+
+DISAS_INSN(shift8_im)
+{
+    shift_im(s, insn, OS_BYTE);
+}
+
+DISAS_INSN(shift16_im)
+{
+    shift_im(s, insn, OS_WORD);
+}
+
+DISAS_INSN(shift_im)
+{
+    shift_im(s, insn, OS_LONG);
+}
+
+DISAS_INSN(shift8_reg)
+{
+    shift_reg(s, insn, OS_BYTE);
+}
+
+DISAS_INSN(shift16_reg)
+{
+    shift_reg(s, insn, OS_WORD);
 }
 
 DISAS_INSN(shift_reg)
 {
-    TCGv reg;
-    TCGv shift;
+    shift_reg(s, insn, OS_LONG);
+}
 
-    reg = DREG(insn, 0);
-    shift = DREG(insn, 9);
-    if (insn & 0x100) {
-        gen_helper_shl_cc(reg, cpu_env, reg, shift);
+DISAS_INSN(shift_mem)
+{
+    int logical = insn & 8;
+    int left = insn & 0x100;
+    TCGv src;
+    TCGv addr;
+
+    SRC_EA(env, src, OS_WORD, !logical, &addr);
+    tcg_gen_movi_i32(QREG_CC_V, 0);
+    if (left) {
+        tcg_gen_shri_i32(QREG_CC_C, src, 15);
+        tcg_gen_shli_i32(QREG_CC_N, src, 1);
+
+        /* Note that ColdFire always clears V,
+           while M68000 sets if the most significant bit is changed at
+           any time during the shift operation */
+        if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+            src = gen_extend(src, OS_WORD, 1);
+            tcg_gen_xor_i32(QREG_CC_V, QREG_CC_N, src);
+        }
     } else {
-        if (insn & 8) {
-            gen_helper_shr_cc(reg, cpu_env, reg, shift);
+        tcg_gen_mov_i32(QREG_CC_C, src);
+        if (logical) {
+            tcg_gen_shri_i32(QREG_CC_N, src, 1);
         } else {
-            gen_helper_sar_cc(reg, cpu_env, reg, shift);
+            tcg_gen_sari_i32(QREG_CC_N, src, 1);
         }
     }
+
+    gen_ext(QREG_CC_N, QREG_CC_N, OS_WORD, 1);
+    tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 1);
+    tcg_gen_mov_i32(QREG_CC_Z, QREG_CC_N);
+    tcg_gen_mov_i32(QREG_CC_X, QREG_CC_C);
+
+    DEST_EA(env, insn, OS_WORD, QREG_CC_N, &addr);
     set_cc_op(s, CC_OP_FLAGS);
 }
 
@@ -3631,6 +3788,13 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(adda,      d0c0, f0c0, M68000);
     INSN(shift_im,  e080, f0f0, CF_ISA_A);
     INSN(shift_reg, e0a0, f0f0, CF_ISA_A);
+    INSN(shift8_im, e000, f0f0, M68000);
+    INSN(shift16_im, e040, f0f0, M68000);
+    INSN(shift_im,  e080, f0f0, M68000);
+    INSN(shift8_reg, e020, f0f0, M68000);
+    INSN(shift16_reg, e060, f0f0, M68000);
+    INSN(shift_reg, e0a0, f0f0, M68000);
+    INSN(shift_mem, e0c0, fcc0, M68000);
     INSN(undef_fpu, f000, f000, CF_ISA_A);
     INSN(fpu,       f200, ffc0, CF_FPU);
     INSN(fbcc,      f280, ffc0, CF_FPU);
-- 
2.7.4

[Qemu-devel] [PATCH 4/5] target-m68k: Implement bitfield ops for registers

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-m68k/translate.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 210 insertions(+)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index b538d74..477a511 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2721,6 +2721,210 @@ DISAS_INSN(shift_mem)
     set_cc_op(s, CC_OP_FLAGS);
 }
 
+DISAS_INSN(bfext_reg)
+{
+    int ext = read_im16(env, s);
+    int is_sign = insn & 0x200;
+    TCGv src = DREG(insn, 0);
+    TCGv dst = DREG(ext, 12);
+    int len = ((extract32(ext, 0, 5) - 1) & 31) + 1;
+    int ofs = extract32(ext, 6, 5);  /* big bit-endian */
+    int pos = 31 - ofs;              /* little bit-endian */
+    TCGv tmp = tcg_temp_new();
+    TCGv shift;
+
+    /* In general, we're going to rotate the field so that it's at the
+       top of the word and then right-shift by the compliment of the
+       width to extend the field.  */
+    if (ext & 0x20) {
+        /* Variable width.  */
+        if (ext & 0x800) {
+            /* Variable offset.  */
+            tcg_gen_andi_i32(tmp, DREG(ext, 6), 31);
+            tcg_gen_rotl_i32(tmp, src, tmp);
+        } else {
+            tcg_gen_rotli_i32(tmp, src, ofs);
+        }
+
+        shift = tcg_temp_new();
+        tcg_gen_neg_i32(shift, DREG(ext, 0));
+        tcg_gen_andi_i32(shift, shift, 31);
+        tcg_gen_sar_i32(QREG_CC_N, tmp, shift);
+        if (is_sign) {
+            tcg_gen_mov_i32(dst, QREG_CC_N);
+        } else {
+            tcg_gen_shr_i32(dst, tmp, shift);
+        }
+        tcg_temp_free(shift);
+    } else {
+        /* Immediate width.  */
+        if (ext & 0x800) {
+            /* Variable offset */
+            tcg_gen_andi_i32(tmp, DREG(ext, 6), 31);
+            tcg_gen_rotl_i32(tmp, src, tmp);
+            src = tmp;
+            pos = 32 - len;
+        } else {
+            /* Immediate offset.  If the field doesn't wrap around the
+               end of the word, rely on (s)extract completely.  */
+            if (pos + len > 32) {
+                tcg_gen_rotli_i32(tmp, src, ofs);
+                src = tmp;
+                pos = 32 - len;
+            }
+        }
+
+        tcg_gen_sextract_i32(QREG_CC_N, src, pos, len);
+        if (is_sign) {
+            tcg_gen_mov_i32(dst, QREG_CC_N);
+        } else {
+            tcg_gen_extract_i32(dst, src, pos, len);
+        }
+    }
+
+    tcg_temp_free(tmp);
+    set_cc_op(s, CC_OP_LOGIC);
+}
+
+DISAS_INSN(bfop_reg)
+{
+    int ext = read_im16(env, s);
+    TCGv src = DREG(insn, 0);
+    int len = ((extract32(ext, 0, 5) - 1) & 31) + 1;
+    int ofs = extract32(ext, 6, 5);  /* big bit-endian */
+    TCGv mask;
+
+    if ((ext & 0x820) == 0) {
+        /* Immediate width and offset.  */
+        uint32_t maski = 0x7fffffffu >> (len - 1);
+        if (ofs + len <= 32) {
+            tcg_gen_shli_i32(QREG_CC_N, src, ofs);
+        } else {
+            tcg_gen_rotli_i32(QREG_CC_N, src, ofs);
+        }
+        tcg_gen_andi_i32(QREG_CC_N, QREG_CC_N, ~maski);
+        mask = tcg_const_i32(ror32(maski, ofs));
+    } else {
+        TCGv tmp = tcg_temp_new();
+        if (ext & 0x20) {
+            /* Variable width */
+            tcg_gen_subi_i32(tmp, DREG(ext, 0), 1);
+            tcg_gen_andi_i32(tmp, tmp, 31);
+            mask = tcg_const_i32(0x7fffffffu);
+            tcg_gen_shr_i32(mask, mask, tmp);
+        } else {
+            /* Immediate width */
+            mask = tcg_const_i32(0x7fffffffu >> (len - 1));
+        }
+        if (ext & 0x800) {
+            /* Variable offset */
+            tcg_gen_andi_i32(tmp, DREG(ext, 6), 31);
+            tcg_gen_rotl_i32(QREG_CC_N, src, tmp);
+            tcg_gen_andc_i32(QREG_CC_N, QREG_CC_N, mask);
+            tcg_gen_rotr_i32(mask, mask, tmp);
+        } else {
+            /* Immediate offset (and variable width) */
+            tcg_gen_rotli_i32(QREG_CC_N, src, ofs);
+            tcg_gen_andc_i32(QREG_CC_N, QREG_CC_N, mask);
+            tcg_gen_rotri_i32(mask, mask, ofs);
+        }
+        tcg_temp_free(tmp);
+    }
+    set_cc_op(s, CC_OP_LOGIC);
+
+    switch (insn & 0x0f00) {
+    case 0x0a00: /* bfchg */
+        tcg_gen_eqv_i32(src, src, mask);
+        break;
+    case 0x0c00: /* bfclr */
+        tcg_gen_and_i32(src, src, mask);
+        break;
+    case 0x0e00: /* bfset */
+        tcg_gen_orc_i32(src, src, mask);
+        break;
+    case 0x0800: /* bftst */
+        /* flags already set; no other work to do.  */
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    tcg_temp_free(mask);
+}
+
+DISAS_INSN(bfins_reg)
+{
+    int ext = read_im16(env, s);
+    TCGv dst = DREG(insn, 0);
+    TCGv src = DREG(ext, 12);
+    int len = ((extract32(ext, 0, 5) - 1) & 31) + 1;
+    int ofs = extract32(ext, 6, 5);  /* big bit-endian */
+    int pos = 31 - ofs;              /* little bit-endian */
+    TCGv tmp;
+
+    tmp = tcg_temp_new();
+
+    if (ext & 0x20) {
+        /* Variable width */
+        tcg_gen_neg_i32(tmp, DREG(ext, 0));
+        tcg_gen_andi_i32(tmp, tmp, 31);
+        tcg_gen_shl_i32(QREG_CC_N, src, tmp);
+    } else {
+        /* Immediate width */
+        tcg_gen_shli_i32(QREG_CC_N, src, 32 - len);
+    }
+    set_cc_op(s, CC_OP_LOGIC);
+
+    /* Immediate width and offset */
+    if ((ext & 0x820) == 0) {
+        /* Check for suitability for deposit.  */
+        if (pos + len <= 32) {
+            tcg_gen_deposit_i32(dst, dst, src, pos, len);
+        } else {
+            uint32_t maski = -2U << (len - 1);
+            uint32_t roti = (ofs + len) & 31;
+            tcg_gen_andi_i32(tmp, src, maski);
+            tcg_gen_rotri_i32(tmp, tmp, roti);
+            tcg_gen_andi_i32(dst, dst, ror32(maski, roti));
+            tcg_gen_or_i32(dst, dst, tmp);
+        }
+    } else {
+        TCGv mask = tcg_temp_new();
+        TCGv rot = tcg_temp_new();
+
+        if (ext & 0x20) {
+            /* Variable width */
+            tcg_gen_subi_i32(rot, DREG(ext, 0), 1);
+            tcg_gen_andi_i32(rot, rot, 31);
+            tcg_gen_movi_i32(mask, -2);
+            tcg_gen_shl_i32(mask, mask, rot);
+            tcg_gen_mov_i32(rot, DREG(ext, 0));
+            tcg_gen_andc_i32(tmp, src, mask);
+        } else {
+            /* Immediate width (variable offset) */
+            uint32_t maski = -2U << (len - 1);
+            tcg_gen_andi_i32(tmp, src, ~maski);
+            tcg_gen_movi_i32(mask, maski);
+            tcg_gen_movi_i32(rot, len & 31);
+        }
+        if (ext & 0x800) {
+            /* Variable offset */
+            tcg_gen_add_i32(rot, rot, DREG(ext, 6));
+        } else {
+            /* Immediate offset (variable width) */
+            tcg_gen_addi_i32(rot, rot, ofs);
+        }
+        tcg_gen_andi_i32(rot, rot, 31);
+        tcg_gen_rotr_i32(mask, mask, rot);
+        tcg_gen_rotr_i32(tmp, tmp, rot);
+        tcg_gen_and_i32(dst, dst, mask);
+        tcg_gen_or_i32(dst, dst, tmp);
+
+        tcg_temp_free(rot);
+        tcg_temp_free(mask);
+    }
+    tcg_temp_free(tmp);
+}
+
 DISAS_INSN(ff1)
 {
     TCGv reg;
@@ -3795,6 +3999,12 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(shift16_reg, e060, f0f0, M68000);
     INSN(shift_reg, e0a0, f0f0, M68000);
     INSN(shift_mem, e0c0, fcc0, M68000);
+    INSN(bfext_reg, e9c0, fdf8, BITFIELD);  /* bfextu & bfexts */
+    INSN(bfins_reg, efc0, fff8, BITFIELD);
+    INSN(bfop_reg, eac0, fff8, BITFIELD);   /* bfchg */
+    INSN(bfop_reg, ecc0, fff8, BITFIELD);   /* bfclr */
+    INSN(bfop_reg, eec0, fff8, BITFIELD);   /* bfset */
+    INSN(bfop_reg, e8c0, fff8, BITFIELD);   /* bftst */
     INSN(undef_fpu, f000, f000, CF_ISA_A);
     INSN(fpu,       f200, ffc0, CF_FPU);
     INSN(fbcc,      f280, ffc0, CF_FPU);
-- 
2.7.4

[Qemu-devel] [PATCH 5/5] target-m68k: Implement bitfield ops for memory

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-m68k/cpu.h       |   1 +
 target-m68k/helper.c    | 183 +++++++++++++++++++++++++++++++++++++++++++++++-
 target-m68k/helper.h    |   7 ++
 target-m68k/translate.c | 142 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 330 insertions(+), 3 deletions(-)

diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 0b4ed7b..fc1f16f 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -37,6 +37,7 @@
 #define OS_DOUBLE   4
 #define OS_EXTENDED 5
 #define OS_PACKED   6
+#define OS_UNSIZED  7
 
 #define MAX_QREGS 32
 
diff --git a/target-m68k/helper.c b/target-m68k/helper.c
index f750d3d..4b0235c 100644
--- a/target-m68k/helper.c
+++ b/target-m68k/helper.c
@@ -22,8 +22,8 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/gdbstub.h"
-
 #include "exec/helper-proto.h"
+#include "exec/cpu_ldst.h"
 
 #define SIGNBIT (1u << 31)
 
@@ -756,3 +756,184 @@ void HELPER(set_mac_extu)(CPUM68KState *env, uint32_t val, uint32_t acc)
     res |= (uint64_t)(val & 0xffff0000) << 16;
     env->macc[acc + 1] = res;
 }
+
+struct bf_data {
+    uint64_t mask;
+    uint32_t addr;
+    int bofs;
+    int blen;
+    int len;
+};
+
+static inline struct bf_data bf_prep(uint32_t addr, int32_t ofs, uint32_t len)
+{
+    uint64_t mask;
+    int bofs;
+
+    /* Bound length; map 0 to 32.  */
+    len = ((len - 1) & 31) + 1;
+    mask = -1ull << (64 - len);
+
+    /* Note that ofs is signed.  */
+    addr += ofs / 8;
+    bofs = ofs % 8;
+    if (bofs < 0) {
+        bofs += 8;
+        addr -= 1;
+    }
+
+    return (struct bf_data){
+        .mask = mask,
+        .addr = addr,
+        .bofs = bofs,
+        .blen = (bofs + len - 1) / 8,
+        .len = len,
+    };
+}
+
+static inline uint64_t bf_load(CPUM68KState *env, struct bf_data *d,
+                               uintptr_t ra)
+{
+    uint32_t addr = d->addr;
+    uint64_t data;
+
+    /* Be careful not to read bytes across page boundaries unless
+       absolutely necessary.  */
+    switch (d->blen) {
+    case 0:
+        d->bofs += 56;
+        data = cpu_ldub_data_ra(env, addr, ra);
+        break;
+
+    case 1:
+        d->bofs += 16;
+        data = cpu_lduw_data_ra(env, addr, ra);
+        break;
+
+    case 2:
+        if ((addr & 3) <= 1) {
+            d->bofs += (addr & 3) * 8;
+            addr -= (addr & 3);
+            d->addr = addr;
+        }
+        /* fallthru */
+
+    case 3:
+        data = cpu_ldl_data_ra(env, addr, ra);
+        break;
+
+    case 4:
+        if ((addr & 7) <= 3) {
+            d->bofs += (addr & 7) * 8;
+            addr -= (addr & 7);
+            d->addr = addr;
+        }
+        data = cpu_ldq_data_ra(env, addr, ra);
+        break;
+
+    default:
+        g_assert_not_reached();
+    }
+
+    return data;
+}
+
+static inline void bf_store(CPUM68KState *env, struct bf_data *d,
+                            uint64_t data, uintptr_t ra)
+{
+    uint32_t addr = d->addr;
+
+    switch (d->blen) {
+    case 0:
+        cpu_stb_data_ra(env, addr, data, ra);
+        break;
+    case 1:
+        cpu_stw_data_ra(env, addr, data, ra);
+        break;
+    case 2:
+    case 3:
+        cpu_stl_data_ra(env, addr, data, ra);
+        break;
+    case 4:
+        cpu_stq_data_ra(env, addr, data, ra);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+}
+
+uint32_t HELPER(bfexts_mem)(CPUM68KState *env, uint32_t addr,
+                            int32_t ofs, uint32_t len)
+{
+    uintptr_t ra = GETPC();
+    struct bf_data d = bf_prep(addr, ofs, len);
+    uint64_t data = bf_load(env, &d, ra);
+
+    return (int64_t)(data << d.bofs) >> (64 - len);
+}
+
+uint64_t HELPER(bfextu_mem)(CPUM68KState *env, uint32_t addr,
+                            int32_t ofs, uint32_t len)
+{
+    uintptr_t ra = GETPC();
+    struct bf_data d = bf_prep(addr, ofs, len);
+    uint64_t data = bf_load(env, &d, ra);
+
+    /* Put CC_N at the top of the high word; put the zero-extended value
+       at the bottom of the low word.  */
+    data <<= d.bofs;
+    data &= d.mask;
+    data |= data >> (64 - d.len);
+    return data;
+}
+
+uint32_t HELPER(bfins_mem)(CPUM68KState *env, uint32_t addr, uint32_t val,
+                           int32_t ofs, uint32_t len)
+{
+    uintptr_t ra = GETPC();
+    struct bf_data d = bf_prep(addr, ofs, len);
+    uint64_t data = bf_load(env, &d, ra);
+
+    data = (data & ~(d.mask >> d.bofs)) | (((uint64_t)val << 32) >> d.bofs);
+
+    bf_store(env, &d, data, ra);
+
+    /* The field at the top of the word is also CC_N for CC_OP_LOGIC.  */
+    return val;
+}
+
+uint32_t HELPER(bfchg_mem)(CPUM68KState *env, uint32_t addr,
+                           int32_t ofs, uint32_t len)
+{
+    uintptr_t ra = GETPC();
+    struct bf_data d = bf_prep(addr, ofs, len);
+    uint64_t data = bf_load(env, &d, ra);
+
+    bf_store(env, &d, data ^ (d.mask >> d.bofs), ra);
+
+    return (data << d.bofs) >> 32;
+}
+
+uint32_t HELPER(bfclr_mem)(CPUM68KState *env, uint32_t addr,
+                           int32_t ofs, uint32_t len)
+{
+    uintptr_t ra = GETPC();
+    struct bf_data d = bf_prep(addr, ofs, len);
+    uint64_t data = bf_load(env, &d, ra);
+
+    bf_store(env, &d, data &~ (d.mask >> d.bofs), ra);
+
+    return (data << d.bofs) >> 32;
+}
+
+uint32_t HELPER(bfset_mem)(CPUM68KState *env, uint32_t addr,
+                           int32_t ofs, uint32_t len)
+{
+    uintptr_t ra = GETPC();
+    struct bf_data d = bf_prep(addr, ofs, len);
+    uint64_t data = bf_load(env, &d, ra);
+
+    bf_store(env, &d, data | (d.mask >> d.bofs), ra);
+
+    return (data << d.bofs) >> 32;
+}
diff --git a/target-m68k/helper.h b/target-m68k/helper.h
index d863e55..8db741a 100644
--- a/target-m68k/helper.h
+++ b/target-m68k/helper.h
@@ -48,3 +48,10 @@ DEF_HELPER_2(flush_flags, void, env, i32)
 DEF_HELPER_2(set_ccr, void, env, i32)
 DEF_HELPER_FLAGS_1(get_ccr, TCG_CALL_NO_WG_SE, i32, env)
 DEF_HELPER_2(raise_exception, void, env, i32)
+
+DEF_HELPER_FLAGS_4(bfexts_mem, TCG_CALL_NO_WG, i32, env, i32, s32, i32)
+DEF_HELPER_FLAGS_4(bfextu_mem, TCG_CALL_NO_WG, i64, env, i32, s32, i32)
+DEF_HELPER_FLAGS_5(bfins_mem, TCG_CALL_NO_WG, i32, env, i32, i32, s32, i32)
+DEF_HELPER_FLAGS_4(bfchg_mem, TCG_CALL_NO_WG, i32, env, i32, s32, i32)
+DEF_HELPER_FLAGS_4(bfclr_mem, TCG_CALL_NO_WG, i32, env, i32, s32, i32)
+DEF_HELPER_FLAGS_4(bfset_mem, TCG_CALL_NO_WG, i32, env, i32, s32, i32)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 477a511..d2dd4bd 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -709,10 +709,17 @@ static TCGv gen_lea_mode(CPUM68KState *env, DisasContext *s,
     case 0: /* Data register direct.  */
     case 1: /* Address register direct.  */
         return NULL_QREG;
-    case 2: /* Indirect register */
     case 3: /* Indirect postincrement.  */
+        if (opsize == OS_UNSIZED) {
+            return NULL_QREG;
+        }
+        /* fallthru */
+    case 2: /* Indirect register */
         return get_areg(s, reg0);
     case 4: /* Indirect predecrememnt.  */
+        if (opsize == OS_UNSIZED) {
+            return NULL_QREG;
+        }
         reg = get_areg(s, reg0);
         tmp = tcg_temp_new();
         tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
@@ -2786,6 +2793,49 @@ DISAS_INSN(bfext_reg)
     set_cc_op(s, CC_OP_LOGIC);
 }
 
+DISAS_INSN(bfext_mem)
+{
+    int ext = read_im16(env, s);
+    int is_sign = insn & 0x200;
+    TCGv dest = DREG(ext, 12);
+    TCGv addr, len, ofs;
+
+    addr = gen_lea(env, s, insn, OS_UNSIZED);
+    if (IS_NULL_QREG(addr)) {
+        gen_addr_fault(s);
+        return;
+    }
+
+    if (ext & 0x20) {
+        len = DREG(ext, 0);
+    } else {
+        len = tcg_const_i32(extract32(ext, 0, 5));
+    }
+    if (ext & 0x800) {
+        ofs = DREG(ext, 6);
+    } else {
+        ofs = tcg_const_i32(extract32(ext, 6, 5));
+    }
+
+    if (is_sign) {
+        gen_helper_bfexts_mem(dest, cpu_env, addr, ofs, len);
+        tcg_gen_mov_i32(QREG_CC_N, dest);
+    } else {
+        TCGv_i64 tmp = tcg_temp_new_i64();
+        gen_helper_bfextu_mem(tmp, cpu_env, addr, ofs, len);
+        tcg_gen_extr_i64_i32(dest, QREG_CC_N, tmp);
+        tcg_temp_free_i64(tmp);
+    }
+    set_cc_op(s, CC_OP_LOGIC);
+
+    if (!(ext & 0x20)) {
+        tcg_temp_free(len);
+    }
+    if (!(ext & 0x800)) {
+        tcg_temp_free(ofs);
+    }
+}
+
 DISAS_INSN(bfop_reg)
 {
     int ext = read_im16(env, s);
@@ -2851,6 +2901,54 @@ DISAS_INSN(bfop_reg)
     tcg_temp_free(mask);
 }
 
+DISAS_INSN(bfop_mem)
+{
+    int ext = read_im16(env, s);
+    TCGv addr, len, ofs;
+
+    addr = gen_lea(env, s, insn, OS_UNSIZED);
+    if (IS_NULL_QREG(addr)) {
+        gen_addr_fault(s);
+        return;
+    }
+
+    if (ext & 0x20) {
+        len = DREG(ext, 0);
+    } else {
+        len = tcg_const_i32(extract32(ext, 0, 5));
+    }
+    if (ext & 0x800) {
+        ofs = DREG(ext, 6);
+    } else {
+        ofs = tcg_const_i32(extract32(ext, 6, 5));
+    }
+
+    switch (insn & 0x0f00) {
+    case 0x0a00: /* bfchg */
+        gen_helper_bfchg_mem(QREG_CC_N, cpu_env, addr, ofs, len);
+        break;
+    case 0x0c00: /* bfclr */
+        gen_helper_bfclr_mem(QREG_CC_N, cpu_env, addr, ofs, len);
+        break;
+    case 0x0e00: /* bfset */
+        gen_helper_bfset_mem(QREG_CC_N, cpu_env, addr, ofs, len);
+        break;
+    case 0x0800: /* bftst */
+        gen_helper_bfexts_mem(QREG_CC_N, cpu_env, addr, ofs, len);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+    set_cc_op(s, CC_OP_LOGIC);
+
+    if (!(ext & 0x20)) {
+        tcg_temp_free(len);
+    }
+    if (!(ext & 0x800)) {
+        tcg_temp_free(ofs);
+    }
+}
+
 DISAS_INSN(bfins_reg)
 {
     int ext = read_im16(env, s);
@@ -2925,6 +3023,40 @@ DISAS_INSN(bfins_reg)
     tcg_temp_free(tmp);
 }
 
+DISAS_INSN(bfins_mem)
+{
+    int ext = read_im16(env, s);
+    TCGv src = DREG(ext, 12);
+    TCGv addr, len, ofs;
+
+    addr = gen_lea(env, s, insn, OS_UNSIZED);
+    if (IS_NULL_QREG(addr)) {
+        gen_addr_fault(s);
+        return;
+    }
+
+    if (ext & 0x20) {
+        len = DREG(ext, 0);
+    } else {
+        len = tcg_const_i32(extract32(ext, 0, 5));
+    }
+    if (ext & 0x800) {
+        ofs = DREG(ext, 6);
+    } else {
+        ofs = tcg_const_i32(extract32(ext, 6, 5));
+    }
+
+    gen_helper_bfins_mem(QREG_CC_N, cpu_env, addr, src, ofs, len);
+    set_cc_op(s, CC_OP_LOGIC);
+
+    if (!(ext & 0x20)) {
+        tcg_temp_free(len);
+    }
+    if (!(ext & 0x800)) {
+        tcg_temp_free(ofs);
+    }
+}
+
 DISAS_INSN(ff1)
 {
     TCGv reg;
@@ -3999,11 +4131,17 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(shift16_reg, e060, f0f0, M68000);
     INSN(shift_reg, e0a0, f0f0, M68000);
     INSN(shift_mem, e0c0, fcc0, M68000);
-    INSN(bfext_reg, e9c0, fdf8, BITFIELD);  /* bfextu & bfexts */
+    INSN(bfext_mem, e9c0, fdc0, BITFIELD);  /* bfextu & bfexts */
+    INSN(bfext_reg, e9c0, fdf8, BITFIELD);
+    INSN(bfins_mem, efc0, ffc0, BITFIELD);
     INSN(bfins_reg, efc0, fff8, BITFIELD);
+    INSN(bfop_mem, eac0, ffc0, BITFIELD);   /* bfchg */
     INSN(bfop_reg, eac0, fff8, BITFIELD);   /* bfchg */
+    INSN(bfop_mem, ecc0, ffc0, BITFIELD);   /* bfclr */
     INSN(bfop_reg, ecc0, fff8, BITFIELD);   /* bfclr */
+    INSN(bfop_mem, eec0, ffc0, BITFIELD);   /* bfset */
     INSN(bfop_reg, eec0, fff8, BITFIELD);   /* bfset */
+    INSN(bfop_mem, e8c0, ffc0, BITFIELD);   /* bftst */
     INSN(bfop_reg, e8c0, fff8, BITFIELD);   /* bftst */
     INSN(undef_fpu, f000, f000, CF_ISA_A);
     INSN(fpu,       f200, ffc0, CF_FPU);
-- 
2.7.4

Re: [Qemu-devel] [PATCH 1/5] target-m68k: implement 680x0 movem

[Laurent Vivier]

Le 06/11/2016 à 07:18, Richard Henderson a écrit :
...
> +    if (is_load) {
> +        /* memory to register */
> +        for (i = 0; i < 16; i++) {
> +            if (mask & (1 << i)) {
> +                r[i] = gen_load(s, opsize, addr, 1);
> +                tcg_gen_add_i32(addr, addr, incr);
> +            }
> +        }
> +        for (i = 0; i < 16; i++) {
> +            if (mask & (1 << i)) {
> +                tcg_gen_mov_i32(mreg(i), r[i]);
> +                tcg_temp_free(r[i]);
> +            }
> +        }
> +        if (mode == 3) {
> +            /* post-increment: movem (An)+,X */
> +            tcg_gen_mov_i32(AREG(insn, 0), addr);

Why AREG() instead of cpu_aregs[]?

> +        }
> +    } else {
> +        /* register to memory */
> +        if (mode == 4) {
> +            /* pre-decrement: movem X,-(An) */
> +            for (i = 15; i >= 0; i--) {
> +                if ((mask << i) & 0x8000) {
> +                    tcg_gen_sub_i32(addr, addr, incr);
> +                    if (reg0 + 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, regardless of how many actual
> +                         * stores have been performed until this point.
> +                         * M68000/M68010: the value is the initial value.
> +                         */
> +                        TCGv tmp = tcg_temp_new();

tmp is already declared at the top of the function, perhaps you can
remove the declaration part here?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 2/5] target-m68k: Do not cpu_abort on undefined insns

[Laurent Vivier]

Le 06/11/2016 à 07:18, Richard Henderson a écrit :
> Report this properly via exception and, importantly, allow
> the disassembler the chance to tell us what insn is not handled.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Laurent Vivier]

Le 06/11/2016 à 07:18, Richard Henderson a écrit :
> I believe the first two patches have already been posted.
> No changes there, but included here for completeness.
> 
> Next is a proposed shift patch, with the changes I believe
> I mentioned wanting during review.
> 
> Finally, implementations of the bitfield instructions.  At least
> the register portion depends on the generic tcg (s)extract primitives
> that I have queued for 2.9.
> 
> The full tree is at
> 
>   git://github.com/rth7680/qemu.git tgt-m68k
> 
> 
> r~
> 
> 
> Laurent Vivier (1):
>   target-m68k: implement 680x0 movem
> 
> Richard Henderson (4):
>   target-m68k: Do not cpu_abort on undefined insns
>   target-m68k: Inline shifts
>   target-m68k: Implement bitfield ops for registers
>   target-m68k: Implement bitfield ops for memory

There is something wrong with the bitfield patches, because when I use
them instead of the version I use usually I'm not able to boot a
container with qemu-m68k (SEGFAULT).

Laurent

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Richard Henderson]

On 11/06/2016 08:52 PM, Laurent Vivier wrote:
>> Richard Henderson (4):
>>   target-m68k: Do not cpu_abort on undefined insns
>>   target-m68k: Inline shifts
>>   target-m68k: Implement bitfield ops for registers
>>   target-m68k: Implement bitfield ops for memory
>
> There is something wrong with the bitfield patches, because when I use
> them instead of the version I use usually I'm not able to boot a
> container with qemu-m68k (SEGFAULT).

Ah, too bad.  I was testing with the linux-user-0.3 busybox, but we don't yet 
implement enough to actually execute it.  So the testing consisted of maybe 3 
or 4 such insns until I reach the fmovemx.  ;-)

Can you tell if the breakage is limited to the memory patch, or if the register 
patch is also broken?


r~

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Laurent Vivier]

Le 07/11/2016 à 13:08, Richard Henderson a écrit :
> On 11/06/2016 08:52 PM, Laurent Vivier wrote:
>>> Richard Henderson (4):
>>>   target-m68k: Do not cpu_abort on undefined insns
>>>   target-m68k: Inline shifts
>>>   target-m68k: Implement bitfield ops for registers
>>>   target-m68k: Implement bitfield ops for memory
>>
>> There is something wrong with the bitfield patches, because when I use
>> them instead of the version I use usually I'm not able to boot a
>> container with qemu-m68k (SEGFAULT).
> 
> Ah, too bad.  I was testing with the linux-user-0.3 busybox, but we
> don't yet implement enough to actually execute it.  So the testing
> consisted of maybe 3 or 4 such insns until I reach the fmovemx.  ;-)
> 
> Can you tell if the breakage is limited to the memory patch, or if the
> register patch is also broken?

I need to test more, because if I only apply the register patch, I have
an illegal instruction exception.

If it can help:

sudo QEMU_LOG=in_asm chroot /var/lib/lxc/virtm68k-etch-m68k/rootfs 2>&1
|grep bf
0xf67f9160:  bfextu %d3,27,3,%d1
0xf67f8988:  bftst %d1,30,2
0xf67f2786:  bfins %d0,%a1@,0,2
0xf67eeb56:  bfins %d0,%a2@(824),0,2
0xf67f362c:  bfins %d0,%a2@(384),5,2
0xf67f8b80:  bftst %d1,30,2
0xf67f8b9e:  bftst %d4,30,2
0xf67f8998:  bftst %d0,30,2
0xf67f8998:  bftst %d0,30,2
0xf67f9160:  bfextu %d3,27,3,%d1
0xf67f8f88:  bftst %d4,30,2
0xf67f39d8:  bfins %d0,%a1@(384),5,2
0xf67f3876:  bfclr %a0@(384),5,2

If you want to have a try, this tree is based on tgt-m68k + what is
needed to boot an etch-m68k container.

git://github.com/vivier/qemu-m68k.git for-rth

The working tree is:

git://github.com/vivier/qemu-m68k.git m68k-dev

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Peter Maydell]

On 7 November 2016 at 12:24, Laurent Vivier <laurent@vivier.eu> wrote:
> If you want to have a try, this tree is based on tgt-m68k + what is
> needed to boot an etch-m68k container.
>
> git://github.com/vivier/qemu-m68k.git for-rth

You could try using 'risu' to do single-instruction level
comprehensive testing, if you've got enough of a linux-user port
to be able to run it...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Laurent Vivier]

Le 07/11/2016 à 14:12, Peter Maydell a écrit :
> On 7 November 2016 at 12:24, Laurent Vivier <laurent@vivier.eu> wrote:
>> If you want to have a try, this tree is based on tgt-m68k + what is
>> needed to boot an etch-m68k container.
>>
>> git://github.com/vivier/qemu-m68k.git for-rth
> 
> You could try using 'risu' to do single-instruction level
> comprehensive testing, if you've got enough of a linux-user port
> to be able to run it...

Thank you Peter.

I know risu, I would like to port it to m68k but for the moment it's too
much work. I think someone is already working on a port to PPC, perhaps
after that it will be easier to port it to m68k.

For m68k, I've taken the work from Ray Arachelian [1] for the LISA
emulator and Gwenole Beauchesne for Basilisk II [2].

I also test FPU with testfloat and softfloat. [3]

Laurent

[1] http://lisa.sunder.net/cputest.html
[2] https://github.com/vivier/m68k-testgen
    https://github.com/vivier/m68k-tester
    (I have 584MB of results I can't put in the repo..)
[3] https://github.com/vivier/m68k-testfloat
    https://github.com/vivier/m68k-softfloat

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Peter Maydell]

On 7 November 2016 at 13:44, Laurent Vivier <laurent@vivier.eu> wrote:
> I know risu, I would like to port it to m68k but for the moment it's too
> much work. I think someone is already working on a port to PPC, perhaps
> after that it will be easier to port it to m68k.

Yes, I have the ppc patchset on my review queue.

> For m68k, I've taken the work from Ray Arachelian [1] for the LISA
> emulator and Gwenole Beauchesne for Basilisk II [2].

Cool. If you've already got a decent test coverage setup then
risu probably won't add much extra for you.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/5] target-m68k patches

[Laurent Vivier]

Le 07/11/2016 à 14:50, Peter Maydell a écrit :
> On 7 November 2016 at 13:44, Laurent Vivier <laurent@vivier.eu> wrote:
>> I know risu, I would like to port it to m68k but for the moment it's too
>> much work. I think someone is already working on a port to PPC, perhaps
>> after that it will be easier to port it to m68k.
> 
> Yes, I have the ppc patchset on my review queue.

Great!

>> For m68k, I've taken the work from Ray Arachelian [1] for the LISA
>> emulator and Gwenole Beauchesne for Basilisk II [2].
> 
> Cool. If you've already got a decent test coverage setup then
> risu probably won't add much extra for you.

I think risu is better because it is more flexible and able to test
memory access. My current test suite can only test instructions with
data in registers.

Laurent