* [Qemu-devel] [PATCH] fix ARMv7 data processing instructions
@ 2009-03-16 12:13 Riihimaki Juha
2009-03-27 11:47 ` Juha Riihimäki
0 siblings, 1 reply; 7+ messages in thread
From: Riihimaki Juha @ 2009-03-16 12:13 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 277 bytes --]
This is a revised version of the "fix ARMv7 MOV R15, xxx operation"
patch I sent here last Friday. This patch should fix the behavior of
all affected commands as far as I know. I put the patch as an
attachment here as well in case the formatting gets abused in transit.
[-- Attachment #2: diff --]
[-- Type: application/octet-stream, Size: 9145 bytes --]
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9e51464..f71162b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -307,7 +307,7 @@ void HELPER(set_user_reg)(uint32_t regno, uint32_t val)
uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
{
uint32_t result;
- result = T0 + T1;
+ result = a + b;
env->NF = env->ZF = result;
env->CF = result < a;
env->VF = (a ^ b ^ -1) & (a ^ result);
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3cef021..02c6258 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -191,6 +191,14 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
dead_tmp(var);
}
+/* Variant of store_reg which uses branch&exchange logic when storing
+ to r15 in ARM architecture v7 and above. The source must be a temporary
+ and will be marked as dead. */
+#define store_reg_bx(dc, reg, var) \
+ if (ENABLE_ARCH_7 && reg == 15) \
+ gen_bx(dc, var); \
+ else \
+ store_reg(dc, reg, var);
/* Basic operations. */
#define gen_op_movl_T0_T1() tcg_gen_mov_i32(cpu_T[0], cpu_T[1])
@@ -436,6 +444,16 @@ static void gen_adc_T0_T1(void)
dead_tmp(tmp);
}
+/* dest = T0 + T1 + CF. */
+static void gen_add_carry(TCGv dest, TCGv t0, TCGv t1)
+{
+ TCGv tmp;
+ tcg_gen_add_i32(dest, t0, t1);
+ tmp = load_cpu_field(CF);
+ tcg_gen_add_i32(dest, dest, tmp);
+ dead_tmp(tmp);
+}
+
/* dest = T0 - T1 + CF - 1. */
static void gen_sub_carry(TCGv dest, TCGv t0, TCGv t1)
{
@@ -3444,11 +3462,11 @@ static int gen_set_psr_T0(DisasContext *s, uint32_t mask, int spsr)
return 0;
}
-/* Generate an old-style exception return. */
-static void gen_exception_return(DisasContext *s)
+/* Generate an old-style exception return. Marks pc as dead. */
+static void gen_exception_return(DisasContext *s, TCGv pc)
{
TCGv tmp;
- gen_movl_reg_T0(s, 15);
+ store_reg(s, 15, pc);
tmp = load_cpu_field(spsr);
gen_set_cpsr(tmp, 0xffffffff);
dead_tmp(tmp);
@@ -6087,146 +6105,151 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
shift = ((insn >> 8) & 0xf) * 2;
if (shift)
val = (val >> shift) | (val << (32 - shift));
- gen_op_movl_T1_im(val);
+ tmp2 = new_tmp();
+ tcg_gen_movi_i32(tmp2, val);
if (logic_cc && shift)
- gen_set_CF_bit31(cpu_T[1]);
+ gen_set_CF_bit31(tmp2);
} else {
/* register */
rm = (insn) & 0xf;
- gen_movl_T1_reg(s, rm);
+ tmp2 = load_reg(s, rm);
shiftop = (insn >> 5) & 3;
if (!(insn & (1 << 4))) {
shift = (insn >> 7) & 0x1f;
- gen_arm_shift_im(cpu_T[1], shiftop, shift, logic_cc);
+ gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
} else {
rs = (insn >> 8) & 0xf;
tmp = load_reg(s, rs);
- gen_arm_shift_reg(cpu_T[1], shiftop, tmp, logic_cc);
+ gen_arm_shift_reg(tmp2, shiftop, tmp, logic_cc);
}
}
if (op1 != 0x0f && op1 != 0x0d) {
rn = (insn >> 16) & 0xf;
- gen_movl_T0_reg(s, rn);
- }
+ gen_movl_T0_reg(s, rn); tmp = load_reg(s, rn);
+ } else
+ tmp = new_tmp();
rd = (insn >> 12) & 0xf;
switch(op1) {
case 0x00:
- gen_op_andl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_and_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x01:
- gen_op_xorl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x02:
if (set_cc && rd == 15) {
/* SUBS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_subl_T0_T1_cc();
- gen_exception_return(s);
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ gen_exception_return(s, tmp);
} else {
if (set_cc)
- gen_op_subl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp, tmp2);
else
- gen_op_subl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
}
break;
case 0x03:
if (set_cc)
- gen_op_rsbl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp2, tmp);
else
- gen_op_rsbl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp2, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x04:
if (set_cc)
- gen_op_addl_T0_T1_cc();
+ gen_helper_add_cc(tmp, tmp, tmp2);
else
- gen_op_addl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_add_i32(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x05:
if (set_cc)
- gen_op_adcl_T0_T1_cc();
+ gen_helper_adc_cc(tmp, tmp, tmp2);
else
- gen_adc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_add_carry(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x06:
if (set_cc)
- gen_op_sbcl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp, tmp2);
else
- gen_sbc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x07:
if (set_cc)
- gen_op_rscl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp2, tmp);
else
- gen_rsc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp2, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x08:
if (set_cc) {
- gen_op_andl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_and_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x09:
if (set_cc) {
- gen_op_xorl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x0a:
- if (set_cc) {
- gen_op_subl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0b:
- if (set_cc) {
- gen_op_addl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_add_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0c:
- gen_op_orl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_or_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x0d:
+ tcg_gen_mov_i32(tmp, tmp2);
if (logic_cc && rd == 15) {
/* MOVS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_movl_T0_T1();
- gen_exception_return(s);
+ gen_exception_return(s, tmp);
} else {
- gen_movl_reg_T1(s, rd);
+ store_reg_bx(s, rd, tmp);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp2);
}
break;
case 0x0e:
- gen_op_bicl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_bic_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
default:
case 0x0f:
- gen_op_notl_T1();
- gen_movl_reg_T1(s, rd);
+ tcg_gen_not_i32(tmp, tmp2);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
}
+ dead_tmp(tmp2);
} else {
/* other instructions */
op1 = (insn >> 24) & 0xf;
[-- Attachment #3: Type: text/plain, Size: 9611 bytes --]
Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
---
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9e51464..f71162b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -307,7 +307,7 @@ void HELPER(set_user_reg)(uint32_t regno, uint32_t
val)
uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
{
uint32_t result;
- result = T0 + T1;
+ result = a + b;
env->NF = env->ZF = result;
env->CF = result < a;
env->VF = (a ^ b ^ -1) & (a ^ result);
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3cef021..02c6258 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -191,6 +191,14 @@ static void store_reg(DisasContext *s, int reg,
TCGv var)
dead_tmp(var);
}
+/* Variant of store_reg which uses branch&exchange logic when storing
+ to r15 in ARM architecture v7 and above. The source must be a
temporary
+ and will be marked as dead. */
+#define store_reg_bx(dc, reg, var) \
+ if (ENABLE_ARCH_7 && reg == 15) \
+ gen_bx(dc, var); \
+ else \
+ store_reg(dc, reg, var);
/* Basic operations. */
#define gen_op_movl_T0_T1() tcg_gen_mov_i32(cpu_T[0], cpu_T[1])
@@ -436,6 +444,16 @@ static void gen_adc_T0_T1(void)
dead_tmp(tmp);
}
+/* dest = T0 + T1 + CF. */
+static void gen_add_carry(TCGv dest, TCGv t0, TCGv t1)
+{
+ TCGv tmp;
+ tcg_gen_add_i32(dest, t0, t1);
+ tmp = load_cpu_field(CF);
+ tcg_gen_add_i32(dest, dest, tmp);
+ dead_tmp(tmp);
+}
+
/* dest = T0 - T1 + CF - 1. */
static void gen_sub_carry(TCGv dest, TCGv t0, TCGv t1)
{
@@ -3444,11 +3462,11 @@ static int gen_set_psr_T0(DisasContext *s,
uint32_t mask, int spsr)
return 0;
}
-/* Generate an old-style exception return. */
-static void gen_exception_return(DisasContext *s)
+/* Generate an old-style exception return. Marks pc as dead. */
+static void gen_exception_return(DisasContext *s, TCGv pc)
{
TCGv tmp;
- gen_movl_reg_T0(s, 15);
+ store_reg(s, 15, pc);
tmp = load_cpu_field(spsr);
gen_set_cpsr(tmp, 0xffffffff);
dead_tmp(tmp);
@@ -6087,146 +6105,151 @@ static void disas_arm_insn(CPUState * env,
DisasContext *s)
shift = ((insn >> 8) & 0xf) * 2;
if (shift)
val = (val >> shift) | (val << (32 - shift));
- gen_op_movl_T1_im(val);
+ tmp2 = new_tmp();
+ tcg_gen_movi_i32(tmp2, val);
if (logic_cc && shift)
- gen_set_CF_bit31(cpu_T[1]);
+ gen_set_CF_bit31(tmp2);
} else {
/* register */
rm = (insn) & 0xf;
- gen_movl_T1_reg(s, rm);
+ tmp2 = load_reg(s, rm);
shiftop = (insn >> 5) & 3;
if (!(insn & (1 << 4))) {
shift = (insn >> 7) & 0x1f;
- gen_arm_shift_im(cpu_T[1], shiftop, shift, logic_cc);
+ gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
} else {
rs = (insn >> 8) & 0xf;
tmp = load_reg(s, rs);
- gen_arm_shift_reg(cpu_T[1], shiftop, tmp, logic_cc);
+ gen_arm_shift_reg(tmp2, shiftop, tmp, logic_cc);
}
}
if (op1 != 0x0f && op1 != 0x0d) {
rn = (insn >> 16) & 0xf;
- gen_movl_T0_reg(s, rn);
- }
+ gen_movl_T0_reg(s, rn); tmp = load_reg(s, rn);
+ } else
+ tmp = new_tmp();
rd = (insn >> 12) & 0xf;
switch(op1) {
case 0x00:
- gen_op_andl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_and_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x01:
- gen_op_xorl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x02:
if (set_cc && rd == 15) {
/* SUBS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_subl_T0_T1_cc();
- gen_exception_return(s);
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ gen_exception_return(s, tmp);
} else {
if (set_cc)
- gen_op_subl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp, tmp2);
else
- gen_op_subl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
}
break;
case 0x03:
if (set_cc)
- gen_op_rsbl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp2, tmp);
else
- gen_op_rsbl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp2, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x04:
if (set_cc)
- gen_op_addl_T0_T1_cc();
+ gen_helper_add_cc(tmp, tmp, tmp2);
else
- gen_op_addl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_add_i32(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x05:
if (set_cc)
- gen_op_adcl_T0_T1_cc();
+ gen_helper_adc_cc(tmp, tmp, tmp2);
else
- gen_adc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_add_carry(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x06:
if (set_cc)
- gen_op_sbcl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp, tmp2);
else
- gen_sbc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x07:
if (set_cc)
- gen_op_rscl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp2, tmp);
else
- gen_rsc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp2, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x08:
if (set_cc) {
- gen_op_andl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_and_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x09:
if (set_cc) {
- gen_op_xorl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x0a:
- if (set_cc) {
- gen_op_subl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0b:
- if (set_cc) {
- gen_op_addl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_add_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0c:
- gen_op_orl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_or_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x0d:
+ tcg_gen_mov_i32(tmp, tmp2);
if (logic_cc && rd == 15) {
/* MOVS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_movl_T0_T1();
- gen_exception_return(s);
+ gen_exception_return(s, tmp);
} else {
- gen_movl_reg_T1(s, rd);
+ store_reg_bx(s, rd, tmp);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp2);
}
break;
case 0x0e:
- gen_op_bicl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_bic_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
default:
case 0x0f:
- gen_op_notl_T1();
- gen_movl_reg_T1(s, rd);
+ tcg_gen_not_i32(tmp, tmp2);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
}
+ dead_tmp(tmp2);
} else {
/* other instructions */
op1 = (insn >> 24) & 0xf;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] fix ARMv7 data processing instructions
2009-03-16 12:13 [Qemu-devel] [PATCH] fix ARMv7 data processing instructions Riihimaki Juha
@ 2009-03-27 11:47 ` Juha Riihimäki
2009-03-27 12:43 ` Laurent Desnogues
2009-03-27 13:53 ` Paul Brook
0 siblings, 2 replies; 7+ messages in thread
From: Juha Riihimäki @ 2009-03-27 11:47 UTC (permalink / raw)
To: qemu-devel@nongnu.org
[-- Attachment #1: Type: text/plain, Size: 774 bytes --]
Any further comments? Or perhaps import the patch? I made a small
performance optimization in the new store_reg_bx macro by performing
the simpler comparison first and the the little bit more expensive
comparison only if the first comparison is true. At least I have
encountered no problems in OMAP2 and OMAP3 emulation so far; instead
the guest code which doesn't run without the patch in OMAP3 now works.
Juha
On Mar 16, 2009, at 14:13, Riihimaki Juha (Nokia-D/Helsinki) wrote:
> This is a revised version of the "fix ARMv7 MOV R15, xxx operation"
> patch I sent here last Friday. This patch should fix the behavior of
> all affected commands as far as I know. I put the patch as an
> attachment here as well in case the formatting gets abused in transit.
[-- Attachment #2: diff --]
[-- Type: application/octet-stream, Size: 9157 bytes --]
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9e51464..f71162b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -307,7 +307,7 @@ void HELPER(set_user_reg)(uint32_t regno, uint32_t val)
uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
{
uint32_t result;
- result = T0 + T1;
+ result = a + b;
env->NF = env->ZF = result;
env->CF = result < a;
env->VF = (a ^ b ^ -1) & (a ^ result);
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f7f2a8d..f14e612 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -191,6 +191,14 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
dead_tmp(var);
}
+/* Variant of store_reg which uses branch&exchange logic when storing
+ to r15 in ARM architecture v7 and above. The source must be a temporary
+ and will be marked as dead. */
+#define store_reg_bx(dc, reg, var) \
+ if ((reg) == 15 && ENABLE_ARCH_7) \
+ gen_bx((dc), (var)); \
+ else \
+ store_reg((dc), (reg), (var));
/* Basic operations. */
#define gen_op_movl_T0_T1() tcg_gen_mov_i32(cpu_T[0], cpu_T[1])
@@ -436,6 +444,16 @@ static void gen_adc_T0_T1(void)
dead_tmp(tmp);
}
+/* dest = T0 + T1 + CF. */
+static void gen_add_carry(TCGv dest, TCGv t0, TCGv t1)
+{
+ TCGv tmp;
+ tcg_gen_add_i32(dest, t0, t1);
+ tmp = load_cpu_field(CF);
+ tcg_gen_add_i32(dest, dest, tmp);
+ dead_tmp(tmp);
+}
+
/* dest = T0 - T1 + CF - 1. */
static void gen_sub_carry(TCGv dest, TCGv t0, TCGv t1)
{
@@ -3446,11 +3464,11 @@ static int gen_set_psr_T0(DisasContext *s, uint32_t mask, int spsr)
return 0;
}
-/* Generate an old-style exception return. */
-static void gen_exception_return(DisasContext *s)
+/* Generate an old-style exception return. Marks pc as dead. */
+static void gen_exception_return(DisasContext *s, TCGv pc)
{
TCGv tmp;
- gen_movl_reg_T0(s, 15);
+ store_reg(s, 15, pc);
tmp = load_cpu_field(spsr);
gen_set_cpsr(tmp, 0xffffffff);
dead_tmp(tmp);
@@ -6089,146 +6107,151 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
shift = ((insn >> 8) & 0xf) * 2;
if (shift)
val = (val >> shift) | (val << (32 - shift));
- gen_op_movl_T1_im(val);
+ tmp2 = new_tmp();
+ tcg_gen_movi_i32(tmp2, val);
if (logic_cc && shift)
- gen_set_CF_bit31(cpu_T[1]);
+ gen_set_CF_bit31(tmp2);
} else {
/* register */
rm = (insn) & 0xf;
- gen_movl_T1_reg(s, rm);
+ tmp2 = load_reg(s, rm);
shiftop = (insn >> 5) & 3;
if (!(insn & (1 << 4))) {
shift = (insn >> 7) & 0x1f;
- gen_arm_shift_im(cpu_T[1], shiftop, shift, logic_cc);
+ gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
} else {
rs = (insn >> 8) & 0xf;
tmp = load_reg(s, rs);
- gen_arm_shift_reg(cpu_T[1], shiftop, tmp, logic_cc);
+ gen_arm_shift_reg(tmp2, shiftop, tmp, logic_cc);
}
}
if (op1 != 0x0f && op1 != 0x0d) {
rn = (insn >> 16) & 0xf;
- gen_movl_T0_reg(s, rn);
- }
+ gen_movl_T0_reg(s, rn); tmp = load_reg(s, rn);
+ } else
+ tmp = new_tmp();
rd = (insn >> 12) & 0xf;
switch(op1) {
case 0x00:
- gen_op_andl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_and_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x01:
- gen_op_xorl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x02:
if (set_cc && rd == 15) {
/* SUBS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_subl_T0_T1_cc();
- gen_exception_return(s);
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ gen_exception_return(s, tmp);
} else {
if (set_cc)
- gen_op_subl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp, tmp2);
else
- gen_op_subl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
}
break;
case 0x03:
if (set_cc)
- gen_op_rsbl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp2, tmp);
else
- gen_op_rsbl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp2, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x04:
if (set_cc)
- gen_op_addl_T0_T1_cc();
+ gen_helper_add_cc(tmp, tmp, tmp2);
else
- gen_op_addl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_add_i32(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x05:
if (set_cc)
- gen_op_adcl_T0_T1_cc();
+ gen_helper_adc_cc(tmp, tmp, tmp2);
else
- gen_adc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_add_carry(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x06:
if (set_cc)
- gen_op_sbcl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp, tmp2);
else
- gen_sbc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp, tmp2);
+ store_reg_bx(s, rd, tmp);
break;
case 0x07:
if (set_cc)
- gen_op_rscl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp2, tmp);
else
- gen_rsc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp2, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x08:
if (set_cc) {
- gen_op_andl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_and_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x09:
if (set_cc) {
- gen_op_xorl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x0a:
- if (set_cc) {
- gen_op_subl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0b:
- if (set_cc) {
- gen_op_addl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_add_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0c:
- gen_op_orl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_or_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x0d:
+ tcg_gen_mov_i32(tmp, tmp2);
if (logic_cc && rd == 15) {
/* MOVS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_movl_T0_T1();
- gen_exception_return(s);
+ gen_exception_return(s, tmp);
} else {
- gen_movl_reg_T1(s, rd);
+ store_reg_bx(s, rd, tmp);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp2);
}
break;
case 0x0e:
- gen_op_bicl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_bic_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
default:
case 0x0f:
- gen_op_notl_T1();
- gen_movl_reg_T1(s, rd);
+ tcg_gen_not_i32(tmp, tmp2);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp);
+ store_reg_bx(s, rd, tmp);
break;
}
+ dead_tmp(tmp2);
} else {
/* other instructions */
op1 = (insn >> 24) & 0xf;
[-- Attachment #3: Type: text/plain, Size: 1 bytes --]
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] fix ARMv7 data processing instructions
2009-03-27 11:47 ` Juha Riihimäki
@ 2009-03-27 12:43 ` Laurent Desnogues
2009-03-27 13:17 ` Juha Riihimäki
2009-03-27 13:53 ` Paul Brook
1 sibling, 1 reply; 7+ messages in thread
From: Laurent Desnogues @ 2009-03-27 12:43 UTC (permalink / raw)
To: qemu-devel
On Fri, Mar 27, 2009 at 12:47 PM, Juha Riihimäki
<juha.riihimaki@nokia.com> wrote:
> Any further comments? Or perhaps import the patch? I made a small
> performance optimization in the new store_reg_bx macro by performing the
> simpler comparison first and the the little bit more expensive comparison
> only if the first comparison is true. At least I have encountered no
> problems in OMAP2 and OMAP3 emulation so far; instead the guest code which
> doesn't run without the patch in OMAP3 now works.
I didn't take a close look at your patch, so I only have a cosmetic comment :)
Wouldn't it be better to use a static inline function instead of a macro for
store_reg_bx? At least I find it better looking and should not degrade
performance.
Laurent
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] fix ARMv7 data processing instructions
2009-03-27 12:43 ` Laurent Desnogues
@ 2009-03-27 13:17 ` Juha Riihimäki
0 siblings, 0 replies; 7+ messages in thread
From: Juha Riihimäki @ 2009-03-27 13:17 UTC (permalink / raw)
To: qemu-devel@nongnu.org
On Mar 27, 2009, at 14:43, ext Laurent Desnogues wrote:
> On Fri, Mar 27, 2009 at 12:47 PM, Juha Riihimäki
> <juha.riihimaki@nokia.com> wrote:
>> Any further comments? Or perhaps import the patch? I made a small
>> performance optimization in the new store_reg_bx macro by
>> performing the
>> simpler comparison first and the the little bit more expensive
>> comparison
>> only if the first comparison is true. At least I have encountered no
>> problems in OMAP2 and OMAP3 emulation so far; instead the guest
>> code which
>> doesn't run without the patch in OMAP3 now works.
>
> I didn't take a close look at your patch, so I only have a cosmetic
> comment :)
> Wouldn't it be better to use a static inline function instead of a
> macro for
> store_reg_bx? At least I find it better looking and should not
> degrade
> performance.
The reason I used a macro was because the ENABLE_ARCH_7 macro would
not work unless one extra parameter is added to the function to pass
the "env" pointer. But that is also possible if you think it is better.
Juha
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] fix ARMv7 data processing instructions
2009-03-27 11:47 ` Juha Riihimäki
2009-03-27 12:43 ` Laurent Desnogues
@ 2009-03-27 13:53 ` Paul Brook
2009-03-27 18:24 ` Juha Riihimäki
1 sibling, 1 reply; 7+ messages in thread
From: Paul Brook @ 2009-03-27 13:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Juha Riihimäki
On Friday 27 March 2009, Juha Riihimäki wrote:
> Any further comments? Or perhaps import the patch?
It looks like you've got several changes all mixed together, including
bugfixes, cleanups and new features. Please divide into logically separate
patches.
Paul
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] fix ARMv7 data processing instructions
2009-03-27 13:53 ` Paul Brook
@ 2009-03-27 18:24 ` Juha Riihimäki
2009-05-06 6:15 ` [Qemu-devel] [PATCH 0/2] " Juha Riihimäki
0 siblings, 1 reply; 7+ messages in thread
From: Juha Riihimäki @ 2009-03-27 18:24 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]
On Mar 27, 2009, at 15:53, ext Paul Brook wrote:
> On Friday 27 March 2009, Juha Riihimäki wrote:
>> Any further comments? Or perhaps import the patch?
>
> It looks like you've got several changes all mixed together, including
> bugfixes, cleanups and new features. Please divide into logically
> separate
> patches.
Yes, there are three logical parts, I have separated them here and
included as separate attachments that can be applied separately in the
numerical order:
i) A bug fix patch (diff1) which makes the add_cc helper function use
its parameters instead of the temporary variables. This is required
for the other patches to work properly. This bug was also reported by
Torbjörn Andersson on this list couple of days ago.
ii) A patch (diff2) to get rid of global temporary variable usage
cpu_T[x] in the ARM data processing instructions. I suppose this is
valid since the cpu_T[x] variables are tagged with a "FIXME: These
should be removed" comment in the beginning of the file. In order to
accomplish this I introduced a new function, gen_add_carry, to replace
the existing gen_adc_T0_T1 usage. I also modified the behavior of an
existing function, gen_exception_return, which was only used within
the ARM data processing instructions handling so the change had no
effect on other code.
iii) A patch (diff3) to add support for the ARMv7 behavior in the ARM
data processing instructions when destination register is R15.
Is this better?
Juha
[-- Attachment #2: diff1 --]
[-- Type: application/octet-stream, Size: 448 bytes --]
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9e51464..f71162b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -307,7 +307,7 @@ void HELPER(set_user_reg)(uint32_t regno, uint32_t val)
uint32_t HELPER (add_cc)(uint32_t a, uint32_t b)
{
uint32_t result;
- result = T0 + T1;
+ result = a + b;
env->NF = env->ZF = result;
env->CF = result < a;
env->VF = (a ^ b ^ -1) & (a ^ result);
[-- Attachment #3: diff2 --]
[-- Type: application/octet-stream, Size: 8107 bytes --]
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f7f2a8d..030b544 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -436,6 +436,16 @@ static void gen_adc_T0_T1(void)
dead_tmp(tmp);
}
+/* dest = T0 + T1 + CF. */
+static void gen_add_carry(TCGv dest, TCGv t0, TCGv t1)
+{
+ TCGv tmp;
+ tcg_gen_add_i32(dest, t0, t1);
+ tmp = load_cpu_field(CF);
+ tcg_gen_add_i32(dest, dest, tmp);
+ dead_tmp(tmp);
+}
+
/* dest = T0 - T1 + CF - 1. */
static void gen_sub_carry(TCGv dest, TCGv t0, TCGv t1)
{
@@ -3446,11 +3456,11 @@ static int gen_set_psr_T0(DisasContext *s, uint32_t mask, int spsr)
return 0;
}
-/* Generate an old-style exception return. */
-static void gen_exception_return(DisasContext *s)
+/* Generate an old-style exception return. Marks pc as dead. */
+static void gen_exception_return(DisasContext *s, TCGv pc)
{
TCGv tmp;
- gen_movl_reg_T0(s, 15);
+ store_reg(s, 15, pc);
tmp = load_cpu_field(spsr);
gen_set_cpsr(tmp, 0xffffffff);
dead_tmp(tmp);
@@ -6089,146 +6099,151 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
shift = ((insn >> 8) & 0xf) * 2;
if (shift)
val = (val >> shift) | (val << (32 - shift));
- gen_op_movl_T1_im(val);
+ tmp2 = new_tmp();
+ tcg_gen_movi_i32(tmp2, val);
if (logic_cc && shift)
- gen_set_CF_bit31(cpu_T[1]);
+ gen_set_CF_bit31(tmp2);
} else {
/* register */
rm = (insn) & 0xf;
- gen_movl_T1_reg(s, rm);
+ tmp2 = load_reg(s, rm);
shiftop = (insn >> 5) & 3;
if (!(insn & (1 << 4))) {
shift = (insn >> 7) & 0x1f;
- gen_arm_shift_im(cpu_T[1], shiftop, shift, logic_cc);
+ gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
} else {
rs = (insn >> 8) & 0xf;
tmp = load_reg(s, rs);
- gen_arm_shift_reg(cpu_T[1], shiftop, tmp, logic_cc);
+ gen_arm_shift_reg(tmp2, shiftop, tmp, logic_cc);
}
}
if (op1 != 0x0f && op1 != 0x0d) {
rn = (insn >> 16) & 0xf;
- gen_movl_T0_reg(s, rn);
- }
+ tmp = load_reg(s, rn);
+ } else
+ tmp = new_tmp();
rd = (insn >> 12) & 0xf;
switch(op1) {
case 0x00:
- gen_op_andl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_and_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg(s, rd, tmp);
break;
case 0x01:
- gen_op_xorl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg(s, rd, tmp);
break;
case 0x02:
if (set_cc && rd == 15) {
/* SUBS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_subl_T0_T1_cc();
- gen_exception_return(s);
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ gen_exception_return(s, tmp);
} else {
if (set_cc)
- gen_op_subl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp, tmp2);
else
- gen_op_subl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp, tmp2);
+ store_reg(s, rd, tmp);
}
break;
case 0x03:
if (set_cc)
- gen_op_rsbl_T0_T1_cc();
+ gen_helper_sub_cc(tmp, tmp2, tmp);
else
- gen_op_rsbl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_sub_i32(tmp, tmp2, tmp);
+ store_reg(s, rd, tmp);
break;
case 0x04:
if (set_cc)
- gen_op_addl_T0_T1_cc();
+ gen_helper_add_cc(tmp, tmp, tmp2);
else
- gen_op_addl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_add_i32(tmp, tmp, tmp2);
+ store_reg(s, rd, tmp);
break;
case 0x05:
if (set_cc)
- gen_op_adcl_T0_T1_cc();
+ gen_helper_adc_cc(tmp, tmp, tmp2);
else
- gen_adc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_add_carry(tmp, tmp, tmp2);
+ store_reg(s, rd, tmp);
break;
case 0x06:
if (set_cc)
- gen_op_sbcl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp, tmp2);
else
- gen_sbc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp, tmp2);
+ store_reg(s, rd, tmp);
break;
case 0x07:
if (set_cc)
- gen_op_rscl_T0_T1_cc();
+ gen_helper_sbc_cc(tmp, tmp2, tmp);
else
- gen_rsc_T0_T1();
- gen_movl_reg_T0(s, rd);
+ gen_sub_carry(tmp, tmp2, tmp);
+ store_reg(s, rd, tmp);
break;
case 0x08:
if (set_cc) {
- gen_op_andl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_and_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x09:
if (set_cc) {
- gen_op_xorl_T0_T1();
- gen_op_logic_T0_cc();
+ tcg_gen_xor_i32(tmp, tmp, tmp2);
+ gen_logic_CC(tmp);
}
+ dead_tmp(tmp);
break;
case 0x0a:
- if (set_cc) {
- gen_op_subl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_sub_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0b:
- if (set_cc) {
- gen_op_addl_T0_T1_cc();
- }
+ if (set_cc)
+ gen_helper_add_cc(tmp, tmp, tmp2);
+ dead_tmp(tmp);
break;
case 0x0c:
- gen_op_orl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_or_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg(s, rd, tmp);
break;
case 0x0d:
+ tcg_gen_mov_i32(tmp, tmp2);
if (logic_cc && rd == 15) {
/* MOVS r15, ... is used for exception return. */
if (IS_USER(s))
goto illegal_op;
- gen_op_movl_T0_T1();
- gen_exception_return(s);
+ gen_exception_return(s, tmp);
} else {
- gen_movl_reg_T1(s, rd);
+ store_reg(s, rd, tmp);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp2);
}
break;
case 0x0e:
- gen_op_bicl_T0_T1();
- gen_movl_reg_T0(s, rd);
+ tcg_gen_bic_i32(tmp, tmp, tmp2);
if (logic_cc)
- gen_op_logic_T0_cc();
+ gen_logic_CC(tmp);
+ store_reg(s, rd, tmp);
break;
default:
case 0x0f:
- gen_op_notl_T1();
- gen_movl_reg_T1(s, rd);
+ tcg_gen_not_i32(tmp, tmp2);
if (logic_cc)
- gen_op_logic_T1_cc();
+ gen_logic_CC(tmp);
+ store_reg(s, rd, tmp);
break;
}
+ dead_tmp(tmp2);
} else {
/* other instructions */
op1 = (insn >> 24) & 0xf;
[-- Attachment #4: diff3 --]
[-- Type: application/octet-stream, Size: 4260 bytes --]
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 030b544..8e5f5bb 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -191,6 +191,14 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
dead_tmp(var);
}
+/* Variant of store_reg which uses branch&exchange logic when storing
+ to r15 in ARM architecture v7 and above. The source must be a temporary
+ and will be marked as dead. */
+#define store_reg_bx(dc, reg, var) \
+ if ((reg) == 15 && ENABLE_ARCH_7) \
+ gen_bx((dc), (var)); \
+ else \
+ store_reg((dc), (reg), (var));
/* Basic operations. */
#define gen_op_movl_T0_T1() tcg_gen_mov_i32(cpu_T[0], cpu_T[1])
@@ -6128,13 +6136,13 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
tcg_gen_and_i32(tmp, tmp, tmp2);
if (logic_cc)
gen_logic_CC(tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x01:
tcg_gen_xor_i32(tmp, tmp, tmp2);
if (logic_cc)
gen_logic_CC(tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x02:
if (set_cc && rd == 15) {
@@ -6148,7 +6156,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
gen_helper_sub_cc(tmp, tmp, tmp2);
else
tcg_gen_sub_i32(tmp, tmp, tmp2);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
}
break;
case 0x03:
@@ -6156,35 +6164,35 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
gen_helper_sub_cc(tmp, tmp2, tmp);
else
tcg_gen_sub_i32(tmp, tmp2, tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x04:
if (set_cc)
gen_helper_add_cc(tmp, tmp, tmp2);
else
tcg_gen_add_i32(tmp, tmp, tmp2);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x05:
if (set_cc)
gen_helper_adc_cc(tmp, tmp, tmp2);
else
gen_add_carry(tmp, tmp, tmp2);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x06:
if (set_cc)
gen_helper_sbc_cc(tmp, tmp, tmp2);
else
gen_sub_carry(tmp, tmp, tmp2);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x07:
if (set_cc)
gen_helper_sbc_cc(tmp, tmp2, tmp);
else
gen_sub_carry(tmp, tmp2, tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x08:
if (set_cc) {
@@ -6214,7 +6222,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
tcg_gen_or_i32(tmp, tmp, tmp2);
if (logic_cc)
gen_logic_CC(tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x0d:
tcg_gen_mov_i32(tmp, tmp2);
@@ -6224,7 +6232,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
goto illegal_op;
gen_exception_return(s, tmp);
} else {
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
if (logic_cc)
gen_logic_CC(tmp2);
}
@@ -6233,14 +6241,14 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
tcg_gen_bic_i32(tmp, tmp, tmp2);
if (logic_cc)
gen_logic_CC(tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
default:
case 0x0f:
tcg_gen_not_i32(tmp, tmp2);
if (logic_cc)
gen_logic_CC(tmp);
- store_reg(s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
}
dead_tmp(tmp2);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 0/2] fix ARMv7 data processing instructions
2009-03-27 18:24 ` Juha Riihimäki
@ 2009-05-06 6:15 ` Juha Riihimäki
0 siblings, 0 replies; 7+ messages in thread
From: Juha Riihimäki @ 2009-05-06 6:15 UTC (permalink / raw)
To: qemu-devel
Time passes... if there are no further comments I'd very much like to
see this patch set integrated to avoid having to maintain delta code
for an obvious defect fix. Since the helper function was already
corrected in svn commit 7034, only the other two patches are relevant
today. In order to avoid writing new code that uses deprecated methods
and/or variables, this fix has been divided into two parts: the first
patch modernizes relevant parts of target-arm/translate.c and the
second patch provides the actual defect fix and should be applied
after the first patch. For this version of the patch set I have also
converted the new macro store_reg_bx into a static function as
suggested by Laurent Desnogues earlier.
Juha
On Mar 27, 2009, at 20:24, Juha Riihimäki wrote:
>
> On Mar 27, 2009, at 15:53, ext Paul Brook wrote:
>
>> On Friday 27 March 2009, Juha Riihimäki wrote:
>>> Any further comments? Or perhaps import the patch?
>>
>> It looks like you've got several changes all mixed together,
>> including
>> bugfixes, cleanups and new features. Please divide into logically
>> separate
>> patches.
>
> Yes, there are three logical parts, I have separated them here and
> included as separate attachments that can be applied separately in the
> numerical order:
>
> i) A bug fix patch (diff1) which makes the add_cc helper function use
> its parameters instead of the temporary variables. This is required
> for the other patches to work properly. This bug was also reported by
> Torbjörn Andersson on this list couple of days ago.
>
> ii) A patch (diff2) to get rid of global temporary variable usage
> cpu_T[x] in the ARM data processing instructions. I suppose this is
> valid since the cpu_T[x] variables are tagged with a "FIXME: These
> should be removed" comment in the beginning of the file. In order to
> accomplish this I introduced a new function, gen_add_carry, to replace
> the existing gen_adc_T0_T1 usage. I also modified the behavior of an
> existing function, gen_exception_return, which was only used within
> the ARM data processing instructions handling so the change had no
> effect on other code.
>
> iii) A patch (diff3) to add support for the ARMv7 behavior in the ARM
> data processing instructions when destination register is R15.
>
> Is this better?
>
>
> Juha
>
> <diff1><diff2><diff3>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-05-06 6:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-16 12:13 [Qemu-devel] [PATCH] fix ARMv7 data processing instructions Riihimaki Juha
2009-03-27 11:47 ` Juha Riihimäki
2009-03-27 12:43 ` Laurent Desnogues
2009-03-27 13:17 ` Juha Riihimäki
2009-03-27 13:53 ` Paul Brook
2009-03-27 18:24 ` Juha Riihimäki
2009-05-06 6:15 ` [Qemu-devel] [PATCH 0/2] " Juha Riihimäki
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.