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

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

[Richard Henderson]

Changes since v1:
  * Tweaked local variable decls for movem.
  * Fixed the bitfield patches such that they work with linux-user-test-0.3.
    (Along with the whole passle of extra patches required to make that work.)

The register bitfield patch still depends on the patches posted
here in another thread implementing extract and sextract.


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    |  52 ----
 target-m68k/helper.h    |  10 +-
 target-m68k/op_helper.c | 185 +++++++++++++
 target-m68k/translate.c | 702 ++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 843 insertions(+), 107 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v2 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>
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 5d91e4e..ca99072 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1793,40 +1793,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(cpu_aregs[reg0], 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.
+                         */
+                        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)
@@ -4029,7 +4111,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 v2 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.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
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 ca99072..4f224d7 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1577,10 +1577,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 v2 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 | 214 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 189 insertions(+), 80 deletions(-)

diff --git a/target-m68k/helper.c b/target-m68k/helper.c
index 60fe997..25d5dc1 100644
--- a/target-m68k/helper.c
+++ b/target-m68k/helper.c
@@ -320,58 +320,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;
-}
-
 /* MAC unit.  */
 /* FIXME: The MAC unit implementation is a bit of a mess.  Some helpers
    take values,  others take register numbers and manipulate the contents
diff --git a/target-m68k/helper.h b/target-m68k/helper.h
index 226ef5f..cae5bcb 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 4f224d7..1b3765f 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2883,48 +2883,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)
+static inline void shift_im(DisasContext *s, uint16_t insn, int opsize)
 {
-    TCGv reg;
-    int tmp;
-    TCGv shift;
+    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);
 }
 
@@ -4208,6 +4365,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, ff80, CF_FPU);
-- 
2.7.4

[Qemu-devel] [PATCH v2 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 1b3765f..be59c37 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -3085,6 +3085,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 = 32 - ofs - len;        /* 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 < 0) {
+                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 = 32 - ofs - len;        /* 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 >= 0) {
+            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;
@@ -4372,6 +4576,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, ff80, CF_FPU);
-- 
2.7.4

[Qemu-devel] [PATCH v2 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.h    |   7 ++
 target-m68k/op_helper.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-m68k/translate.c | 142 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 333 insertions(+), 2 deletions(-)

diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 7c119ba..0b63c0a 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.h b/target-m68k/helper.h
index cae5bcb..301b7d1 100644
--- a/target-m68k/helper.h
+++ b/target-m68k/helper.h
@@ -77,3 +77,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/op_helper.c b/target-m68k/op_helper.c
index a4bfa4e..77d1f58 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -359,3 +359,188 @@ void HELPER(divsll)(CPUM68KState *env, int numr, int regr, int32_t den)
     env->dregs[regr] = rem;
     env->dregs[numr] = quot;
 }
+
+struct bf_data {
+    uint32_t addr;
+    uint32_t bofs;
+    uint32_t blen;
+    uint32_t len;
+};
+
+static struct bf_data bf_prep(uint32_t addr, int32_t ofs, uint32_t len)
+{
+    int bofs, blen;
+
+    /* Bound length; map 0 to 32.  */
+    len = ((len - 1) & 31) + 1;
+
+    /* Note that ofs is signed.  */
+    addr += ofs / 8;
+    bofs = ofs % 8;
+    if (bofs < 0) {
+        bofs += 8;
+        addr -= 1;
+    }
+
+    /* Compute the number of bytes required (minus one) to
+       satisfy the bitfield.  */
+    blen = (bofs + len - 1) / 8;
+
+    /* Canonicalize the bit offset for data loaded into a 64-bit big-endian
+       word.  For the cases where BLEN is not a power of 2, adjust ADDR so
+       that we can use the next power of two sized load without crossing a
+       page boundary, unless the field itself crosses the boundary.  */
+    switch (blen) {
+    case 0:
+        bofs += 56;
+        break;
+    case 1:
+        bofs += 48;
+        break;
+    case 2:
+        if (addr & 1) {
+            bofs += 8;
+            addr -= 1;
+        }
+        /* fallthru */
+    case 3:
+        bofs += 32;
+        break;
+    case 4:
+        if (addr & 3) {
+            bofs += 8 * (addr & 3);
+            addr &= -4;
+        }
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return (struct bf_data){
+        .addr = addr,
+        .bofs = bofs,
+        .blen = blen,
+        .len = len,
+    };
+}
+
+static uint64_t bf_load(CPUM68KState *env, uint32_t addr, int blen,
+                        uintptr_t ra)
+{
+    switch (blen) {
+    case 0:
+        return cpu_ldub_data_ra(env, addr, ra);
+    case 1:
+        return cpu_lduw_data_ra(env, addr, ra);
+    case 2:
+    case 3:
+        return cpu_ldl_data_ra(env, addr, ra);
+    case 4:
+        return cpu_ldq_data_ra(env, addr, ra);
+    default:
+        g_assert_not_reached();
+    }
+}
+
+static void bf_store(CPUM68KState *env, uint32_t addr, int blen,
+                     uint64_t data, uintptr_t ra)
+{
+    switch (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.addr, d.blen, 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.addr, d.blen, 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 >>= 64 - d.len;
+    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.addr, d.blen, ra);
+    uint64_t mask = -1ull << (64 - d.len) >> d.bofs;
+
+    data = (data & ~mask) | (((uint64_t)val << (64 - d.len)) >> d.bofs);
+
+    bf_store(env, d.addr, d.blen, data, ra);
+
+    /* The field at the top of the word is also CC_N for CC_OP_LOGIC.  */
+    return val << (32 - d.len);
+}
+
+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.addr, d.blen, ra);
+    uint64_t mask = -1ull << (64 - d.len) >> d.bofs;
+
+    bf_store(env, d.addr, d.blen, data ^ mask, ra);
+
+    return ((data & mask) << 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.addr, d.blen, ra);
+    uint64_t mask = -1ull << (64 - d.len) >> d.bofs;
+
+    bf_store(env, d.addr, d.blen, data & ~mask, ra);
+
+    return ((data & mask) << 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.addr, d.blen, ra);
+    uint64_t mask = -1ull << (64 - d.len) >> d.bofs;
+
+    bf_store(env, d.addr, d.blen, data | mask, ra);
+
+    return ((data & mask) << d.bofs) >> 32;
+}
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index be59c37..61d9681 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -752,10 +752,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));
@@ -3150,6 +3157,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);
@@ -3215,6 +3265,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);
@@ -3289,6 +3387,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;
@@ -4576,11 +4708,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 v2 0/5] target-m68k-patches

