* [Qemu-devel] [PATCH] target-xtensa: make sar_m32 global instead of local temp
@ 2012-11-24 0:51 Max Filippov
2012-11-24 10:59 ` Aurelien Jarno
0 siblings, 1 reply; 3+ messages in thread
From: Max Filippov @ 2012-11-24 0:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, Max Filippov, Aurelien Jarno
This fixes the following assertion caused by local temp reaching the end
of TB in discarded state:
qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
Aborted
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
target-xtensa/cpu.h | 1 +
target-xtensa/translate.c | 28 ++++++++--------------------
2 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 74e9888..f021a9a 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -323,6 +323,7 @@ typedef struct CPUXtensaState {
const XtensaConfig *config;
uint32_t regs[16];
uint32_t pc;
+ uint32_t sar_m32;
uint32_t sregs[256];
uint32_t uregs[256];
uint32_t phys_regs[MAX_NAREG];
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index e5a3f49..4f3cf32 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -56,8 +56,6 @@ typedef struct DisasContext {
bool sar_5bit;
bool sar_m32_5bit;
- bool sar_m32_allocated;
- TCGv_i32 sar_m32;
uint32_t ccount_delta;
unsigned used_window;
@@ -71,6 +69,7 @@ typedef struct DisasContext {
static TCGv_ptr cpu_env;
static TCGv_i32 cpu_pc;
+static TCGv_i32 cpu_sar_m32;
static TCGv_i32 cpu_R[16];
static TCGv_i32 cpu_FR[16];
static TCGv_i32 cpu_SR[256];
@@ -169,6 +168,8 @@ void xtensa_translate_init(void)
cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
cpu_pc = tcg_global_mem_new_i32(TCG_AREG0,
offsetof(CPUXtensaState, pc), "pc");
+ cpu_sar_m32 = tcg_global_mem_new_i32(TCG_AREG0,
+ offsetof(CPUXtensaState, sar_m32), "sar_m32");
for (i = 0; i < 16; i++) {
cpu_R[i] = tcg_global_mem_new_i32(TCG_AREG0,
@@ -230,21 +231,13 @@ static void init_sar_tracker(DisasContext *dc)
{
dc->sar_5bit = false;
dc->sar_m32_5bit = false;
- dc->sar_m32_allocated = false;
-}
-
-static void reset_sar_tracker(DisasContext *dc)
-{
- if (dc->sar_m32_allocated) {
- tcg_temp_free(dc->sar_m32);
- }
}
static void gen_right_shift_sar(DisasContext *dc, TCGv_i32 sa)
{
tcg_gen_andi_i32(cpu_SR[SAR], sa, 0x1f);
if (dc->sar_m32_5bit) {
- tcg_gen_discard_i32(dc->sar_m32);
+ tcg_gen_discard_i32(cpu_sar_m32);
}
dc->sar_5bit = true;
dc->sar_m32_5bit = false;
@@ -253,12 +246,8 @@ static void gen_right_shift_sar(DisasContext *dc, TCGv_i32 sa)
static void gen_left_shift_sar(DisasContext *dc, TCGv_i32 sa)
{
TCGv_i32 tmp = tcg_const_i32(32);
- if (!dc->sar_m32_allocated) {
- dc->sar_m32 = tcg_temp_local_new_i32();
- dc->sar_m32_allocated = true;
- }
- tcg_gen_andi_i32(dc->sar_m32, sa, 0x1f);
- tcg_gen_sub_i32(cpu_SR[SAR], tmp, dc->sar_m32);
+ tcg_gen_andi_i32(cpu_sar_m32, sa, 0x1f);
+ tcg_gen_sub_i32(cpu_SR[SAR], tmp, cpu_sar_m32);
dc->sar_5bit = false;
dc->sar_m32_5bit = true;
tcg_temp_free(tmp);
@@ -498,7 +487,7 @@ static void gen_wsr_sar(DisasContext *dc, uint32_t sr, TCGv_i32 s)
{
tcg_gen_andi_i32(cpu_SR[sr], s, 0x3f);
if (dc->sar_m32_5bit) {
- tcg_gen_discard_i32(dc->sar_m32);
+ tcg_gen_discard_i32(cpu_sar_m32);
}
dc->sar_5bit = false;
dc->sar_m32_5bit = false;
@@ -1483,7 +1472,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc)
case 10: /*SLL*/
gen_window_check2(dc, RRR_R, RRR_S);
if (dc->sar_m32_5bit) {
- tcg_gen_shl_i32(cpu_R[RRR_R], cpu_R[RRR_S], dc->sar_m32);
+ tcg_gen_shl_i32(cpu_R[RRR_R], cpu_R[RRR_S], cpu_sar_m32);
} else {
TCGv_i64 v = tcg_temp_new_i64();
TCGv_i32 s = tcg_const_i32(32);
@@ -2947,7 +2936,6 @@ static void gen_intermediate_code_internal(
tcg_ctx.gen_opc_ptr < gen_opc_end);
reset_litbase(&dc);
- reset_sar_tracker(&dc);
if (dc.icount) {
tcg_temp_free(dc.next_icount);
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] target-xtensa: make sar_m32 global instead of local temp
2012-11-24 0:51 [Qemu-devel] [PATCH] target-xtensa: make sar_m32 global instead of local temp Max Filippov
@ 2012-11-24 10:59 ` Aurelien Jarno
2012-11-24 11:27 ` Max Filippov
0 siblings, 1 reply; 3+ messages in thread
From: Aurelien Jarno @ 2012-11-24 10:59 UTC (permalink / raw)
To: Max Filippov; +Cc: Blue Swirl, qemu-devel
On Sat, Nov 24, 2012 at 04:51:36AM +0400, Max Filippov wrote:
> This fixes the following assertion caused by local temp reaching the end
> of TB in discarded state:
>
> qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
> Aborted
>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
> target-xtensa/cpu.h | 1 +
> target-xtensa/translate.c | 28 ++++++++--------------------
> 2 files changed, 9 insertions(+), 20 deletions(-)
I have just send a patch to fix the issue in the TCG code instead (sorry
for being so long, the last weeks have been quite busy). I think it is
better than fixing the issue in target-xtensa.
If it works for you, I'll commit it.
> diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
> index 74e9888..f021a9a 100644
> --- a/target-xtensa/cpu.h
> +++ b/target-xtensa/cpu.h
> @@ -323,6 +323,7 @@ typedef struct CPUXtensaState {
> const XtensaConfig *config;
> uint32_t regs[16];
> uint32_t pc;
> + uint32_t sar_m32;
> uint32_t sregs[256];
> uint32_t uregs[256];
> uint32_t phys_regs[MAX_NAREG];
> diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
> index e5a3f49..4f3cf32 100644
> --- a/target-xtensa/translate.c
> +++ b/target-xtensa/translate.c
> @@ -56,8 +56,6 @@ typedef struct DisasContext {
>
> bool sar_5bit;
> bool sar_m32_5bit;
> - bool sar_m32_allocated;
> - TCGv_i32 sar_m32;
>
> uint32_t ccount_delta;
> unsigned used_window;
> @@ -71,6 +69,7 @@ typedef struct DisasContext {
>
> static TCGv_ptr cpu_env;
> static TCGv_i32 cpu_pc;
> +static TCGv_i32 cpu_sar_m32;
> static TCGv_i32 cpu_R[16];
> static TCGv_i32 cpu_FR[16];
> static TCGv_i32 cpu_SR[256];
> @@ -169,6 +168,8 @@ void xtensa_translate_init(void)
> cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
> cpu_pc = tcg_global_mem_new_i32(TCG_AREG0,
> offsetof(CPUXtensaState, pc), "pc");
> + cpu_sar_m32 = tcg_global_mem_new_i32(TCG_AREG0,
> + offsetof(CPUXtensaState, sar_m32), "sar_m32");
>
> for (i = 0; i < 16; i++) {
> cpu_R[i] = tcg_global_mem_new_i32(TCG_AREG0,
> @@ -230,21 +231,13 @@ static void init_sar_tracker(DisasContext *dc)
> {
> dc->sar_5bit = false;
> dc->sar_m32_5bit = false;
> - dc->sar_m32_allocated = false;
> -}
> -
> -static void reset_sar_tracker(DisasContext *dc)
> -{
> - if (dc->sar_m32_allocated) {
> - tcg_temp_free(dc->sar_m32);
> - }
> }
>
> static void gen_right_shift_sar(DisasContext *dc, TCGv_i32 sa)
> {
> tcg_gen_andi_i32(cpu_SR[SAR], sa, 0x1f);
> if (dc->sar_m32_5bit) {
> - tcg_gen_discard_i32(dc->sar_m32);
> + tcg_gen_discard_i32(cpu_sar_m32);
> }
> dc->sar_5bit = true;
> dc->sar_m32_5bit = false;
> @@ -253,12 +246,8 @@ static void gen_right_shift_sar(DisasContext *dc, TCGv_i32 sa)
> static void gen_left_shift_sar(DisasContext *dc, TCGv_i32 sa)
> {
> TCGv_i32 tmp = tcg_const_i32(32);
> - if (!dc->sar_m32_allocated) {
> - dc->sar_m32 = tcg_temp_local_new_i32();
> - dc->sar_m32_allocated = true;
> - }
> - tcg_gen_andi_i32(dc->sar_m32, sa, 0x1f);
> - tcg_gen_sub_i32(cpu_SR[SAR], tmp, dc->sar_m32);
> + tcg_gen_andi_i32(cpu_sar_m32, sa, 0x1f);
> + tcg_gen_sub_i32(cpu_SR[SAR], tmp, cpu_sar_m32);
> dc->sar_5bit = false;
> dc->sar_m32_5bit = true;
> tcg_temp_free(tmp);
> @@ -498,7 +487,7 @@ static void gen_wsr_sar(DisasContext *dc, uint32_t sr, TCGv_i32 s)
> {
> tcg_gen_andi_i32(cpu_SR[sr], s, 0x3f);
> if (dc->sar_m32_5bit) {
> - tcg_gen_discard_i32(dc->sar_m32);
> + tcg_gen_discard_i32(cpu_sar_m32);
> }
> dc->sar_5bit = false;
> dc->sar_m32_5bit = false;
> @@ -1483,7 +1472,7 @@ static void disas_xtensa_insn(CPUXtensaState *env, DisasContext *dc)
> case 10: /*SLL*/
> gen_window_check2(dc, RRR_R, RRR_S);
> if (dc->sar_m32_5bit) {
> - tcg_gen_shl_i32(cpu_R[RRR_R], cpu_R[RRR_S], dc->sar_m32);
> + tcg_gen_shl_i32(cpu_R[RRR_R], cpu_R[RRR_S], cpu_sar_m32);
> } else {
> TCGv_i64 v = tcg_temp_new_i64();
> TCGv_i32 s = tcg_const_i32(32);
> @@ -2947,7 +2936,6 @@ static void gen_intermediate_code_internal(
> tcg_ctx.gen_opc_ptr < gen_opc_end);
>
> reset_litbase(&dc);
> - reset_sar_tracker(&dc);
> if (dc.icount) {
> tcg_temp_free(dc.next_icount);
> }
> --
> 1.7.7.6
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] target-xtensa: make sar_m32 global instead of local temp
2012-11-24 10:59 ` Aurelien Jarno
@ 2012-11-24 11:27 ` Max Filippov
0 siblings, 0 replies; 3+ messages in thread
From: Max Filippov @ 2012-11-24 11:27 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: Blue Swirl, qemu-devel
On Sat, Nov 24, 2012 at 2:59 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Sat, Nov 24, 2012 at 04:51:36AM +0400, Max Filippov wrote:
>> This fixes the following assertion caused by local temp reaching the end
>> of TB in discarded state:
>>
>> qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
>> Aborted
>>
>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> ---
>> target-xtensa/cpu.h | 1 +
>> target-xtensa/translate.c | 28 ++++++++--------------------
>> 2 files changed, 9 insertions(+), 20 deletions(-)
>
> I have just send a patch to fix the issue in the TCG code instead (sorry
> for being so long, the last weeks have been quite busy). I think it is
> better than fixing the issue in target-xtensa.
>
> If it works for you, I'll commit it.
Works perfectly, thanks. (nothing to be sorry for, BTW).
And it seems to me that switch from local temp to global makes code a bit
cleaner. However, since it's no longer a fix I will hold it until 1.3 is out.
--
Thanks.
-- Max
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-11-24 11:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-24 0:51 [Qemu-devel] [PATCH] target-xtensa: make sar_m32 global instead of local temp Max Filippov
2012-11-24 10:59 ` Aurelien Jarno
2012-11-24 11:27 ` Max Filippov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).