* [Qemu-devel] [PATCH v2 0/3] 680x0 instruction set, part 2
@ 2016-10-27 21:43 Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm Laurent Vivier
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Laurent Vivier @ 2016-10-27 21:43 UTC (permalink / raw)
To: qemu-devel; +Cc: schwab, agraf, Richard Henderson, gerg, Laurent Vivier
This series is another subset of the series I sent in May:
https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg00501.html
It must be applied on top of series:
"target-m68k: 680x0 instruction set, part 1"
This subset contains reworked patches for:
- cmpm, delay the writeback of the value of the register
after the second load to avoid problem with page fault,
- shift instruction, compute V flags as advised by Richard
I've checked it doesn't break coldfire support:
http://wiki.qemu.org/download/coldfire-test-0.1.tar.bz2
but it can't boot a 680x0 processor kernel.
v2:
- don't modify gen_extend() prototype
- use one "sari" instead of two "shri"s
- use "movi 0" to set QREG_CC_V to zero
Laurent Vivier (2):
target-m68k: add cmpm
target-m68k: shift ops manage word and byte operands
Richard Henderson (1):
target-m68k: Inline shifts
target-m68k/helper.c | 52 ----------
target-m68k/helper.h | 3 -
target-m68k/translate.c | 255 +++++++++++++++++++++++++++++++++++++++++++-----
3 files changed, 229 insertions(+), 81 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm
2016-10-27 21:43 [Qemu-devel] [PATCH v2 0/3] 680x0 instruction set, part 2 Laurent Vivier
@ 2016-10-27 21:43 ` Laurent Vivier
2016-11-01 17:48 ` Richard Henderson
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 2/3] target-m68k: Inline shifts Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands Laurent Vivier
2 siblings, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2016-10-27 21:43 UTC (permalink / raw)
To: qemu-devel; +Cc: schwab, agraf, Richard Henderson, gerg, Laurent Vivier
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
target-m68k/translate.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index ee0ffe3..92e67eb 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2173,6 +2173,33 @@ DISAS_INSN(cmpa)
gen_update_cc_cmp(s, reg, src, opsize);
}
+DISAS_INSN(cmpm)
+{
+ int opsize = insn_opsize(insn);
+ TCGv tmp = tcg_temp_new();
+ TCGv src, dst, addr;
+
+ src = gen_load(s, opsize, AREG(insn, 0), 1);
+ /* delay the update after the second gen_load() */
+ tcg_gen_addi_i32(tmp, AREG(insn, 0), opsize_bytes(opsize));
+
+ /* but if the we use the same address register to
+ * read the second value, we must use the updated address
+ */
+ if (REG(insn, 0) == REG(insn, 9)) {
+ addr = tmp;
+ } else {
+ addr = AREG(insn, 9);
+ }
+
+ dst = gen_load(s, opsize, addr, 1);
+ tcg_gen_mov_i32(AREG(insn, 0), tmp);
+ tcg_gen_addi_i32(AREG(insn, 9), addr, opsize_bytes(opsize));
+ tcg_temp_free(tmp);
+
+ gen_update_cc_cmp(s, dst, src, opsize);
+}
+
DISAS_INSN(eor)
{
TCGv src;
@@ -3414,6 +3441,7 @@ void register_m68k_insns (CPUM68KState *env)
INSN(cmpa, b1c0, f1c0, CF_ISA_A);
INSN(cmp, b000, f100, M68000);
INSN(eor, b100, f100, M68000);
+ INSN(cmpm, b108, f138, M68000);
INSN(cmpa, b0c0, f0c0, M68000);
INSN(eor, b180, f1c0, CF_ISA_A);
BASE(and, c000, f000);
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] target-m68k: Inline shifts
2016-10-27 21:43 [Qemu-devel] [PATCH v2 0/3] 680x0 instruction set, part 2 Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm Laurent Vivier
@ 2016-10-27 21:43 ` Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands Laurent Vivier
2 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2016-10-27 21:43 UTC (permalink / raw)
To: qemu-devel; +Cc: schwab, agraf, Richard Henderson, gerg, Laurent Vivier
From: Richard Henderson <rth@twiddle.net>
Original patch from Richard Henderson <rth@twiddle.net>
Fix arithmetical/logical switch
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
target-m68k/helper.c | 52 ---------------------------
target-m68k/helper.h | 3 --
target-m68k/translate.c | 94 +++++++++++++++++++++++++++++++++++++------------
3 files changed, 72 insertions(+), 77 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 2697e32..aae01f9 100644
--- a/target-m68k/helper.h
+++ b/target-m68k/helper.h
@@ -3,9 +3,6 @@ DEF_HELPER_1(ff1, i32, i32)
DEF_HELPER_FLAGS_2(sats, TCG_CALL_NO_RWG_SE, i32, i32, i32)
DEF_HELPER_2(divu, void, env, i32)
DEF_HELPER_2(divs, void, env, i32)
-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 92e67eb..461d756 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2348,48 +2348,98 @@ 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;
+ TCGv reg = DREG(insn, 0);
+ int count = (insn >> 9) & 7;
+ int logical = insn & 8;
- set_cc_op(s, CC_OP_FLAGS);
+ if (count == 0) {
+ count = 8;
+ }
- 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);
+ tcg_gen_shri_i32(QREG_CC_C, reg, 31 - count);
+ tcg_gen_shli_i32(QREG_CC_N, reg, count);
} else {
- if (insn & 8) {
- gen_helper_shr_cc(reg, cpu_env, reg, shift);
+ tcg_gen_shri_i32(QREG_CC_C, reg, count - 1);
+ if (logical) {
+ tcg_gen_shri_i32(QREG_CC_N, reg, count);
} else {
- gen_helper_sar_cc(reg, cpu_env, reg, shift);
+ tcg_gen_sari_i32(QREG_CC_N, reg, count);
}
}
+ 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);
+
+ /* Note that ColdFire always clears V, while M68000 sets it for
+ a change in the sign bit. */
+ if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+ tcg_gen_xor_i32(QREG_CC_V, QREG_CC_N, reg);
+ } else {
+ tcg_gen_movi_i32(QREG_CC_V, 0);
+ }
+
+ tcg_gen_mov_i32(reg, QREG_CC_N);
+ set_cc_op(s, CC_OP_FLAGS);
}
DISAS_INSN(shift_reg)
{
- TCGv reg;
- TCGv shift;
+ TCGv reg, s32;
+ TCGv_i64 t64, s64;
+ int logical = insn & 8;
reg = DREG(insn, 0);
- shift = DREG(insn, 9);
+ 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);
+
+ /* Non-arithmetic shift clears V. Use it as a source zero here. */
+ tcg_gen_movi_i32(QREG_CC_V, 0);
+
if (insn & 0x100) {
- gen_helper_shl_cc(reg, cpu_env, reg, shift);
+ tcg_gen_extu_i32_i64(t64, reg);
+ tcg_gen_shl_i64(t64, t64, s64);
+ tcg_temp_free_i64(s64);
+ tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
+ tcg_temp_free_i64(t64);
+ tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 1);
} else {
- if (insn & 8) {
- gen_helper_shr_cc(reg, cpu_env, reg, shift);
+ tcg_gen_extu_i32_i64(t64, reg);
+ 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_temp_free_i64(s64);
+ tcg_gen_extr_i64_i32(QREG_CC_C, QREG_CC_N, t64);
+ tcg_temp_free_i64(t64);
+ tcg_gen_shri_i32(QREG_CC_C, QREG_CC_C, 31);
}
+ tcg_gen_mov_i32(QREG_CC_Z, QREG_CC_N);
+
+ /* Note that 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);
+ tcg_temp_free(s32);
+
+ /* Note that ColdFire always clears V (which we have done above),
+ while M68000 sets it for a change in the sign bit. */
+ if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
+ tcg_gen_xor_i32(QREG_CC_V, QREG_CC_N, reg);
+ }
+
+ /* Write back the result. */
+ tcg_gen_mov_i32(reg, QREG_CC_N);
set_cc_op(s, CC_OP_FLAGS);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands
2016-10-27 21:43 [Qemu-devel] [PATCH v2 0/3] 680x0 instruction set, part 2 Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 2/3] target-m68k: Inline shifts Laurent Vivier
@ 2016-10-27 21:43 ` Laurent Vivier
2016-10-28 17:42 ` Richard Henderson
2 siblings, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2016-10-27 21:43 UTC (permalink / raw)
To: qemu-devel; +Cc: schwab, agraf, Richard Henderson, gerg, Laurent Vivier
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
target-m68k/translate.c | 187 ++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 156 insertions(+), 31 deletions(-)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 461d756..c00978d 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2348,19 +2348,40 @@ DISAS_INSN(addx_mem)
gen_store(s, opsize, addr_dest, QREG_CC_N);
}
-DISAS_INSN(shift_im)
+static inline void shift_im(DisasContext *s, uint16_t insn, int opsize)
{
- TCGv reg = DREG(insn, 0);
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;
}
- if (insn & 0x100) {
- tcg_gen_shri_i32(QREG_CC_C, reg, 31 - count);
+ 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,
+ 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();
+ TCGv t1 = tcg_temp_new();
+ /* mask of sign bit */
+ tcg_gen_sari_i32(t0, reg, 31);
+ tcg_gen_sari_i32(t1, reg, bits - count);
+ tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, t0, t1);
+ }
+ tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
+ }
} else {
tcg_gen_shri_i32(QREG_CC_C, reg, count - 1);
if (logical) {
@@ -2369,29 +2390,26 @@ DISAS_INSN(shift_im)
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);
- /* Note that ColdFire always clears V, while M68000 sets it for
- a change in the sign bit. */
- if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
- tcg_gen_xor_i32(QREG_CC_V, QREG_CC_N, reg);
- } else {
- tcg_gen_movi_i32(QREG_CC_V, 0);
- }
-
- tcg_gen_mov_i32(reg, QREG_CC_N);
+ gen_partset_reg(opsize, DREG(insn, 0), QREG_CC_N);
set_cc_op(s, CC_OP_FLAGS);
}
-DISAS_INSN(shift_reg)
+static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
{
- TCGv reg, s32;
- TCGv_i64 t64, s64;
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;
+ TCGv zero;
- reg = DREG(insn, 0);
t64 = tcg_temp_new_i64();
s64 = tcg_temp_new_i64();
s32 = tcg_temp_new();
@@ -2402,44 +2420,144 @@ DISAS_INSN(shift_reg)
tcg_gen_andi_i32(s32, DREG(insn, 9), 63);
tcg_gen_extu_i32_i64(s64, s32);
- /* Non-arithmetic shift clears V. Use it as a source zero here. */
- tcg_gen_movi_i32(QREG_CC_V, 0);
+ zero = tcg_const_i32(0);
+ tcg_gen_mov_i32(QREG_CC_V, zero);
- if (insn & 0x100) {
- tcg_gen_extu_i32_i64(t64, reg);
+ tcg_gen_extu_i32_i64(t64, reg);
+ if (left) {
+ tcg_gen_shli_i64(t64, t64, 32 - bits);
tcg_gen_shl_i64(t64, t64, s64);
tcg_temp_free_i64(s64);
tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
tcg_temp_free_i64(t64);
+ tcg_gen_sari_i32(QREG_CC_N, QREG_CC_N, 32 - bits);
tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 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)) {
+ TCGv t0 = tcg_temp_new();
+ TCGv t1 = tcg_temp_new();
+ TCGv t2 = tcg_const_i32(bits);
+
+ tcg_gen_sub_i32(t2, t2, s32); /* t2 = bits - count */
+
+ tcg_gen_sari_i32(t0, reg, 31);
+ tcg_gen_sar_i32(t1, reg, t2);
+ tcg_temp_free(t2);
+ tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, t0, t1);
+ tcg_temp_free(t1);
+
+ /* if shift count >= bits, V is (reg != 0) */
+ tcg_gen_setcond_i32(TCG_COND_NE, t0, reg, zero);
+ tcg_gen_movcond_i32(TCG_COND_GE, QREG_CC_V, s32, t2, t0, QREG_CC_V);
+
+ tcg_temp_free(t0);
+
+ tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
+
+ /* if shift count is zero, V is 0 */
+ tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_V, s32, zero,
+ QREG_CC_V, zero);
+ }
} else {
- tcg_gen_extu_i32_i64(t64, reg);
- tcg_gen_shli_i64(t64, t64, 32);
+ tcg_gen_shli_i64(t64, t64, 64 - bits);
if (logical) {
+ tcg_gen_shri_i64(t64, t64, 32 - bits);
tcg_gen_shr_i64(t64, t64, s64);
} else {
+ tcg_gen_sari_i64(t64, t64, 32 - bits);
tcg_gen_sar_i64(t64, t64, s64);
}
tcg_temp_free_i64(s64);
tcg_gen_extr_i64_i32(QREG_CC_C, QREG_CC_N, t64);
tcg_temp_free_i64(t64);
+ gen_ext(QREG_CC_N, QREG_CC_N, opsize, 1);
tcg_gen_shri_i32(QREG_CC_C, QREG_CC_C, 31);
}
tcg_gen_mov_i32(QREG_CC_Z, QREG_CC_N);
- /* Note that 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,
+ /* C is cleared if shift count was zero */
+ tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_C, s32, zero,
+ QREG_CC_C, zero);
+
+ /* X = C, but only if the shift count was non-zero. */
+ tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_X, s32, zero,
QREG_CC_C, QREG_CC_X);
+ tcg_temp_free(zero);
tcg_temp_free(s32);
- /* Note that ColdFire always clears V (which we have done above),
- while M68000 sets it for a change in the sign bit. */
- if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) {
- tcg_gen_xor_i32(QREG_CC_V, QREG_CC_N, reg);
+ /* 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)
+{
+ shift_reg(s, insn, OS_LONG);
+}
+
+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 {
+ tcg_gen_mov_i32(QREG_CC_C, src);
+ if (logical) {
+ tcg_gen_shri_i32(QREG_CC_N, src, 1);
+ } else {
+ tcg_gen_sari_i32(QREG_CC_N, src, 1);
+ }
}
- /* Write back the result. */
- tcg_gen_mov_i32(reg, QREG_CC_N);
+ 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);
}
@@ -3508,6 +3626,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands Laurent Vivier
@ 2016-10-28 17:42 ` Richard Henderson
0 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2016-10-28 17:42 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: schwab, agraf, gerg
On 10/27/2016 02:43 PM, Laurent Vivier wrote:
> +static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize)
> {
> - TCGv reg, s32;
> - TCGv_i64 t64, s64;
> 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;
> + TCGv zero;
>
> - reg = DREG(insn, 0);
> t64 = tcg_temp_new_i64();
> s64 = tcg_temp_new_i64();
> s32 = tcg_temp_new();
> @@ -2402,44 +2420,144 @@ DISAS_INSN(shift_reg)
> tcg_gen_andi_i32(s32, DREG(insn, 9), 63);
> tcg_gen_extu_i32_i64(s64, s32);
>
> - /* Non-arithmetic shift clears V. Use it as a source zero here. */
> - tcg_gen_movi_i32(QREG_CC_V, 0);
> + zero = tcg_const_i32(0);
> + tcg_gen_mov_i32(QREG_CC_V, zero);
>
> - if (insn & 0x100) {
> - tcg_gen_extu_i32_i64(t64, reg);
> + tcg_gen_extu_i32_i64(t64, reg);
> + if (left) {
> + tcg_gen_shli_i64(t64, t64, 32 - bits);
> tcg_gen_shl_i64(t64, t64, s64);
> tcg_temp_free_i64(s64);
> tcg_gen_extr_i64_i32(QREG_CC_N, QREG_CC_C, t64);
> tcg_temp_free_i64(t64);
> + tcg_gen_sari_i32(QREG_CC_N, QREG_CC_N, 32 - bits);
> tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 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)) {
> + TCGv t0 = tcg_temp_new();
> + TCGv t1 = tcg_temp_new();
> + TCGv t2 = tcg_const_i32(bits);
> +
> + tcg_gen_sub_i32(t2, t2, s32); /* t2 = bits - count */
> +
> + tcg_gen_sari_i32(t0, reg, 31);
> + tcg_gen_sar_i32(t1, reg, t2);
> + tcg_temp_free(t2);
> + tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, t0, t1);
> + tcg_temp_free(t1);
> +
> + /* if shift count >= bits, V is (reg != 0) */
> + tcg_gen_setcond_i32(TCG_COND_NE, t0, reg, zero);
> + tcg_gen_movcond_i32(TCG_COND_GE, QREG_CC_V, s32, t2, t0, QREG_CC_V);
> +
> + tcg_temp_free(t0);
> +
> + tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);
> +
> + /* if shift count is zero, V is 0 */
> + tcg_gen_movcond_i32(TCG_COND_NE, QREG_CC_V, s32, zero,
> + QREG_CC_V, zero);
This computation is not quite right, in that you compute bits - count and don't
bound that against 0..31 for the sar_i32.
I think we can also to better for the two things you're special-casing: count
>= bits and count == 0. I need to think about that some more.
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm Laurent Vivier
@ 2016-11-01 17:48 ` Richard Henderson
2016-11-01 19:58 ` Laurent Vivier
0 siblings, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2016-11-01 17:48 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: gerg, schwab, agraf
On 10/27/2016 03:43 PM, Laurent Vivier wrote:
> +DISAS_INSN(cmpm)
> +{
> + int opsize = insn_opsize(insn);
> + TCGv tmp = tcg_temp_new();
> + TCGv src, dst, addr;
> +
> + src = gen_load(s, opsize, AREG(insn, 0), 1);
> + /* delay the update after the second gen_load() */
> + tcg_gen_addi_i32(tmp, AREG(insn, 0), opsize_bytes(opsize));
> +
> + /* but if the we use the same address register to
> + * read the second value, we must use the updated address
> + */
> + if (REG(insn, 0) == REG(insn, 9)) {
> + addr = tmp;
> + } else {
> + addr = AREG(insn, 9);
> + }
> +
> + dst = gen_load(s, opsize, addr, 1);
> + tcg_gen_mov_i32(AREG(insn, 0), tmp);
> + tcg_gen_addi_i32(AREG(insn, 9), addr, opsize_bytes(opsize));
I wonder if we should make this more generic.
I'm thinking of transparently fixing
case 3: /* Indirect postincrement. */
reg = AREG(insn, 0);
result = gen_ldst(s, opsize, reg, val, what);
/* ??? This is not exception safe. The instruction may still
fault after this point. */
if (what == EA_STORE || !addrp)
tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize));
return result;
If we add to DisasContext
unsigned writeback_mask;
TCGv writeback[8];
Then
static TCGv get_areg(DisasContext *s, unsigned regno)
{
if (s->writeback_mask & (1 << regno)) {
return s->writeback[regno];
} else {
return cpu_aregs[regno];
}
}
static void delay_set_areg(DisasContext *s, unsigned regno,
TCGv val, bool give_temp)
{
if (s->writeback_mask & (1 << regno)) {
tcg_gen_mov_i32(s->writeback[regno], val);
if (give_temp) {
tcg_temp_free(val);
}
} else {
s->writeback_mask |= 1 << regno;
if (give_temp) {
s->writeback[regno] = val;
} else {
TCGv tmp = tcg_temp_new();
tcg_gen_mov_i32(tmp, val);
s->writeback[regno] = tmp;
}
}
}
static void do_writebacks(DisasContext *s)
{
unsigned mask = s->writeback_mask;
if (mask) {
s->writeback_mask = 0;
do {
unsigned regno = ctz32(mask);
tcg_gen_mov_i32(cpu_aregs[regno], s->writeback[regno]);
tcg_temp_free(s->writeback[regno]);
mask &= mask - 1;
} while (mask);
}
}
where get_areg is used within the AREG macro, delay_set_areg is used in those
cases where pre- and post-increment are needed, and do_writebacks is invoked at
the end of disas_m68k_insn.
This all pre-supposes that cmpm is not a special-case within the ISA, that any
time one reuses a register after modification one sees the new value. Given
the existing code within gen_ea, this would seem to be the case.
Thoughts?
r~
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm
2016-11-01 17:48 ` Richard Henderson
@ 2016-11-01 19:58 ` Laurent Vivier
0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2016-11-01 19:58 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: gerg, schwab, agraf
Le 01/11/2016 à 18:48, Richard Henderson a écrit :
> On 10/27/2016 03:43 PM, Laurent Vivier wrote:
>> +DISAS_INSN(cmpm)
>> +{
>> + int opsize = insn_opsize(insn);
>> + TCGv tmp = tcg_temp_new();
>> + TCGv src, dst, addr;
>> +
>> + src = gen_load(s, opsize, AREG(insn, 0), 1);
>> + /* delay the update after the second gen_load() */
>> + tcg_gen_addi_i32(tmp, AREG(insn, 0), opsize_bytes(opsize));
>> +
>> + /* but if the we use the same address register to
>> + * read the second value, we must use the updated address
>> + */
>> + if (REG(insn, 0) == REG(insn, 9)) {
>> + addr = tmp;
>> + } else {
>> + addr = AREG(insn, 9);
>> + }
>> +
>> + dst = gen_load(s, opsize, addr, 1);
>> + tcg_gen_mov_i32(AREG(insn, 0), tmp);
>> + tcg_gen_addi_i32(AREG(insn, 9), addr, opsize_bytes(opsize));
>
> I wonder if we should make this more generic.
> I'm thinking of transparently fixing
>
> case 3: /* Indirect postincrement. */
> reg = AREG(insn, 0);
> result = gen_ldst(s, opsize, reg, val, what);
> /* ??? This is not exception safe. The instruction may still
> fault after this point. */
> if (what == EA_STORE || !addrp)
> tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize));
> return result;
>
> If we add to DisasContext
>
> unsigned writeback_mask;
> TCGv writeback[8];
>
> Then
>
> static TCGv get_areg(DisasContext *s, unsigned regno)
> {
> if (s->writeback_mask & (1 << regno)) {
> return s->writeback[regno];
> } else {
> return cpu_aregs[regno];
> }
> }
>
> static void delay_set_areg(DisasContext *s, unsigned regno,
> TCGv val, bool give_temp)
> {
> if (s->writeback_mask & (1 << regno)) {
> tcg_gen_mov_i32(s->writeback[regno], val);
> if (give_temp) {
> tcg_temp_free(val);
> }
> } else {
> s->writeback_mask |= 1 << regno;
> if (give_temp) {
> s->writeback[regno] = val;
> } else {
> TCGv tmp = tcg_temp_new();
> tcg_gen_mov_i32(tmp, val);
> s->writeback[regno] = tmp;
> }
> }
> }
>
> static void do_writebacks(DisasContext *s)
> {
> unsigned mask = s->writeback_mask;
> if (mask) {
> s->writeback_mask = 0;
> do {
> unsigned regno = ctz32(mask);
> tcg_gen_mov_i32(cpu_aregs[regno], s->writeback[regno]);
> tcg_temp_free(s->writeback[regno]);
> mask &= mask - 1;
> } while (mask);
> }
> }
>
> where get_areg is used within the AREG macro, delay_set_areg is used in
> those cases where pre- and post-increment are needed, and do_writebacks
> is invoked at the end of disas_m68k_insn.
>
> This all pre-supposes that cmpm is not a special-case within the ISA,
> that any time one reuses a register after modification one sees the new
> value. Given the existing code within gen_ea, this would seem to be the
> case.
>
> Thoughts?
I think it's really a good idea to manage this in a generic way.
As you have already written 90% of the code, do you want to provide a
patch? I can test this with coldfire kernel and 68020 linux-user container.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-11-01 19:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 21:43 [Qemu-devel] [PATCH v2 0/3] 680x0 instruction set, part 2 Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm Laurent Vivier
2016-11-01 17:48 ` Richard Henderson
2016-11-01 19:58 ` Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 2/3] target-m68k: Inline shifts Laurent Vivier
2016-10-27 21:43 ` [Qemu-devel] [PATCH v2 3/3] target-m68k: shift ops manage word and byte operands Laurent Vivier
2016-10-28 17:42 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.