[Laurent Vivier]

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> Changes since v1:
>   * Tweaked local variable decls for movem.
>   * Fixed the bitfield patches such that they work with linux-user-test-0.3.
>     (Along with the whole passle of extra patches required to make that work.)
> 
> The register bitfield patch still depends on the patches posted
> here in another thread implementing extract and sextract.
> 
> 
> 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    |  52 ----
>  target-m68k/helper.h    |  10 +-
>  target-m68k/op_helper.c | 185 +++++++++++++
>  target-m68k/translate.c | 702 ++++++++++++++++++++++++++++++++++++++++++++----
>  5 files changed, 843 insertions(+), 107 deletions(-)
> 

It seems I have always a segfault when I start a container. I'm going to
try to understand what happens.

Thanks,
Laurent

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

[Laurent Vivier]

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index 4f224d7..1b3765f 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -2883,48 +2883,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)
> +static inline void shift_im(DisasContext *s, uint16_t insn, int opsize)
>  {
> -    TCGv reg;
> -    int tmp;
> -    TCGv shift;
> +    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);

t0 is used unitialized, I think we should have here:

                tcg_gen_sari_i32(t0, reg, bits - count - 1);

moreover we must use "bits - count - 1" to use also the current most
significant bit to compute V flag.

> +                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);
> +        }
> +    }

