qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] assertion in temp_save
@ 2012-11-18  3:19 Max Filippov
  2012-11-18  3:34 ` Max Filippov
  0 siblings, 1 reply; 5+ messages in thread
From: Max Filippov @ 2012-11-18  3:19 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: qemu-devel, Richard Henderson

Hi Aurelien,

starting with commit 2c0366f tcg: don't explicitly save globals and temps
I get the following abort on target-xtensa:

qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion
`s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
Aborted

I see that that commit only adds assertion and that bad thing happens
elsewhere. I've found that removal of tcg_gen_discard_i32 in the
gen_right_shift_sar makes it work again. The trace of the TB that fails
translation is below. If 'discard loc5' is removed it starts to work.

Any idea of what might be wrong?

OP:
 ---- 0xd010af67
 movi_i32 tmp1,$0xd010af67
 movi_i32 tmp2,$0x1
 movi_i32 tmp3,$0x1
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp3
 movi_i64 tmp4,$window_check
 call tmp4,$0x0,$0,env,tmp1,tmp2
 movi_i32 tmp1,$0x58
 add_i32 tmp0,ar1,tmp1
 qemu_ld32 ar4,tmp0,$0x0

 ---- 0xd010af6a
 movi_i32 tmp0,$0xd010af6a
 movi_i32 tmp1,$0x2
 movi_i32 tmp2,$0x1
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp2
 movi_i64 tmp4,$window_check
 call tmp4,$0x0,$0,env,tmp0,tmp1
 movi_i32 ar10,$0x3

 ---- 0xd010af6c
 movi_i32 tmp0,$0xfffffff0
 add_i32 ar9,ar9,tmp0

 ---- 0xd010af6f
 movi_i32 tmp0,$0xffffffff
 add_i32 ar6,ar4,tmp0

 ---- 0xd010af71
 movi_i32 ar4,$0x4

 ---- 0xd010af73
 movi_i32 tmp0,$0x0
 movcond_i32 ar10,ar9,tmp0,ar4,ar10,eq

 ---- 0xd010af76
 movi_i32 ar9,$0x0

 ---- 0xd010af78
 movi_i32 tmp0,$0xd010af78
 movi_i32 tmp1,$0x3
 movi_i32 tmp2,$0x6
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp2
 movi_i64 tmp4,$window_check
 call tmp4,$0x0,$0,env,tmp0,tmp1
 movi_i32 ar14,$0xffffffff

 ---- 0xd010af7a
 movi_i32 tmp0,$0xd0106524
 qemu_ld32 ar4,tmp0,$0x0

 ---- 0xd010af7d
 mov_i32 ar13,ar2

 ---- 0xd010af7f
 mov_i32 ar11,ar9

 ---- 0xd010af81
 mov_i32 ar15,ar9

 ---- 0xd010af83
 xor_i32 ar14,ar10,ar14

 ---- 0xd010af86
 mov_i32 ar2,ar8

 ---- 0xd010af88
 mov_i32 tmp0,ar2
 ext8u_i32 ar12,tmp0

 ---- 0xd010af8b
 and_i32 ar12,ar12,ar6

 ---- 0xd010af8e
 movi_i32 tmp0,$0x1
 shl_i32 ar8,ar5,tmp0

 ---- 0xd010af91
 add_i32 ar12,ar4,ar12

 ---- 0xd010af93
 movi_i32 tmp0,$0x20
 movi_i32 tmp1,$0x1f
 and_i32 loc5,ar14,tmp1
 sub_i32 SAR,tmp0,loc5

 ---- 0xd010af96
 shl_i32 ar8,ar8,loc5

 ---- 0xd010af99
 movi_i32 tmp0,$0x1f
 and_i32 SAR,ar10,tmp0
 discard loc5

 ---- 0xd010af9c
 shr_i32 ar2,ar2,SAR

 ---- 0xd010af9f
 mov_i32 tmp0,ar12
 qemu_ld8u ar12,tmp0,$0x0

 ---- 0xd010afa2
 movi_i32 tmp0,$0x1f
 and_i32 SAR,ar10,tmp0

 ---- 0xd010afa5
 shr_i32 ar5,ar5,SAR

 ---- 0xd010afa8
 or_i32 ar2,ar8,ar2

 ---- 0xd010afab
 movi_i32 tmp0,$0x0
 movcond_i32 ar2,ar11,tmp0,ar5,ar2,ne

 ---- 0xd010afae
 add_i32 ar8,ar1,ar9

 ---- 0xd010afb0
 movi_i32 tmp0,$0x0
 movcond_i32 ar5,ar11,tmp0,ar15,ar5,ne

 ---- 0xd010afb3
 or_i32 ar12,ar7,ar12

 ---- 0xd010afb6
 mov_i32 tmp0,ar8
 qemu_st8 ar12,tmp0,$0x0

 ---- 0xd010afb9
 or_i32 ar8,ar2,ar5

 ---- 0xd010afbc
 movi_i32 tmp0,$0x1
 add_i32 ar9,ar9,tmp0

 ---- 0xd010afbe
 movi_i32 tmp0,$0x0
 movi_i32 tmp1,$0x1a
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp1
 brcond_i32 ar8,tmp0,ne,$0x0
 movi_i32 tmp1,$0xd010afc1
 mov_i32 pc,tmp1
 goto_tb $0x0
 exit_tb $0x7f4cd9091630
 set_label $0x0
 movi_i32 tmp1,$0xd010af88
 mov_i32 pc,tmp1
 goto_tb $0x1
 exit_tb $0x7f4cd9091631

