qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] x86_64-softmmu broken on Windows (TCG?)
@ 2012-10-29 17:53 Paolo Bonzini
  2012-10-29 18:29 ` Aurelien Jarno
  0 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2012-10-29 17:53 UTC (permalink / raw)
  To: qemu-devel, Stefan Weil, Aurelien Jarno

Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9

i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
glad if somebody else beats me.  It can be reproduced with Wine and
"x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.

Paolo

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

* Re: [Qemu-devel] x86_64-softmmu broken on Windows (TCG?)
  2012-10-29 17:53 [Qemu-devel] x86_64-softmmu broken on Windows (TCG?) Paolo Bonzini
@ 2012-10-29 18:29 ` Aurelien Jarno
  2012-10-30  8:15   ` [Qemu-devel] 64-on-32 TCG broken [was Re: x86_64-softmmu broken on Windows (TCG?)] Paolo Bonzini
  0 siblings, 1 reply; 15+ messages in thread
From: Aurelien Jarno @ 2012-10-29 18:29 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Stefan Weil, qemu-devel

On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
> Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
> Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
> 
> i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
> glad if somebody else beats me.  It can be reproduced with Wine and
> "x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.

Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
but right now I don't have a good network connection enough to either
setup a mingw build environment or to connect to a remote machine with
such an environment.


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

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

* [Qemu-devel] 64-on-32 TCG broken [was Re: x86_64-softmmu broken on Windows (TCG?)]
  2012-10-29 18:29 ` Aurelien Jarno
@ 2012-10-30  8:15   ` Paolo Bonzini
  2012-10-30 22:24     ` [Qemu-devel] 64-on-32 TCG broken Stefan Weil
  0 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2012-10-30  8:15 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Stefan Weil, qemu-devel

Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
> On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
>> > Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
>> > Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
>> > 
>> > i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
>> > glad if somebody else beats me.  It can be reproduced with Wine and
>> > "x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
> Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
> but right now I don't have a good network connection enough to either
> setup a mingw build environment or to connect to a remote machine with
> such an environment.

It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
and it segfaults on startup.  Even the previous commit cannot run
qemu-x86_64 /bin/ls correctly:

$ git whatis HEAD
ec7a869 (tcg: sync output arguments on liveness request, 2012-10-09)
$ x86_64-linux-user/qemu-x86_64 /bin/ls
inux-user

$ git whatis HEAD
9c43b68 (tcg: rework liveness analysis, 2012-10-09)
$ x86_64-linux-user/qemu-x86_64 /bin/ls
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Errore di segmentazione


Regarding the win32 failure, it's early enough that the TCG logs give
an idea of what is happening.  This *might* be a reduced testcase,
but the general breakage makes it impossible to check:

asm("\n\
h:\n\
         .byte 2\n\
f:\n\
         push %rax\n\
	 push %rdx\n\
	 movb h, %al\n\
	 cmp $0x12, %al\n\
	 pop %rdx\n\
	 pop %rax\n\
	 ret\n\
g:\n\
         xor %eax, %eax\n\
         call f\n\
	 setne %al\n\
	 ret\n\
	 ");

extern int g();
int main()
{
	printf("%d\n", g());
}


Anyhow, here are the logs (good on the left, differences on the
right).  A write to cc_dst is incorrectly deleted as dead:

IN:                                         (
0x00000000000c83e9:  push   %ax             (
0x00000000000c83ea:  push   %dx             (
0x00000000000c83eb:  mov    $0x9206,%ax     (
0x00000000000c83ee:  mov    $0x3c4,%dx      (
0x00000000000c83f1:  out    %ax,(%dx)       (
0x00000000000c83f2:  inc    %dx             (
0x00000000000c83f3:  in     (%dx),%al       (
0x00000000000c83f4:  cmp    $0x12,%al       (
0x00000000000c83f6:  pop    %dx             (
0x00000000000c83f7:  pop    %ax             (
0x00000000000c83f8:  ret                    (
                                            (
OP:                                         (
 ---- 0xc83e9                               (
 mov_i32 tmp0,rax_0                         (
 mov_i32 tmp1,rax_1                         (
 mov_i32 tmp4,rsp_0                         (
 mov_i32 tmp5,rsp_1                         (
 movi_i32 tmp20,$0xfffffffe                 (
 movi_i32 tmp21,$0xffffffff                 (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp20,tmp21   (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 ext16u_i32 tmp4,tmp4                       (
 movi_i32 tmp5,$0x0                         (
 mov_i32 tmp2,tmp4                          (
 mov_i32 tmp3,tmp5                          (
 ld_i32 tmp8,env,$0xe8                      (
 ld_i32 tmp9,env,$0xec                      (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp8,tmp9     (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 qemu_st16 tmp0,tmp4,tmp5,$0x0              (
 deposit_i32 rsp_0,rsp_0,tmp2,$0x0,$0x10    (
                                            (
 ---- 0xc83ea                               (
 mov_i32 tmp0,rdx_0                         (
 mov_i32 tmp1,rdx_1                         (
 mov_i32 tmp4,rsp_0                         (
 mov_i32 tmp5,rsp_1                         (
 movi_i32 tmp20,$0xfffffffe                 (
 movi_i32 tmp21,$0xffffffff                 (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp20,tmp21   (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 ext16u_i32 tmp4,tmp4                       (
 movi_i32 tmp5,$0x0                         (
 mov_i32 tmp2,tmp4                          (
 mov_i32 tmp3,tmp5                          (
 ld_i32 tmp8,env,$0xe8                      (
 ld_i32 tmp9,env,$0xec                      (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp8,tmp9     (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 qemu_st16 tmp0,tmp4,tmp5,$0x0              (
 deposit_i32 rsp_0,rsp_0,tmp2,$0x0,$0x10    (
                                            (
 ---- 0xc83eb                               (
 movi_i32 tmp0,$0x9206                      (
 movi_i32 tmp1,$0x0                         (
 deposit_i32 rax_0,rax_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83ee                               (
 movi_i32 tmp0,$0x3c4                       (
 movi_i32 tmp1,$0x0                         (
 deposit_i32 rdx_0,rdx_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83f1                               (
 mov_i32 tmp0,rdx_0                         (
 mov_i32 tmp1,rdx_1                         (
 ext16u_i32 tmp0,tmp0                       (
 movi_i32 tmp1,$0x0                         (
 mov_i32 tmp2,rax_0                         (
 mov_i32 tmp3,rax_1                         (
 mov_i32 tmp12,tmp0                         (
 mov_i32 tmp13,tmp2                         (
 movi_i32 tmp22,$outw                       (
 call tmp22,$0x0,$0,tmp12,tmp13             (
                                            (
 ---- 0xc83f2                               (
 mov_i32 tmp0,rdx_0                         (
 mov_i32 tmp1,rdx_1                         (
 movi_i32 tmp20,$0x1                        (
 movi_i32 tmp21,$0x0                        (
 add2_i32 tmp0,tmp1,tmp0,tmp1,tmp20,tmp21   (
 nop                                        (
 deposit_i32 rdx_0,rdx_0,tmp0,$0x0,$0x10    (
 movi_i32 tmp22,$cc_compute_c               (
 call tmp22,$0x10,$1,tmp12,env,cc_op        (
 mov_i32 cc_src_0,tmp12                     (
 movi_i32 cc_src_1,$0x0                     (
 mov_i32 cc_dst_0,tmp0                      (
 mov_i32 cc_dst_1,tmp1                      (
                                            (
 ---- 0xc83f3                               (
 mov_i32 tmp0,rdx_0                         (
 mov_i32 tmp1,rdx_1                         (
 ext16u_i32 tmp0,tmp0                       (
 movi_i32 tmp1,$0x0                         (
 mov_i32 tmp12,tmp0                         (
 movi_i32 tmp22,$inb                        (
 call tmp22,$0x0,$2,tmp2,tmp3,tmp12         (
 deposit_i32 rax_0,rax_0,tmp2,$0x0,$0x8     (
                                            (
 ---- 0xc83f4                               (
 movi_i32 tmp2,$0x12                        (
 movi_i32 tmp3,$0x0                         (
 mov_i32 tmp0,rax_0                         (
 mov_i32 tmp1,rax_1                         (
 mov_i32 cc_src_0,tmp2                      (
 mov_i32 cc_src_1,tmp3                      (
 sub2_i32 cc_dst_0,cc_dst_1,tmp0,tmp1,tmp2  (
 nop                                        (
                                            (
 ---- 0xc83f6                               (
 mov_i32 tmp4,rsp_0                         (
 mov_i32 tmp5,rsp_1                         (
 ext16u_i32 tmp4,tmp4                       (
 movi_i32 tmp5,$0x0                         (
 ld_i32 tmp8,env,$0xe8                      (
 ld_i32 tmp9,env,$0xec                      (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp8,tmp9     (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 qemu_ld16u tmp0,tmp4,tmp5,$0x0             (
 movi_i32 tmp1,$0x0                         (
 movi_i32 tmp20,$0x2                        (
 movi_i32 tmp21,$0x0                        (
 add2_i32 tmp8,tmp9,rsp_0,rsp_1,tmp20,tmp2  (
 nop                                        (
 deposit_i32 rsp_0,rsp_0,tmp8,$0x0,$0x10    (
 deposit_i32 rdx_0,rdx_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83f7                               (
 mov_i32 tmp4,rsp_0                         (
 mov_i32 tmp5,rsp_1                         (
 ext16u_i32 tmp4,tmp4                       (
 movi_i32 tmp5,$0x0                         (
 ld_i32 tmp8,env,$0xe8                      (
 ld_i32 tmp9,env,$0xec                      (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp8,tmp9     (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 qemu_ld16u tmp0,tmp4,tmp5,$0x0             (
 movi_i32 tmp1,$0x0                         (
 movi_i32 tmp20,$0x2                        (
 movi_i32 tmp21,$0x0                        (
 add2_i32 tmp8,tmp9,rsp_0,rsp_1,tmp20,tmp2  (
 nop                                        (
 deposit_i32 rsp_0,rsp_0,tmp8,$0x0,$0x10    (
 deposit_i32 rax_0,rax_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83f8                               (
 mov_i32 tmp4,rsp_0                         (
 mov_i32 tmp5,rsp_1                         (
 ext16u_i32 tmp4,tmp4                       (
 movi_i32 tmp5,$0x0                         (
 ld_i32 tmp8,env,$0xe8                      (
 ld_i32 tmp9,env,$0xec                      (
 add2_i32 tmp4,tmp5,tmp4,tmp5,tmp8,tmp9     (
 nop                                        (
 movi_i32 tmp5,$0x0                         (
 qemu_ld16u tmp0,tmp4,tmp5,$0x0             (
 movi_i32 tmp1,$0x0                         (
 movi_i32 tmp20,$0x2                        (
 movi_i32 tmp21,$0x0                        (
 add2_i32 tmp8,tmp9,rsp_0,rsp_1,tmp20,tmp2  (
 nop                                        (
 deposit_i32 rsp_0,rsp_0,tmp8,$0x0,$0x10    (
 ext16u_i32 tmp0,tmp0                       (
 movi_i32 tmp1,$0x0                         (
 st_i32 tmp0,env,$0x80                      (
 st_i32 tmp1,env,$0x84                      (
 movi_i32 cc_op,$0xe                        (
 exit_tb $0x0                               (
                                            (
OP after optimization and liveness analysi  (
 ---- 0xc83e9                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 movi_i32 tmp20,$0xfffffffe                 (
 nopn $0x2,$0x2                             (
 add_i32 tmp4,rsp_0,tmp20                   (
 nopn $0x3,$0x3c,$0x3                       (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp4,tmp4                       (
 nopn $0x2,$0x2                             (
 mov_i32 tmp2,tmp4                          (
 nopn $0x2,$0x2                             (
 ld_i32 tmp8,env,$0xe8                      (
 nopn $0x3,$0x0,$0x3                        (
 add_i32 tmp4,tmp4,tmp8                     (
 nopn $0x3,$0x30,$0x3                       (
 movi_i32 tmp5,$0x0                         (
 qemu_st16 rax_0,tmp4,tmp5,$0x0             (
 deposit_i32 rsp_0,rsp_0,tmp2,$0x0,$0x10    (
                                            (
 ---- 0xc83ea                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 movi_i32 tmp20,$0xfffffffe                 (
 nopn $0x2,$0x2                             (
 add_i32 tmp4,rsp_0,tmp20                   (
 nopn $0x3,$0x3c,$0x3                       (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp4,tmp4                       (
 nopn $0x2,$0x2                             (
 mov_i32 tmp2,tmp4                          (
 nopn $0x2,$0x2                             (
 ld_i32 tmp8,env,$0xe8                      (
 nopn $0x3,$0x0,$0x3                        (
 add_i32 tmp4,tmp4,tmp8                     (
 nopn $0x3,$0x30,$0x3                       (
 movi_i32 tmp5,$0x0                         (
 qemu_st16 rdx_0,tmp4,tmp5,$0x0             (
 deposit_i32 rsp_0,rsp_0,tmp2,$0x0,$0x10    (
                                            (
 ---- 0xc83eb                               (
 movi_i32 tmp0,$0x9206                      (
 nopn $0x2,$0x2                             (
 deposit_i32 rax_0,rax_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83ee                               (
 movi_i32 tmp0,$0x3c4                       (
 nopn $0x2,$0x2                             (
 deposit_i32 rdx_0,rdx_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83f1                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp0,rdx_0                      (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 mov_i32 tmp12,tmp0                         (
 nopn $0x2,$0x2                             (
 movi_i32 tmp22,$outw                       (
 call tmp22,$0x0,$0,tmp12,rax_0             (
                                            (
 ---- 0xc83f2                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 movi_i32 tmp20,$0x1                        (
 movi_i32 tmp21,$0x0                        (
 add2_i32 tmp0,tmp1,rdx_0,rdx_1,tmp20,tmp2  (
 nop                                        (
 deposit_i32 rdx_0,rdx_0,tmp0,$0x0,$0x10    (
 movi_i32 tmp22,$cc_compute_c               (
 call tmp22,$0x10,$1,tmp12,env,cc_op        (
 mov_i32 cc_src_0,tmp12                     (
 movi_i32 cc_src_1,$0x0                     (
 mov_i32 cc_dst_0,tmp0                      (
 mov_i32 cc_dst_1,tmp1                      (
                                            (
 ---- 0xc83f3                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp0,rdx_0                      (
 nopn $0x2,$0x2                             (
 mov_i32 tmp12,tmp0                         (
 movi_i32 tmp22,$inb                        (
 call tmp22,$0x0,$2,tmp2,tmp3,tmp12         (
 deposit_i32 rax_0,rax_0,tmp2,$0x0,$0x8     (
                                            (
 ---- 0xc83f4                               (
 movi_i32 tmp2,$0x12                        |    nopn $0x2,$0x2
 movi_i32 tmp3,$0x0                         |    nopn $0x2,$0x2
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 movi_i32 cc_src_0,$0x12                    (
 movi_i32 cc_src_1,$0x0                     (
 sub2_i32 cc_dst_0,cc_dst_1,rax_0,rax_1,tm  |    nopn $0x6,$0x5,$0x8,$0x9,$0x2a,$0x6
 nop                                        (
                                            (
 ---- 0xc83f6                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp4,rsp_0                      (
 nopn $0x2,$0x2                             (
 ld_i32 tmp8,env,$0xe8                      (
 nopn $0x3,$0x0,$0x3                        (
 add_i32 tmp4,tmp4,tmp8                     (
 nopn $0x3,$0x30,$0x3                       (
 movi_i32 tmp5,$0x0                         (
 qemu_ld16u tmp0,tmp4,tmp5,$0x0             (
 nopn $0x2,$0x2                             (
 movi_i32 tmp20,$0x2                        (
 nopn $0x2,$0x2                             (
 add_i32 tmp8,rsp_0,tmp20                   (
 nopn $0x3,$0x3c,$0x3                       (
 deposit_i32 rsp_0,rsp_0,tmp8,$0x0,$0x10    (
 deposit_i32 rdx_0,rdx_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83f7                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp4,rsp_0                      (
 nopn $0x2,$0x2                             (
 ld_i32 tmp8,env,$0xe8                      (
 nopn $0x3,$0x0,$0x3                        (
 add_i32 tmp4,tmp4,tmp8                     (
 nopn $0x3,$0x30,$0x3                       (
 movi_i32 tmp5,$0x0                         (
 qemu_ld16u tmp0,tmp4,tmp5,$0x0             (
 nopn $0x2,$0x2                             (
 movi_i32 tmp20,$0x2                        (
 nopn $0x2,$0x2                             (
 add_i32 tmp8,rsp_0,tmp20                   (
 nopn $0x3,$0x3c,$0x3                       (
 deposit_i32 rsp_0,rsp_0,tmp8,$0x0,$0x10    (
 deposit_i32 rax_0,rax_0,tmp0,$0x0,$0x10    (
                                            (
 ---- 0xc83f8                               (
 nopn $0x2,$0x2                             (
 nopn $0x2,$0x2                             (
 ext16u_i32 tmp4,rsp_0                      (
 nopn $0x2,$0x2                             (
 ld_i32 tmp8,env,$0xe8                      (
 nopn $0x3,$0x0,$0x3                        (
 add_i32 tmp4,tmp4,tmp8                     (
 nopn $0x3,$0x30,$0x3                       (
 movi_i32 tmp5,$0x0                         (
 qemu_ld16u tmp0,tmp4,tmp5,$0x0             (
 nopn $0x2,$0x2                             (
 movi_i32 tmp20,$0x2                        (
 nopn $0x2,$0x2                             (
 add_i32 tmp8,rsp_0,tmp20                   (
 nopn $0x3,$0x3c,$0x3                       (
 deposit_i32 rsp_0,rsp_0,tmp8,$0x0,$0x10    (
 ext16u_i32 tmp0,tmp0                       (
 movi_i32 tmp1,$0x0                         (
 st_i32 tmp0,env,$0x80                      (
 st_i32 tmp1,env,$0x84                      (
 movi_i32 cc_op,$0xe                        (
 exit_tb $0x0                               (
 end                                        (
                                            (

and then the next basic block jumps in the weeds:

IN:                                         (
0x00000000000c83a0:  jne    0xc83d3         (

IN:                                         (
0x00000000000c83a2:  push   %ds             | 0x00000000000c83d3:  ret
0x00000000000c83a3:  xor    %ax,%ax         <
0x00000000000c83a5:  mov    %ax,%ds         <
0x00000000000c83a7:  mov    $0x83f9,%ax     <
0x00000000000c83aa:  mov    %ax,0x40        <
0x00000000000c83ad:  mov    $0xc000,%ax     <
0x00000000000c83b0:  mov    %ax,0x42        <
0x00000000000c83b3:  pop    %ds             <

etc.

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-30  8:15   ` [Qemu-devel] 64-on-32 TCG broken [was Re: x86_64-softmmu broken on Windows (TCG?)] Paolo Bonzini
@ 2012-10-30 22:24     ` Stefan Weil
  2012-10-30 23:22       ` Aurelien Jarno
  2012-10-30 23:56       ` Aurelien Jarno
  0 siblings, 2 replies; 15+ messages in thread
From: Stefan Weil @ 2012-10-30 22:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Aurelien Jarno

Am 30.10.2012 09:15, schrieb Paolo Bonzini:
> Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
>> On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
>>>> Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
>>>> Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
>>>>
>>>> i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
>>>> glad if somebody else beats me.  It can be reproduced with Wine and
>>>> "x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
>> Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
>> but right now I don't have a good network connection enough to either
>> setup a mingw build environment or to connect to a remote machine with
>> such an environment.
>
> It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
> analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
> generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
> and it segfaults on startup.  Even the previous commit cannot run
> qemu-x86_64 /bin/ls correctly:
>

I just tested with latest qemu-system-x86_64 on 32 bit Linux.

It also hangs during boot (BIOS), so it looks like this
is not a MinGW only problem.

Your test with x86_64-linux-user indicates that, too.

I also get the problem with TCI. Therefore I expect that any
32 bit TCG target will show it.

Regards
Stefan W.



> $ git whatis HEAD
> ec7a869 (tcg: sync output arguments on liveness request, 2012-10-09)
> $ x86_64-linux-user/qemu-x86_64 /bin/ls
> inux-user
>
> $ git whatis HEAD
> 9c43b68 (tcg: rework liveness analysis, 2012-10-09)
> $ x86_64-linux-user/qemu-x86_64 /bin/ls
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Errore di segmentazione
>
>
> Regarding the win32 failure, it's early enough that the TCG logs give
> an idea of what is happening.  This *might* be a reduced testcase,
> but the general breakage makes it impossible to check:

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-30 22:24     ` [Qemu-devel] 64-on-32 TCG broken Stefan Weil
@ 2012-10-30 23:22       ` Aurelien Jarno
  2012-10-30 23:56       ` Aurelien Jarno
  1 sibling, 0 replies; 15+ messages in thread
From: Aurelien Jarno @ 2012-10-30 23:22 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

On Tue, Oct 30, 2012 at 11:24:34PM +0100, Stefan Weil wrote:
> Am 30.10.2012 09:15, schrieb Paolo Bonzini:
> >Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
> >>On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
> >>>>Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
> >>>>Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
> >>>>
> >>>>i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
> >>>>glad if somebody else beats me.  It can be reproduced with Wine and
> >>>>"x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
> >>Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
> >>but right now I don't have a good network connection enough to either
> >>setup a mingw build environment or to connect to a remote machine with
> >>such an environment.
> >
> >It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
> >analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
> >generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
> >and it segfaults on startup.  Even the previous commit cannot run
> >qemu-x86_64 /bin/ls correctly:
> >
> 
> I just tested with latest qemu-system-x86_64 on 32 bit Linux.
> 
> It also hangs during boot (BIOS), so it looks like this
> is not a MinGW only problem.
> 
> Your test with x86_64-linux-user indicates that, too.
> 
> I also get the problem with TCI. Therefore I expect that any
> 32 bit TCG target will show it.
> 

The problem is related to the recent cases for add2/sub2/mulu2 addition
which do not play well with the optimizations. I am still trying to
understand why.

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

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-30 22:24     ` [Qemu-devel] 64-on-32 TCG broken Stefan Weil
  2012-10-30 23:22       ` Aurelien Jarno
@ 2012-10-30 23:56       ` Aurelien Jarno
  2012-10-31 12:40         ` Aurelien Jarno
                           ` (2 more replies)
  1 sibling, 3 replies; 15+ messages in thread
From: Aurelien Jarno @ 2012-10-30 23:56 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

On Tue, Oct 30, 2012 at 11:24:34PM +0100, Stefan Weil wrote:
> Am 30.10.2012 09:15, schrieb Paolo Bonzini:
> >Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
> >>On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
> >>>>Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
> >>>>Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
> >>>>
> >>>>i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
> >>>>glad if somebody else beats me.  It can be reproduced with Wine and
> >>>>"x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
> >>Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
> >>but right now I don't have a good network connection enough to either
> >>setup a mingw build environment or to connect to a remote machine with
> >>such an environment.
> >
> >It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
> >analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
> >generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
> >and it segfaults on startup.  Even the previous commit cannot run
> >qemu-x86_64 /bin/ls correctly:
> >
> 
> I just tested with latest qemu-system-x86_64 on 32 bit Linux.
> 
> It also hangs during boot (BIOS), so it looks like this
> is not a MinGW only problem.
> 
> Your test with x86_64-linux-user indicates that, too.
> 
> I also get the problem with TCI. Therefore I expect that any
> 32 bit TCG target will show it.
> 

It ended up to be a merge issue. The newly added special cases
for half-dead operations also need to be changed with the liveness
analysis rework.

The attached patch fixes the issue on a 32-bit linux host. I haven't
tried win32 yet, maybe someone will beat me.

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

[-- Attachment #2: 0001-tcg-don-t-remove-op-if-output-needs-to-be-synced-to-.patch --]
[-- Type: text/x-diff, Size: 1809 bytes --]

>From 8a99a0e875f2de8bf47e6fd27523723176251333 Mon Sep 17 00:00:00 2001
From: Aurelien Jarno <aurelien@aurel32.net>
Date: Wed, 31 Oct 2012 00:50:15 +0100
Subject: [PATCH] tcg: don't remove op if output needs to be synced to memory

Commit 9c43b68de628a1e2cba556adfb71c17028eb802e do not correctly check
for dead outputs when they need to be synced to memory in case of
half-dead operations.

Fix that by applying the same pattern than for the default case.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index c3a7f19..1133438 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1329,8 +1329,8 @@ static void tcg_liveness_analysis(TCGContext *s)
                the low part.  The result can be optimized to a simple
                add or sub.  This happens often for x86_64 guest when the
                cpu mode is set to 32 bit.  */
-            if (dead_temps[args[1]]) {
-                if (dead_temps[args[0]]) {
+            if (dead_temps[args[1]] && !mem_temps[1]) {
+                if (dead_temps[args[0]] && !mem_temps[0]) {
                     goto do_remove;
                 }
                 /* Create the single operation plus nop.  */
@@ -1355,8 +1355,8 @@ static void tcg_liveness_analysis(TCGContext *s)
             nb_iargs = 2;
             nb_oargs = 2;
             /* Likewise, test for the high part of the operation dead.  */
-            if (dead_temps[args[1]]) {
-                if (dead_temps[args[0]]) {
+            if (dead_temps[args[1]] && !mem_temps[1]) {
+                if (dead_temps[args[0]] && !mem_temps[0]) {
                     goto do_remove;
                 }
                 gen_opc_buf[op_index] = op = INDEX_op_mul_i32;
-- 
1.7.2.5


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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-30 23:56       ` Aurelien Jarno
@ 2012-10-31 12:40         ` Aurelien Jarno
  2012-10-31 14:01           ` Paolo Bonzini
  2012-10-31 17:05         ` Stefan Weil
  2012-11-07 13:26         ` Kirill Batuzov
  2 siblings, 1 reply; 15+ messages in thread
From: Aurelien Jarno @ 2012-10-31 12:40 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

On Wed, Oct 31, 2012 at 12:56:36AM +0100, Aurelien Jarno wrote:
> On Tue, Oct 30, 2012 at 11:24:34PM +0100, Stefan Weil wrote:
> > Am 30.10.2012 09:15, schrieb Paolo Bonzini:
> > >Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
> > >>On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
> > >>>>Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
> > >>>>Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
> > >>>>
> > >>>>i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
> > >>>>glad if somebody else beats me.  It can be reproduced with Wine and
> > >>>>"x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
> > >>Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
> > >>but right now I don't have a good network connection enough to either
> > >>setup a mingw build environment or to connect to a remote machine with
> > >>such an environment.
> > >
> > >It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
> > >analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
> > >generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
> > >and it segfaults on startup.  Even the previous commit cannot run
> > >qemu-x86_64 /bin/ls correctly:
> > >
> > 
> > I just tested with latest qemu-system-x86_64 on 32 bit Linux.
> > 
> > It also hangs during boot (BIOS), so it looks like this
> > is not a MinGW only problem.
> > 
> > Your test with x86_64-linux-user indicates that, too.
> > 
> > I also get the problem with TCI. Therefore I expect that any
> > 32 bit TCG target will show it.
> > 
> 
> It ended up to be a merge issue. The newly added special cases
> for half-dead operations also need to be changed with the liveness
> analysis rework.
> 
> The attached patch fixes the issue on a 32-bit linux host. I haven't
> tried win32 yet, maybe someone will beat me.
> 

I have just been able to try, and I confirm it fixes the problem on
win32.

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

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-31 12:40         ` Aurelien Jarno
@ 2012-10-31 14:01           ` Paolo Bonzini
  2012-10-31 14:05             ` Peter Maydell
  0 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2012-10-31 14:01 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Stefan Weil, qemu-devel

Il 31/10/2012 13:40, Aurelien Jarno ha scritto:
>>> I just tested with latest qemu-system-x86_64 on 32 bit Linux.
>>>
>>> It also hangs during boot (BIOS), so it looks like this
>>> is not a MinGW only problem.
>>>
>>> Your test with x86_64-linux-user indicates that, too.
>>>
>>> I also get the problem with TCI. Therefore I expect that any
>>> 32 bit TCG target will show it.
>>>
>>
>> It ended up to be a merge issue. The newly added special cases
>> for half-dead operations also need to be changed with the liveness
>> analysis rework.
>>
>> The attached patch fixes the issue on a 32-bit linux host. I haven't
>> tried win32 yet, maybe someone will beat me.
>>
> 
> I have just been able to try, and I confirm it fixes the problem on
> win32.

Thanks!  In general, do not rebase a branch unless you were able to test
the rebase fully.  Use "git merge" instead.  This does not apply to
people without commit access (unless they use pull requests---perhaps we
should use them more), but it is easy for you.

If you hadn't rebased the series, "git bisect" would have pointed out
that the original series worked, and that the merge was the problem.
(Yes, this is less accurate than knowing *which* patch made invalid
assumptions after the rebase.  However, you can always do a temporary
rebase to find that.  A linearized history doesn't say what was the
latest "master" commit on top of which you tested).

Paolo

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-31 14:01           ` Paolo Bonzini
@ 2012-10-31 14:05             ` Peter Maydell
  2012-10-31 14:08               ` Paolo Bonzini
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2012-10-31 14:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Stefan Weil, qemu-devel, Aurelien Jarno

On 31 October 2012 15:01, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Thanks!  In general, do not rebase a branch unless you were able to test
> the rebase fully.  Use "git merge" instead.  This does not apply to
> people without commit access (unless they use pull requests---perhaps we
> should use them more), but it is easy for you.
>
> If you hadn't rebased the series, "git bisect" would have pointed out
> that the original series worked, and that the merge was the problem.

I don't think this actually gains us anything, because we've still
checked broken code into master, whether it was via a rebase or a
merge. The correct answer is "test your commits before sending them",
surely?

-- PMM

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-31 14:05             ` Peter Maydell
@ 2012-10-31 14:08               ` Paolo Bonzini
  2012-10-31 15:23                 ` Aurelien Jarno
  0 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2012-10-31 14:08 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Stefan Weil, qemu-devel, Aurelien Jarno

Il 31/10/2012 15:05, Peter Maydell ha scritto:
>> > Thanks!  In general, do not rebase a branch unless you were able to test
>> > the rebase fully.  Use "git merge" instead.  This does not apply to
>> > people without commit access (unless they use pull requests---perhaps we
>> > should use them more), but it is easy for you.
>> >
>> > If you hadn't rebased the series, "git bisect" would have pointed out
>> > that the original series worked, and that the merge was the problem.
> I don't think this actually gains us anything, because we've still
> checked broken code into master, whether it was via a rebase or a
> merge. The correct answer is "test your commits before sending them",
> surely?

I think Aurelien did test, but only on a 64-bit host.  So the idea still
holds: if you have tested more than that earlier, do not rebase.

Paolo

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-31 14:08               ` Paolo Bonzini
@ 2012-10-31 15:23                 ` Aurelien Jarno
  0 siblings, 0 replies; 15+ messages in thread
From: Aurelien Jarno @ 2012-10-31 15:23 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Peter Maydell, qemu-devel, Stefan Weil

Le 31/10/2012 15:08, Paolo Bonzini a écrit :
> Il 31/10/2012 15:05, Peter Maydell ha scritto:
>>>> Thanks!  In general, do not rebase a branch unless you were able to test
>>>> the rebase fully.  Use "git merge" instead.  This does not apply to
>>>> people without commit access (unless they use pull requests---perhaps we
>>>> should use them more), but it is easy for you.
>>>>
>>>> If you hadn't rebased the series, "git bisect" would have pointed out
>>>> that the original series worked, and that the merge was the problem.
>> I don't think this actually gains us anything, because we've still
>> checked broken code into master, whether it was via a rebase or a
>> merge. The correct answer is "test your commits before sending them",
>> surely?
> 
> I think Aurelien did test, but only on a 64-bit host.  So the idea still
> holds: if you have tested more than that earlier, do not rebase.

When I said a merge issue, it's not in the sense "git merge issue". Both
series are touching different part of the code, but the resulting
behaviour was wrong.

If we want to do it correctly here, it means that we have to rebase and
test all series each time a single patch is committed. Not really doable.

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

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-30 23:56       ` Aurelien Jarno
  2012-10-31 12:40         ` Aurelien Jarno
@ 2012-10-31 17:05         ` Stefan Weil
  2012-10-31 21:48           ` Aurelien Jarno
  2012-11-07 13:26         ` Kirill Batuzov
  2 siblings, 1 reply; 15+ messages in thread
From: Stefan Weil @ 2012-10-31 17:05 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Paolo Bonzini, qemu-devel

Am 31.10.2012 00:56, schrieb Aurelien Jarno:
> On Tue, Oct 30, 2012 at 11:24:34PM +0100, Stefan Weil wrote:
>> Am 30.10.2012 09:15, schrieb Paolo Bonzini:
>>> Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
>>>> On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
>>>>>> Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
>>>>>> Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
>>>>>>
>>>>>> i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
>>>>>> glad if somebody else beats me.  It can be reproduced with Wine and
>>>>>> "x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
>>>> Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
>>>> but right now I don't have a good network connection enough to either
>>>> setup a mingw build environment or to connect to a remote machine with
>>>> such an environment.
>>> It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
>>> analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
>>> generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
>>> and it segfaults on startup.  Even the previous commit cannot run
>>> qemu-x86_64 /bin/ls correctly:
>>>
>> I just tested with latest qemu-system-x86_64 on 32 bit Linux.
>>
>> It also hangs during boot (BIOS), so it looks like this
>> is not a MinGW only problem.
>>
>> Your test with x86_64-linux-user indicates that, too.
>>
>> I also get the problem with TCI. Therefore I expect that any
>> 32 bit TCG target will show it.
>>
> It ended up to be a merge issue. The newly added special cases
> for half-dead operations also need to be changed with the liveness
> analysis rework.
>
> The attached patch fixes the issue on a 32-bit linux host. I haven't
> tried win32 yet, maybe someone will beat me.


Tested-by: Stefan Weil <sw@weilnetz.de>

Your attached patch fixes the problem seen with qemu-system-x86_64
on 32 bit Linux and with 32 bit Windows.

I think you can commit it to git master.

Merci,
Stefan

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-31 17:05         ` Stefan Weil
@ 2012-10-31 21:48           ` Aurelien Jarno
  0 siblings, 0 replies; 15+ messages in thread
From: Aurelien Jarno @ 2012-10-31 21:48 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

On Wed, Oct 31, 2012 at 06:05:57PM +0100, Stefan Weil wrote:
> Am 31.10.2012 00:56, schrieb Aurelien Jarno:
> >On Tue, Oct 30, 2012 at 11:24:34PM +0100, Stefan Weil wrote:
> >>Am 30.10.2012 09:15, schrieb Paolo Bonzini:
> >>>Il 29/10/2012 19:29, Aurelien Jarno ha scritto:
> >>>>On Mon, Oct 29, 2012 at 06:53:14PM +0100, Paolo Bonzini wrote:
> >>>>>>Known-good commit: 8473f377393219390ea6f2d8d450a2b054bb823e
> >>>>>>Known-bad commit: d262cb02861dd33375c08fc798930653b14769e9
> >>>>>>
> >>>>>>i386-softmmu seems to work.  I may try to bisect it tomorrow, but I'd be
> >>>>>>glad if somebody else beats me.  It can be reproduced with Wine and
> >>>>>>"x86_64-softmmu/qemu-system-x86_64.exe -L ../pc-bios"; it hangs at iPXE.
> >>>>Oops, sorry about that. Is it win32 or win64? I'll try to fix it asap,
> >>>>but right now I don't have a good network connection enough to either
> >>>>setup a mingw build environment or to connect to a remote machine with
> >>>>such an environment.
> >>>It's win32, and the first bad commit is 9c43b68 (tcg: rework liveness
> >>>analysis, 2012-10-09).  But it looks like 64-on-32 emulation is more
> >>>generally broken.  I now tried x86_64-linux-user compiled for 32-bit,
> >>>and it segfaults on startup.  Even the previous commit cannot run
> >>>qemu-x86_64 /bin/ls correctly:
> >>>
> >>I just tested with latest qemu-system-x86_64 on 32 bit Linux.
> >>
> >>It also hangs during boot (BIOS), so it looks like this
> >>is not a MinGW only problem.
> >>
> >>Your test with x86_64-linux-user indicates that, too.
> >>
> >>I also get the problem with TCI. Therefore I expect that any
> >>32 bit TCG target will show it.
> >>
> >It ended up to be a merge issue. The newly added special cases
> >for half-dead operations also need to be changed with the liveness
> >analysis rework.
> >
> >The attached patch fixes the issue on a 32-bit linux host. I haven't
> >tried win32 yet, maybe someone will beat me.
> 
> 
> Tested-by: Stefan Weil <sw@weilnetz.de>
> 
> Your attached patch fixes the problem seen with qemu-system-x86_64
> on 32 bit Linux and with 32 bit Windows.
> 
> I think you can commit it to git master.
> 

Done. Thanks for the test.

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

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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-10-30 23:56       ` Aurelien Jarno
  2012-10-31 12:40         ` Aurelien Jarno
  2012-10-31 17:05         ` Stefan Weil
@ 2012-11-07 13:26         ` Kirill Batuzov
  2012-11-11 16:05           ` Aurelien Jarno
  2 siblings, 1 reply; 15+ messages in thread
From: Kirill Batuzov @ 2012-11-07 13:26 UTC (permalink / raw)
  To: Aurelien Jarno; +Cc: Stefan Weil, qemu-devel, Paolo Bonzini

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1909 bytes --]

> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index c3a7f19..1133438 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1329,8 +1329,8 @@ static void tcg_liveness_analysis(TCGContext *s)
>                 the low part.  The result can be optimized to a simple
>                 add or sub.  This happens often for x86_64 guest when the
>                 cpu mode is set to 32 bit.  */
> -            if (dead_temps[args[1]]) {
> -                if (dead_temps[args[0]]) {
> +            if (dead_temps[args[1]] && !mem_temps[1]) {
> +                if (dead_temps[args[0]] && !mem_temps[0]) {

This should be mem_temps[args[1]] and mem_temps[args[0]] I believe.

>                      goto do_remove;
>                  }
>                  /* Create the single operation plus nop.  */
> @@ -1355,8 +1355,8 @@ static void tcg_liveness_analysis(TCGContext *s)
>              nb_iargs = 2;
>              nb_oargs = 2;
>              /* Likewise, test for the high part of the operation dead.  */
> -            if (dead_temps[args[1]]) {
> -                if (dead_temps[args[0]]) {
> +            if (dead_temps[args[1]] && !mem_temps[1]) {
> +                if (dead_temps[args[0]] && !mem_temps[0]) {

Same here.

>                      goto do_remove;
>                  }
>                  gen_opc_buf[op_index] = op = INDEX_op_mul_i32;

Looks like for x86_64 guest temp 0 is the env (always mem_temp), temp 1 -
cc_op. As a result it can accidentally remove high part of operation
when it is actually alive but will never optimize out whole operation
even if its output is really dead.

I've attached a small patch to fix this issue.

I was not able to boot gentoo install CD (amd64) with current trunk.
Boot process hangs soon after framebuffer initialization. With the patch
it boots successfully. Command line to reproduce:

qemu-system-x86_64 -cdrom install-amd64-minimal-20121013.iso

-- 
Kirill Batuzov

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=tcg-properly-check-op-s-output.patch, Size: 1843 bytes --]

From 33e1fc03934cebea8d32c98ea34961c80f05d94a Mon Sep 17 00:00:00 2001
From: Kirill Batuzov <batuzovk@ispras.ru>
Date: Wed, 7 Nov 2012 15:26:38 +0400
Subject: [PATCH] tcg: properly check that op's output needs to be synced to
 memory

Fix typo introduced in b3a1be87bac3a6aaa59bb88c1410f170dc9b22d5.

Reported-by: Ruslan Savchenko <ruslan.savchenko@gmail.com>
Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
---
 tcg/tcg.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 42052db..35fba50 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1337,8 +1337,8 @@ static void tcg_liveness_analysis(TCGContext *s)
                the low part.  The result can be optimized to a simple
                add or sub.  This happens often for x86_64 guest when the
                cpu mode is set to 32 bit.  */
-            if (dead_temps[args[1]] && !mem_temps[1]) {
-                if (dead_temps[args[0]] && !mem_temps[0]) {
+            if (dead_temps[args[1]] && !mem_temps[args[1]]) {
+                if (dead_temps[args[0]] && !mem_temps[args[0]]) {
                     goto do_remove;
                 }
                 /* Create the single operation plus nop.  */
@@ -1363,8 +1363,8 @@ static void tcg_liveness_analysis(TCGContext *s)
             nb_iargs = 2;
             nb_oargs = 2;
             /* Likewise, test for the high part of the operation dead.  */
-            if (dead_temps[args[1]] && !mem_temps[1]) {
-                if (dead_temps[args[0]] && !mem_temps[0]) {
+            if (dead_temps[args[1]] && !mem_temps[args[1]]) {
+                if (dead_temps[args[0]] && !mem_temps[args[0]]) {
                     goto do_remove;
                 }
                 gen_opc_buf[op_index] = op = INDEX_op_mul_i32;
-- 
1.7.9.5


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

* Re: [Qemu-devel] 64-on-32 TCG broken
  2012-11-07 13:26         ` Kirill Batuzov
@ 2012-11-11 16:05           ` Aurelien Jarno
  0 siblings, 0 replies; 15+ messages in thread
From: Aurelien Jarno @ 2012-11-11 16:05 UTC (permalink / raw)
  To: Kirill Batuzov; +Cc: Stefan Weil, qemu-devel, Paolo Bonzini

On Wed, Nov 07, 2012 at 05:26:58PM +0400, Kirill Batuzov wrote:
> > diff --git a/tcg/tcg.c b/tcg/tcg.c
> > index c3a7f19..1133438 100644
> > --- a/tcg/tcg.c
> > +++ b/tcg/tcg.c
> > @@ -1329,8 +1329,8 @@ static void tcg_liveness_analysis(TCGContext *s)
> >                 the low part.  The result can be optimized to a simple
> >                 add or sub.  This happens often for x86_64 guest when the
> >                 cpu mode is set to 32 bit.  */
> > -            if (dead_temps[args[1]]) {
> > -                if (dead_temps[args[0]]) {
> > +            if (dead_temps[args[1]] && !mem_temps[1]) {
> > +                if (dead_temps[args[0]] && !mem_temps[0]) {
> 
> This should be mem_temps[args[1]] and mem_temps[args[0]] I believe.
> 
> >                      goto do_remove;
> >                  }
> >                  /* Create the single operation plus nop.  */
> > @@ -1355,8 +1355,8 @@ static void tcg_liveness_analysis(TCGContext *s)
> >              nb_iargs = 2;
> >              nb_oargs = 2;
> >              /* Likewise, test for the high part of the operation dead.  */
> > -            if (dead_temps[args[1]]) {
> > -                if (dead_temps[args[0]]) {
> > +            if (dead_temps[args[1]] && !mem_temps[1]) {
> > +                if (dead_temps[args[0]] && !mem_temps[0]) {
> 
> Same here.
> 
> >                      goto do_remove;
> >                  }
> >                  gen_opc_buf[op_index] = op = INDEX_op_mul_i32;
> 
> Looks like for x86_64 guest temp 0 is the env (always mem_temp), temp 1 -
> cc_op. As a result it can accidentally remove high part of operation
> when it is actually alive but will never optimize out whole operation
> even if its output is really dead.
> 
> I've attached a small patch to fix this issue.
> 
> I was not able to boot gentoo install CD (amd64) with current trunk.
> Boot process hangs soon after framebuffer initialization. With the patch
> it boots successfully. Command line to reproduce:
> 
> qemu-system-x86_64 -cdrom install-amd64-minimal-20121013.iso
> 
> -- 
> Kirill Batuzov

> From 33e1fc03934cebea8d32c98ea34961c80f05d94a Mon Sep 17 00:00:00 2001
> From: Kirill Batuzov <batuzovk@ispras.ru>
> Date: Wed, 7 Nov 2012 15:26:38 +0400
> Subject: [PATCH] tcg: properly check that op's output needs to be synced to
>  memory
> 
> Fix typo introduced in b3a1be87bac3a6aaa59bb88c1410f170dc9b22d5.
> 
> Reported-by: Ruslan Savchenko <ruslan.savchenko@gmail.com>
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
> ---
>  tcg/tcg.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 42052db..35fba50 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1337,8 +1337,8 @@ static void tcg_liveness_analysis(TCGContext *s)
>                 the low part.  The result can be optimized to a simple
>                 add or sub.  This happens often for x86_64 guest when the
>                 cpu mode is set to 32 bit.  */
> -            if (dead_temps[args[1]] && !mem_temps[1]) {
> -                if (dead_temps[args[0]] && !mem_temps[0]) {
> +            if (dead_temps[args[1]] && !mem_temps[args[1]]) {
> +                if (dead_temps[args[0]] && !mem_temps[args[0]]) {
>                      goto do_remove;
>                  }
>                  /* Create the single operation plus nop.  */
> @@ -1363,8 +1363,8 @@ static void tcg_liveness_analysis(TCGContext *s)
>              nb_iargs = 2;
>              nb_oargs = 2;
>              /* Likewise, test for the high part of the operation dead.  */
> -            if (dead_temps[args[1]] && !mem_temps[1]) {
> -                if (dead_temps[args[0]] && !mem_temps[0]) {
> +            if (dead_temps[args[1]] && !mem_temps[args[1]]) {
> +                if (dead_temps[args[0]] && !mem_temps[args[0]]) {
>                      goto do_remove;
>                  }
>                  gen_opc_buf[op_index] = op = INDEX_op_mul_i32;

Thanks, applied.

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

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

end of thread, other threads:[~2012-11-11 16:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-29 17:53 [Qemu-devel] x86_64-softmmu broken on Windows (TCG?) Paolo Bonzini
2012-10-29 18:29 ` Aurelien Jarno
2012-10-30  8:15   ` [Qemu-devel] 64-on-32 TCG broken [was Re: x86_64-softmmu broken on Windows (TCG?)] Paolo Bonzini
2012-10-30 22:24     ` [Qemu-devel] 64-on-32 TCG broken Stefan Weil
2012-10-30 23:22       ` Aurelien Jarno
2012-10-30 23:56       ` Aurelien Jarno
2012-10-31 12:40         ` Aurelien Jarno
2012-10-31 14:01           ` Paolo Bonzini
2012-10-31 14:05             ` Peter Maydell
2012-10-31 14:08               ` Paolo Bonzini
2012-10-31 15:23                 ` Aurelien Jarno
2012-10-31 17:05         ` Stefan Weil
2012-10-31 21:48           ` Aurelien Jarno
2012-11-07 13:26         ` Kirill Batuzov
2012-11-11 16:05           ` Aurelien Jarno

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).