Thanks,
Laurent

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

[Laurent Vivier]

There is another bug on this one.

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index 4f224d7..1b3765f 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> +static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
...
> +        /* 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);

if s64 is greater than 32, we lose all the bits needed to compute the V
flag. I think we can just add this to fix the problem:

         if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+            TCGv_i64 tt = tcg_const_i64(32);
+            /* if shift is greater than 32, use 32 */
+            tcg_gen_movcond_i64(TCG_COND_GT, s64, s64, tt, tt, s64);
+            tcg_temp_free_i64(tt);
             /* 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);

Is it correct?

Laurent

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

[Laurent Vivier]

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index 4f224d7..1b3765f 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c

> +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);

This does not extract correctly the C flag when the opsize is word or byte.
I think we should use a shift instead:

-        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.  */
+        if (opsize == OS_LONG) {
+            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.  */
+        } else {
+            TCGv zero = tcg_const_i32(0);
+            tcg_gen_extrl_i64_i32(QREG_CC_N, t64);
+            tcg_gen_shri_i64(t64, t64, bits);
+            tcg_gen_extrl_i64_i32(QREG_CC_C, t64);
+            tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_C,
+                                s32, zero, zero, QREG_CC_C);
+            tcg_temp_free(zero);
+        }

Do you have a better idea?

thanks,
Laurent

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

[Laurent Vivier]

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> 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 1b3765f..be59c37 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c

> +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 = 32 - ofs - len;        /* 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 >= 0) {
> +            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);

should be:

               tcg_gen_andi_i32(tmp, src, ~maski);

Is it correct?

Thanks,
Laurent

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

[Laurent Vivier]

Le 09/11/2016 à 14:46, Richard Henderson a écrit :
> --- a/target-m68k/op_helper.c
> +++ b/target-m68k/op_helper.c
> +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.addr, d.blen, ra);
> +
> +    return (int64_t)(data << d.bofs) >> (64 - len);

We must use d.len instead of len, otherwise we shift with 32bit value.

-    return (int64_t)(data << d.bofs) >> (64 - len);
+    return (int64_t)(data << d.bofs) >> (64 - d.len);


Laurent

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

[Richard Henderson]

On 11/27/2016 09:53 AM, Laurent Vivier wrote:
>> > +                TCGv t0 = tcg_temp_new();
>> > +                tcg_gen_sari_i32(QREG_CC_V, reg, bits - 1);
>> > +                tcg_gen_sari_i32(t0, t0, bits - count);
> t0 is used unitialized, I think we should have here:
> 
>                 tcg_gen_sari_i32(t0, reg, bits - count - 1);
> 
> moreover we must use "bits - count - 1" to use also the current most
> significant bit to compute V flag.
> 

Yep.


r~

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

[Richard Henderson]

On 11/27/2016 11:30 AM, Laurent Vivier wrote:
> There is another bug on this one.
> 
> Le 09/11/2016 à 14:46, Richard Henderson a écrit :
>> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
>> index 4f224d7..1b3765f 100644
>> --- a/target-m68k/translate.c
>> +++ b/target-m68k/translate.c
>> +static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
> ...
>> +        /* 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);
> 
> if s64 is greater than 32, we lose all the bits needed to compute the V
> flag. I think we can just add this to fix the problem:
> 
>          if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +            TCGv_i64 tt = tcg_const_i64(32);
> +            /* if shift is greater than 32, use 32 */
> +            tcg_gen_movcond_i64(TCG_COND_GT, s64, s64, tt, tt, s64);
> +            tcg_temp_free_i64(tt);
>              /* 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);

Hmm.  I guess the test case is input like 8 << 63, where the sign bit is clear,
the input is non-zero, and we shift out all of the set bits.

I can't think of anything cleaner than your movcond solution.


r~

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

[Richard Henderson]

On 11/27/2016 11:35 AM, Laurent Vivier wrote:
>> > +        tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
> This does not extract correctly the C flag when the opsize is word or byte.
> I think we should use a shift instead:
> 
> -        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.  */
> +        if (opsize == OS_LONG) {
> +            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.  */
> +        } else {
> +            TCGv zero = tcg_const_i32(0);
> +            tcg_gen_extrl_i64_i32(QREG_CC_N, t64);
> +            tcg_gen_shri_i64(t64, t64, bits);
> +            tcg_gen_extrl_i64_i32(QREG_CC_C, t64);
> +            tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_C,
> +                                s32, zero, zero, QREG_CC_C);
> +            tcg_temp_free(zero);
> +        }
> 
> Do you have a better idea?

  if (opsize == OS_LONG) {
    tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
  } else {
    tcg_gen_extrl_i64_i32(QREG_CC_N, t64);
    tcg_gen_shri_i32(QREG_CC_C, QREG_CC_N, bits);
  }