OP after optimization and liveness analysis:
 ---- 0xd010af67
 movi_i32 tmp1,$0xd010af67
 movi_i32 tmp2,$0x1
 movi_i32 tmp3,$0x1
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp3
 movi_i64 tmp4,$window_check
 call tmp4,$0x0,$0,env,tmp1,tmp2
 movi_i32 tmp1,$0x58
 add_i32 tmp0,ar1,tmp1
 qemu_ld32 ar4,tmp0,$0x0

 ---- 0xd010af6a
 movi_i32 tmp0,$0xd010af6a
 movi_i32 tmp1,$0x2
 movi_i32 tmp2,$0x1
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp2
 movi_i64 tmp4,$window_check
 call tmp4,$0x0,$0,env,tmp0,tmp1
 movi_i32 ar10,$0x3

 ---- 0xd010af6c
 movi_i32 tmp0,$0xfffffff0
 add_i32 ar9,ar9,tmp0

 ---- 0xd010af6f
 movi_i32 tmp0,$0xffffffff
 add_i32 ar6,ar4,tmp0

 ---- 0xd010af71
 movi_i32 ar4,$0x4

 ---- 0xd010af73
 movi_i32 tmp0,$0x0
 movcond_i32 ar10,ar9,tmp0,ar4,ar10,eq

 ---- 0xd010af76
 movi_i32 ar9,$0x0

 ---- 0xd010af78
 movi_i32 tmp0,$0xd010af78
 movi_i32 tmp1,$0x3
 movi_i32 tmp2,$0x6
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp2
 movi_i64 tmp4,$window_check
 call tmp4,$0x0,$0,env,tmp0,tmp1
 movi_i32 ar14,$0xffffffff

 ---- 0xd010af7a
 movi_i32 tmp0,$0xd0106524
 qemu_ld32 ar4,tmp0,$0x0

 ---- 0xd010af7d
 mov_i32 ar13,ar2

 ---- 0xd010af7f
 mov_i32 ar11,ar9

 ---- 0xd010af81
 mov_i32 ar15,ar9

 ---- 0xd010af83
 xor_i32 ar14,ar10,ar14

 ---- 0xd010af86
 mov_i32 ar2,ar8

 ---- 0xd010af88
 nopn $0x2,$0x2
 ext8u_i32 ar12,ar8

 ---- 0xd010af8b
 and_i32 ar12,ar12,ar6

 ---- 0xd010af8e
 movi_i32 tmp0,$0x1
 shl_i32 ar8,ar5,tmp0

 ---- 0xd010af91
 add_i32 ar12,ar12,ar4

 ---- 0xd010af93
 nopn $0x2,$0x2
 movi_i32 tmp1,$0x1f
 and_i32 loc5,ar14,tmp1
 nopn $0x3,$0x64,$0x3

 ---- 0xd010af96
 shl_i32 ar8,ar8,loc5

 ---- 0xd010af99
 movi_i32 tmp0,$0x1f
 and_i32 SAR,ar10,tmp0
 discard loc5

 ---- 0xd010af9c
 shr_i32 ar2,ar2,SAR

 ---- 0xd010af9f
 nopn $0x2,$0x2
 qemu_ld8u ar12,ar12,$0x0

 ---- 0xd010afa2
 movi_i32 tmp0,$0x1f
 and_i32 SAR,ar10,tmp0

 ---- 0xd010afa5
 shr_i32 ar5,ar5,SAR

 ---- 0xd010afa8
 or_i32 ar2,ar2,ar8

 ---- 0xd010afab
 movi_i32 tmp0,$0x0
 movcond_i32 ar2,ar11,tmp0,ar5,ar2,ne

 ---- 0xd010afae
 add_i32 ar8,ar1,ar9

 ---- 0xd010afb0
 movi_i32 tmp0,$0x0
 movcond_i32 ar5,ar11,tmp0,ar15,ar5,ne

 ---- 0xd010afb3
 or_i32 ar12,ar12,ar7

 ---- 0xd010afb6
 nopn $0x2,$0x2
 qemu_st8 ar12,ar8,$0x0

 ---- 0xd010afb9
 or_i32 ar8,ar2,ar5

 ---- 0xd010afbc
 movi_i32 tmp0,$0x1
 add_i32 ar9,ar9,tmp0

 ---- 0xd010afbe
 movi_i32 tmp0,$0x0
 movi_i32 tmp1,$0x1a
 movi_i64 tmp4,$advance_ccount
 call tmp4,$0x0,$0,env,tmp1
 brcond_i32 ar8,tmp0,ne,$0x0
 nopn $0x2,$0x2
 movi_i32 pc,$0xd010afc1
 goto_tb $0x0
 exit_tb $0x7f4cd9091630
 set_label $0x0
 nopn $0x2,$0x2
 movi_i32 pc,$0xd010af88
 goto_tb $0x1
 exit_tb $0x7f4cd9091631
 end


-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] assertion in temp_save
  2012-11-18  3:19 [Qemu-devel] assertion in temp_save Max Filippov
@ 2012-11-18  3:34 ` Max Filippov
  2012-11-20  2:09   ` Max Filippov
  0 siblings, 1 reply; 5+ messages in thread
From: Max Filippov @ 2012-11-18  3:34 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: qemu-devel, Richard Henderson

On Sun, Nov 18, 2012 at 7:19 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Hi Aurelien,
>
> starting with commit 2c0366f tcg: don't explicitly save globals and temps
> I get the following abort on target-xtensa:
>
> qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion
> `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
> Aborted
>
> I see that that commit only adds assertion and that bad thing happens
> elsewhere. I've found that removal of tcg_gen_discard_i32 in the
> gen_right_shift_sar makes it work again. The trace of the TB that fails
> translation is below. If 'discard loc5' is removed it starts to work.
>
> Any idea of what might be wrong?

In the debugger loc5 looks like this when abort happens:

(gdb) p s->temps[105]
$2 = {
  base_type = TCG_TYPE_I32,
  type = TCG_TYPE_I32,
  val_type = 0,
  reg = 11,
  val = 32,
  mem_reg = 4,
  mem_offset = 128,
  fixed_reg = 0,
  mem_coherent = 0,
  mem_allocated = 0,
  temp_local = 1,
  temp_allocated = 0,
  next_free_temp = -1,
  name = 0x0
}

[...]

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] assertion in temp_save
  2012-11-18  3:34 ` Max Filippov
@ 2012-11-20  2:09   ` Max Filippov
  2012-11-20 17:47     ` Aurelien Jarno
  0 siblings, 1 reply; 5+ messages in thread
From: Max Filippov @ 2012-11-20  2:09 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: qemu-devel, Richard Henderson

On Sun, Nov 18, 2012 at 7:34 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Sun, Nov 18, 2012 at 7:19 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> Hi Aurelien,
>>
>> starting with commit 2c0366f tcg: don't explicitly save globals and temps
>> I get the following abort on target-xtensa:
>>
>> qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion
>> `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
>> Aborted
>>
>> I see that that commit only adds assertion and that bad thing happens
>> elsewhere. I've found that removal of tcg_gen_discard_i32 in the
>> gen_right_shift_sar makes it work again. The trace of the TB that fails
>> translation is below. If 'discard loc5' is removed it starts to work.
>>
>> Any idea of what might be wrong?
>
> In the debugger loc5 looks like this when abort happens:
>
> (gdb) p s->temps[105]
> $2 = {
>   base_type = TCG_TYPE_I32,
>   type = TCG_TYPE_I32,
>   val_type = 0,
>   reg = 11,
>   val = 32,
>   mem_reg = 4,
>   mem_offset = 128,
>   fixed_reg = 0,
>   mem_coherent = 0,
>   mem_allocated = 0,
>   temp_local = 1,
>   temp_allocated = 0,
>   next_free_temp = -1,
>   name = 0x0
> }

Looks like the issue is local temp reaching the end of TB in a dead state.
Hence the question: is discard applicable to local temps?
Or maybe I should just make it global (two other tcg values used with discard
in other targets are also globals) and avoid temp_local_new/temp_free at
first place?

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] assertion in temp_save
  2012-11-20  2:09   ` Max Filippov
@ 2012-11-20 17:47     ` Aurelien Jarno
  2012-11-20 19:33       ` Max Filippov
  0 siblings, 1 reply; 5+ messages in thread
From: Aurelien Jarno @ 2012-11-20 17:47 UTC (permalink / raw)
  To: Max Filippov; +Cc: qemu-devel, Richard Henderson