Since we zero-extend the input from bits, it's still true that a zero shift
gets C=0 for free.


r~

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

[Richard Henderson]

On 11/27/2016 11:46 AM, Laurent Vivier wrote:
>> +            uint32_t maski = -2U << (len - 1);
>> > +            uint32_t roti = (ofs + len) & 31;
>> > +            tcg_gen_andi_i32(tmp, src, maski);
> should be:
> 
>                tcg_gen_andi_i32(tmp, src, ~maski);
> 
> Is it correct?

Yes.


r~

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

[Richard Henderson]

On 11/27/2016 11:48 AM, Laurent Vivier wrote:
>> > +    return (int64_t)(data << d.bofs) >> (64 - len);
> We must use d.len instead of len, otherwise we shift with 32bit value.
> 
> -    return (int64_t)(data << d.bofs) >> (64 - len);
> +    return (int64_t)(data << d.bofs) >> (64 - d.len);

Whoops, yes.


r~

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

[Laurent Vivier]

Le 28/11/2016 à 15:54, Richard Henderson a écrit :
> On 11/27/2016 11:35 AM, Laurent Vivier wrote:
>>>> +        tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
>> This does not extract correctly the C flag when the opsize is word or byte.
>> I think we should use a shift instead:
>>
>> -        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.  */
>> +        if (opsize == OS_LONG) {
>> +            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.  */
>> +        } else {
>> +            TCGv zero = tcg_const_i32(0);
>> +            tcg_gen_extrl_i64_i32(QREG_CC_N, t64);
>> +            tcg_gen_shri_i64(t64, t64, bits);
>> +            tcg_gen_extrl_i64_i32(QREG_CC_C, t64);
>> +            tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_C,
>> +                                s32, zero, zero, QREG_CC_C);
>> +            tcg_temp_free(zero);
>> +        }
>>
>> Do you have a better idea?
> 
>   if (opsize == OS_LONG) {
>     tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
>   } else {
>     tcg_gen_extrl_i64_i32(QREG_CC_N, t64);
>     tcg_gen_shri_i32(QREG_CC_C, QREG_CC_N, bits);
>   }
> 
> Since we zero-extend the input from bits, it's still true that a zero shift
> gets C=0 for free.
> 

It doesn't work because we work with word/byte where the bit sign has
been extended to the long word. So in the case of 0 shift, with retrieve
C=<original bit sign> and not 0.

Laurent

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

[Richard Henderson]

On 11/29/2016 03:52 PM, Laurent Vivier wrote:
> It doesn't work because we work with word/byte where the bit sign has
> been extended to the long word. So in the case of 0 shift, with retrieve
> C=<original bit sign> and not 0.

Ah, right.  I wonder if it's better to always zero-extend the inputs for
left-shifts, as opposed to needing the movcond.


r~