On Tue, Nov 20, 2012 at 05:09:57AM +0300, Max Filippov wrote:
> On Sun, Nov 18, 2012 at 7:34 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> > On Sun, Nov 18, 2012 at 7:19 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> >> Hi Aurelien,
> >>
> >> starting with commit 2c0366f tcg: don't explicitly save globals and temps
> >> I get the following abort on target-xtensa:
> >>
> >> qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion
> >> `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
> >> Aborted
> >>
> >> I see that that commit only adds assertion and that bad thing happens
> >> elsewhere. I've found that removal of tcg_gen_discard_i32 in the
> >> gen_right_shift_sar makes it work again. The trace of the TB that fails
> >> translation is below. If 'discard loc5' is removed it starts to work.
> >>
> >> Any idea of what might be wrong?
> >
> > In the debugger loc5 looks like this when abort happens:
> >
> > (gdb) p s->temps[105]
> > $2 = {
> >   base_type = TCG_TYPE_I32,
> >   type = TCG_TYPE_I32,
> >   val_type = 0,
> >   reg = 11,
> >   val = 32,
> >   mem_reg = 4,
> >   mem_offset = 128,
> >   fixed_reg = 0,
> >   mem_coherent = 0,
> >   mem_allocated = 0,
> >   temp_local = 1,
> >   temp_allocated = 0,
> >   next_free_temp = -1,
> >   name = 0x0
> > }
> 
> Looks like the issue is local temp reaching the end of TB in a dead state.
> Hence the question: is discard applicable to local temps?
> Or maybe I should just make it global (two other tcg values used with discard
> in other targets are also globals) and avoid temp_local_new/temp_free at
> first place?
> 

Indeed, it looks like that discard doesn't work correctly with a temp
local with this new patch. I might have a fix, but I would like to do
some more tests first. Would it be possible to provide a way to reproduce
the issue?

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] assertion in temp_save
  2012-11-20 17:47     ` Aurelien Jarno
@ 2012-11-20 19:33       ` Max Filippov
  0 siblings, 0 replies; 5+ messages in thread
From: Max Filippov @ 2012-11-20 19:33 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: qemu-devel, Richard Henderson

On Tue, Nov 20, 2012 at 9:47 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Tue, Nov 20, 2012 at 05:09:57AM +0300, Max Filippov wrote:
>> On Sun, Nov 18, 2012 at 7:34 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> > On Sun, Nov 18, 2012 at 7:19 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> >> Hi Aurelien,
>> >>
>> >> starting with commit 2c0366f tcg: don't explicitly save globals and temps
>> >> I get the following abort on target-xtensa:
>> >>
>> >> qemu-system-xtensa: tcg/tcg.c:1665: temp_save: Assertion
>> >> `s->temps[temp].val_type == 2 || s->temps[temp].fixed_reg' failed.
>> >> Aborted
>> >>
>> >> I see that that commit only adds assertion and that bad thing happens
>> >> elsewhere. I've found that removal of tcg_gen_discard_i32 in the
>> >> gen_right_shift_sar makes it work again. The trace of the TB that fails
>> >> translation is below. If 'discard loc5' is removed it starts to work.
>> >>
>> >> Any idea of what might be wrong?
>> >
>> > In the debugger loc5 looks like this when abort happens:
>> >
>> > (gdb) p s->temps[105]
>> > $2 = {
>> >   base_type = TCG_TYPE_I32,
>> >   type = TCG_TYPE_I32,
>> >   val_type = 0,
>> >   reg = 11,
>> >   val = 32,
>> >   mem_reg = 4,
>> >   mem_offset = 128,
>> >   fixed_reg = 0,
>> >   mem_coherent = 0,
>> >   mem_allocated = 0,
>> >   temp_local = 1,
>> >   temp_allocated = 0,
>> >   next_free_temp = -1,
>> >   name = 0x0
>> > }
>>
>> Looks like the issue is local temp reaching the end of TB in a dead state.
>> Hence the question: is discard applicable to local temps?
>> Or maybe I should just make it global (two other tcg values used with discard
>> in other targets are also globals) and avoid temp_local_new/temp_free at
>> first place?
>>
>
> Indeed, it looks like that discard doesn't work correctly with a temp
> local with this new patch. I might have a fix, but I would like to do
> some more tests first. Would it be possible to provide a way to reproduce
> the issue?

Here's the test case (xtensa assembler):

.section .init
.global _start
_start:
    ssl     a2
    ssr     a2
    j       _start

Sources + compiled version is available at
http://jcmvbkbc.spb.ru/~dumb/ws/osll/qemu-xtensa/20121120/

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-11-20 19:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-18  3:19 [Qemu-devel] assertion in temp_save Max Filippov
2012-11-18  3:34 ` Max Filippov
2012-11-20  2:09   ` Max Filippov
2012-11-20 17:47     ` Aurelien Jarno
2012-11-20 19:33       ` 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).