[Qemu-devel] [PATCH 1/5] Fix i386 Host

[Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

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

This patch is mostly a cleanup of Michael Matz's patch with the ideas
that came last time included.


[-- Attachment #2: qemu-gcc4-i386.patch --]
[-- Type: text/x-patch, Size: 6040 bytes --]

Index: qemu/softmmu_header.h
===================================================================
--- qemu.orig/softmmu_header.h
+++ qemu/softmmu_header.h
@@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+                  "pushl %%ecx\n"
                   "pushl %6\n"
                   "call %7\n"
                   "popl %%eax\n"
+                  "popl %%ecx\n"
                   "jmp 2f\n"
                   "1:\n"
                   "addl 8(%%edx), %%eax\n"
@@ -209,14 +211,18 @@ static inline void glue(glue(st, SUFFIX)
                   : "r" (ptr),
 /* NOTE: 'q' would be needed as constraint, but we could not use it
    with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+                  "q" (v),
+#else
                   "r" (v),
+#endif
                   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
                   "m" (*(uint32_t *)offsetof(CPUState, tlb_table[CPU_MMU_INDEX][0].addr_write)),
                   "i" (CPU_MMU_INDEX),
                   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
-                  : "%eax", "%ecx", "%edx", "memory", "cc");
+                  : "%eax", "%edx", "memory", "cc");
 }
 
 #else
Index: qemu/target-alpha/cpu.h
===================================================================
--- qemu.orig/target-alpha/cpu.h
+++ qemu/target-alpha/cpu.h
@@ -275,6 +275,8 @@ struct CPUAlphaState {
      * used to emulate 64 bits target on 32 bits hosts
      */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     /* */
     double ft0, ft1, ft2;
Index: qemu/target-alpha/exec.h
===================================================================
--- qemu.orig/target-alpha/exec.h
+++ qemu/target-alpha/exec.h
@@ -40,7 +40,11 @@ register struct CPUAlphaState *env asm(A
 
 register uint64_t T0 asm(AREG1);
 register uint64_t T1 asm(AREG2);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register uint64_t T2 asm(AREG3);
+#endif
 
 #endif /* TARGET_LONG_BITS > HOST_LONG_BITS */
 
Index: qemu/target-arm/cpu.h
===================================================================
--- qemu.orig/target-arm/cpu.h
+++ qemu/target-arm/cpu.h
@@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
  */
 
 typedef struct CPUARMState {
+#if defined(GCC_BREAKS_T_REGISTER)
+    uint32_t t2;
+#endif
     /* Regs for current mode.  */
     uint32_t regs[16];
     /* Frequently accessed CPSR bits are stored separately for efficiently.
Index: qemu/target-arm/exec.h
===================================================================
--- qemu.orig/target-arm/exec.h
+++ qemu/target-arm/exec.h
@@ -23,7 +23,12 @@
 register struct CPUARMState *env asm(AREG0);
 register uint32_t T0 asm(AREG1);
 register uint32_t T1 asm(AREG2);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register uint32_t T2 asm(AREG3);
+#endif
+
 
 /* TODO: Put these in FP regs on targets that have such things.  */
 /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.  */
Index: qemu/target-i386/cpu.h
===================================================================
--- qemu.orig/target-i386/cpu.h
+++ qemu/target-i386/cpu.h
@@ -470,6 +470,8 @@ typedef struct CPUX86State {
 #if TARGET_LONG_BITS > HOST_LONG_BITS
     /* temporaries if we cannot store them in host registers */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t1;
 #endif
 
     /* standard registers */
Index: qemu/target-i386/exec.h
===================================================================
--- qemu.orig/target-i386/exec.h
+++ qemu/target-i386/exec.h
@@ -44,7 +44,11 @@ register struct CPUX86State *env asm(ARE
 /* XXX: use unsigned long instead of target_ulong - better code will
    be generated for 64 bit CPUs */
 register target_ulong T0 asm(AREG1);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T1 (env->t1)
+#else
 register target_ulong T1 asm(AREG2);
+#endif
 register target_ulong T2 asm(AREG3);
 
 /* if more registers are available, we define some registers too */
Index: qemu/target-mips/cpu.h
===================================================================
--- qemu.orig/target-mips/cpu.h
+++ qemu/target-mips/cpu.h
@@ -149,6 +149,8 @@ struct CPUMIPSState {
     target_ulong t0;
     target_ulong t1;
     target_ulong t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
     target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
Index: qemu/target-mips/exec.h
===================================================================
--- qemu.orig/target-mips/exec.h
+++ qemu/target-mips/exec.h
@@ -17,8 +17,12 @@ register struct CPUMIPSState *env asm(AR
 #else
 register target_ulong T0 asm(AREG1);
 register target_ulong T1 asm(AREG2);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register target_ulong T2 asm(AREG3);
 #endif
+#endif
 
 #if defined (USE_HOST_FLOAT_REGS)
 #error "implement me."
Index: qemu/target-ppc/exec.h
===================================================================
--- qemu.orig/target-ppc/exec.h
+++ qemu/target-ppc/exec.h
@@ -41,7 +41,11 @@ register struct CPUPPCState *env asm(ARE
 #else
 register unsigned long T0 asm(AREG1);
 register unsigned long T1 asm(AREG2);
+#if GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register unsigned long T2 asm(AREG3);
+#endif
 #define TDX "%016lx"
 #endif
 /* We may, sometime, need 64 bits registers on 32 bits targets */
Index: qemu/target-sparc/exec.h
===================================================================
--- qemu.orig/target-sparc/exec.h
+++ qemu/target-sparc/exec.h
@@ -32,10 +32,14 @@ register uint32_t T2 asm(AREG4);
 
 #else
 #define REGWPTR env->regwptr
+#ifdef HOST_I386
+#define T2 (env->t2)
+#else
 register uint32_t T2 asm(AREG3);
-#endif
 #define reg_T2
 #endif
+#endif
+#endif
 
 #define FT0 (env->ft0)
 #define FT1 (env->ft1)

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

Johannes Schindelin wrote:
> Hi,
>
> On Thu, 17 Jan 2008, Alexander Graf wrote:
>
>   
>> This patch is mostly a cleanup of Michael Matz's patch with the ideas 
>> that came last time included.
>>     
>
> Maybe the patch subject could be "prepare i386 host for gcc4, step 1)?
>
>   

Basically it fixes it, just not completely ;-).

>> Index: qemu/softmmu_header.h
>> ===================================================================
>> --- qemu.orig/softmmu_header.h
>> +++ qemu/softmmu_header.h
>> @@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
>>  #else
>>  #error unsupported size
>>  #endif
>> +                  "pushl %%ecx\n"
>>                    "pushl %6\n"
>>                    "call %7\n"
>>                    "popl %%eax\n"
>> +                  "popl %%ecx\n"
>>     
>
> Could you guard this so that this code is only included for gcc4?  (I am 
> assuming that it is not needed for gcc3, as it worked so far...)
>
>   
You're right on that one. Thanks for the catch.

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

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

Alexander Graf wrote:
> This patch is mostly a cleanup of Michael Matz's patch with the ideas
> that came last time included.
>
>   
I didn't include one file in the previous patch, sorry.
This patch also includes Johannes' catch to use #ifdefs around most changes.

[-- Attachment #2: qemu-gcc4-i386.patch --]
[-- Type: text/x-patch, Size: 6523 bytes --]

Index: qemu/softmmu_header.h
===================================================================
--- qemu.orig/softmmu_header.h
+++ qemu/softmmu_header.h
@@ -189,9 +189,15 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+#ifdef GCC_BREAKS_T_REGISTER
+                  "pushl %%ecx\n"
+#endif
                   "pushl %6\n"
                   "call %7\n"
                   "popl %%eax\n"
+#ifdef GCC_BREAKS_T_REGISTER
+                  "popl %%ecx\n"
+#endif
                   "jmp 2f\n"
                   "1:\n"
                   "addl 8(%%edx), %%eax\n"
@@ -209,14 +215,22 @@ static inline void glue(glue(st, SUFFIX)
                   : "r" (ptr),
 /* NOTE: 'q' would be needed as constraint, but we could not use it
    with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+                  "q" (v),
+#else
                   "r" (v),
+#endif
                   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
                   "m" (*(uint32_t *)offsetof(CPUState, tlb_table[CPU_MMU_INDEX][0].addr_write)),
                   "i" (CPU_MMU_INDEX),
                   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
+#ifdef GCC_BREAKS_T_REGISTER
+                  : "%eax", "%edx", "memory", "cc");
+#else
                   : "%eax", "%ecx", "%edx", "memory", "cc");
+#endif
 }
 
 #else
Index: qemu/target-alpha/cpu.h
===================================================================
--- qemu.orig/target-alpha/cpu.h
+++ qemu/target-alpha/cpu.h
@@ -275,6 +275,8 @@ struct CPUAlphaState {
      * used to emulate 64 bits target on 32 bits hosts
      */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     /* */
     double ft0, ft1, ft2;
Index: qemu/target-alpha/exec.h
===================================================================
--- qemu.orig/target-alpha/exec.h
+++ qemu/target-alpha/exec.h
@@ -40,7 +40,11 @@ register struct CPUAlphaState *env asm(A
 
 register uint64_t T0 asm(AREG1);
 register uint64_t T1 asm(AREG2);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register uint64_t T2 asm(AREG3);
+#endif
 
 #endif /* TARGET_LONG_BITS > HOST_LONG_BITS */
 
Index: qemu/target-arm/cpu.h
===================================================================
--- qemu.orig/target-arm/cpu.h
+++ qemu/target-arm/cpu.h
@@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
  */
 
 typedef struct CPUARMState {
+#if defined(GCC_BREAKS_T_REGISTER)
+    uint32_t t2;
+#endif
     /* Regs for current mode.  */
     uint32_t regs[16];
     /* Frequently accessed CPSR bits are stored separately for efficiently.
Index: qemu/target-arm/exec.h
===================================================================
--- qemu.orig/target-arm/exec.h
+++ qemu/target-arm/exec.h
@@ -23,7 +23,12 @@
 register struct CPUARMState *env asm(AREG0);
 register uint32_t T0 asm(AREG1);
 register uint32_t T1 asm(AREG2);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register uint32_t T2 asm(AREG3);
+#endif
+
 
 /* TODO: Put these in FP regs on targets that have such things.  */
 /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.  */
Index: qemu/target-i386/cpu.h
===================================================================
--- qemu.orig/target-i386/cpu.h
+++ qemu/target-i386/cpu.h
@@ -470,6 +470,8 @@ typedef struct CPUX86State {
 #if TARGET_LONG_BITS > HOST_LONG_BITS
     /* temporaries if we cannot store them in host registers */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t1;
 #endif
 
     /* standard registers */
Index: qemu/target-i386/exec.h
===================================================================
--- qemu.orig/target-i386/exec.h
+++ qemu/target-i386/exec.h
@@ -44,7 +44,11 @@ register struct CPUX86State *env asm(ARE
 /* XXX: use unsigned long instead of target_ulong - better code will
    be generated for 64 bit CPUs */
 register target_ulong T0 asm(AREG1);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T1 (env->t1)
+#else
 register target_ulong T1 asm(AREG2);
+#endif
 register target_ulong T2 asm(AREG3);
 
 /* if more registers are available, we define some registers too */
Index: qemu/target-mips/cpu.h
===================================================================
--- qemu.orig/target-mips/cpu.h
+++ qemu/target-mips/cpu.h
@@ -149,6 +149,8 @@ struct CPUMIPSState {
     target_ulong t0;
     target_ulong t1;
     target_ulong t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
     target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
Index: qemu/target-mips/exec.h
===================================================================
--- qemu.orig/target-mips/exec.h
+++ qemu/target-mips/exec.h
@@ -17,8 +17,12 @@ register struct CPUMIPSState *env asm(AR
 #else
 register target_ulong T0 asm(AREG1);
 register target_ulong T1 asm(AREG2);
+#ifdef GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register target_ulong T2 asm(AREG3);
 #endif
+#endif
 
 #if defined (USE_HOST_FLOAT_REGS)
 #error "implement me."
Index: qemu/target-ppc/exec.h
===================================================================
--- qemu.orig/target-ppc/exec.h
+++ qemu/target-ppc/exec.h
@@ -41,7 +41,11 @@ register struct CPUPPCState *env asm(ARE
 #else
 register unsigned long T0 asm(AREG1);
 register unsigned long T1 asm(AREG2);
+#if GCC_BREAKS_T_REGISTER
+#define T2 (env->t2)
+#else
 register unsigned long T2 asm(AREG3);
+#endif
 #define TDX "%016lx"
 #endif
 /* We may, sometime, need 64 bits registers on 32 bits targets */
Index: qemu/target-sparc/exec.h
===================================================================
--- qemu.orig/target-sparc/exec.h
+++ qemu/target-sparc/exec.h
@@ -32,10 +32,14 @@ register uint32_t T2 asm(AREG4);
 
 #else
 #define REGWPTR env->regwptr
+#ifdef HOST_I386
+#define T2 (env->t2)
+#else
 register uint32_t T2 asm(AREG3);
-#endif
 #define reg_T2
 #endif
+#endif
+#endif
 
 #define FT0 (env->ft0)
 #define FT1 (env->ft1)
Index: qemu/dyngen-exec.h
===================================================================
--- qemu.orig/dyngen-exec.h
+++ qemu/dyngen-exec.h
@@ -90,6 +90,9 @@ extern int printf(const char *, ...);
 #define NULL 0
 
 #if defined(__i386__)
+#if __GNUC__ > 3
+#define GCC_BREAKS_T_REGISTER
+#endif
 #define AREG0 "ebp"
 #define AREG1 "ebx"
 #define AREG2 "esi"

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

Jens Arm wrote:
> Hi
>
> With this I can compile, but qemu segfaults now when trying to start a ros image:
>
> Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
> [Thread debugging using libthread_db enabled]
> [New Thread -1212344640 (LWP 18268)]
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread -1212344640 (LWP 18268)]
> 0x08bcb3b4 in code_gen_buffer ()
> (gdb) bt
> #0  0x08bcb3b4 in code_gen_buffer ()
> #1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
> #2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445
>
> Any hints?
>
>   

Looks like it's still broken. This can only be something minor, as it
did work for me in between, but now I can reproduce you failure here as
well. Stay tuned.

Alex

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, Alexander Graf wrote:

> This patch is mostly a cleanup of Michael Matz's patch with the ideas 
> that came last time included.

Maybe the patch subject could be "prepare i386 host for gcc4, step 1)?

> Index: qemu/softmmu_header.h
> ===================================================================
> --- qemu.orig/softmmu_header.h
> +++ qemu/softmmu_header.h
> @@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
>  #else
>  #error unsupported size
>  #endif
> +                  "pushl %%ecx\n"
>                    "pushl %6\n"
>                    "call %7\n"
>                    "popl %%eax\n"
> +                  "popl %%ecx\n"

Could you guard this so that this code is only included for gcc4?  (I am 
assuming that it is not needed for gcc3, as it worked so far...)

AFAICT the rest is okay, even if it took me (stupidly!) quite some time to 
realize why it is okay for t2 to have different sizes with different 
targets.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Jens Arm]

Hi

I tried to compile on Debian 4.0, but I get this error:

gcc -Wall -O2 -g -fno-strict-aliasing  -fno-reorder-blocks  -fno-gcse  -fno-tree-ch  -fno-optimize-sibling-calls  -fno-crossjumping  -fno-align-labels  -fno-align-jumps  -fno-align-functions   -mpreferred-stack-boundary=2 -fomit-frame-pointer   -I. -I.. -I/home/tux/compile/qemu/target-i386 -I/home/tux/compile/qemu -MMD -MP -DNEED_CPU_H -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/home/tux/compile/qemu/fpu  -DHAS_AUDIO -DHAS_AUDIO_CHOICE -I/home/tux/compile/qemu/slirp -c -o op.o /home/tux/compile/qemu/target-i386/op.c
/home/tux/compile/qemu/target-i386/op.c: In function ‘op_fstl_ST0_A0’:
/home/tux/compile/qemu/target-i386/op.c:1925: error: unable to find a register to spill in class ‘GENERAL_REGS’
/home/tux/compile/qemu/target-i386/op.c:1925: error: this is the insn:
(insn:HI 55 54 56 2 ../cpu-all.h:372 (set (mem:DI (plus:SI (reg/v:SI 60 [ addr ])
                (reg:SI 84 [ <variable>.addend ])) [0 S8 A64])
        (reg/v:DI 63 [ u ])) 80 {*movdi_2} (insn_list:REG_DEP_TRUE 54 (nil))
    (expr_list:REG_DEAD (reg/v:DI 63 [ u ])
        (expr_list:REG_DEAD (reg/v:SI 60 [ addr ])
            (expr_list:REG_DEAD (reg:SI 84 [ <variable>.addend ])
                (nil)))))
/home/tux/compile/qemu/target-i386/op.c:1925: confused by earlier errors, bailing out

I use "gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)".

Qemu is configured like this: ./configure --disable-gcc-check --disable-darwin-user --disable-linux-user --target-list=i386-softmmu

Hope this helps.

Jens


On Thu, 17 Jan 2008 08:54:32 +0100
Alexander Graf <alex@csgraf.de> wrote:

> Johannes Schindelin wrote:
> > Hi,
> >
> > On Thu, 17 Jan 2008, Alexander Graf wrote:
> >
> >   
> >> This patch is mostly a cleanup of Michael Matz's patch with the ideas 
> >> that came last time included.
> >>     
> >
> > Maybe the patch subject could be "prepare i386 host for gcc4, step 1)?
> >
> >   
> 
> Basically it fixes it, just not completely ;-).
> 
> >> Index: qemu/softmmu_header.h
> >> ===================================================================
> >> --- qemu.orig/softmmu_header.h
> >> +++ qemu/softmmu_header.h
> >> @@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
> >>  #else
> >>  #error unsupported size
> >>  #endif
> >> +                  "pushl %%ecx\n"
> >>                    "pushl %6\n"
> >>                    "call %7\n"
> >>                    "popl %%eax\n"
> >> +                  "popl %%ecx\n"
> >>     
> >
> > Could you guard this so that this code is only included for gcc4?  (I am 
> > assuming that it is not needed for gcc3, as it worked so far...)
> >
> >   
> You're right on that one. Thanks for the catch.
> 
> 

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Jens Arm]

Hi

With this I can compile, but qemu segfaults now when trying to start a ros image:

Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
[Thread debugging using libthread_db enabled]
[New Thread -1212344640 (LWP 18268)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1212344640 (LWP 18268)]
0x08bcb3b4 in code_gen_buffer ()
(gdb) bt
#0  0x08bcb3b4 in code_gen_buffer ()
#1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
#2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445

Any hints?

Jens



On Thu, 17 Jan 2008 09:10:07 +0100
Alexander Graf <alex@csgraf.de> wrote:

> Alexander Graf wrote:
> > This patch is mostly a cleanup of Michael Matz's patch with the ideas
> > that came last time included.
> >
> >   
> I didn't include one file in the previous patch, sorry.
> This patch also includes Johannes' catch to use #ifdefs around most changes.
> 

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi Alexander,

On Thu, 17 Jan 2008, Alexander Graf wrote:

> Johannes Schindelin wrote:
> > Hi,
> >
> > On Thu, 17 Jan 2008, Alexander Graf wrote:
> >
> >> This patch is mostly a cleanup of Michael Matz's patch with the ideas 
> >> that came last time included.
> >
> > Maybe the patch subject could be "prepare i386 host for gcc4, step 1)?
> 
> Basically it fixes it, just not completely ;-).

Heh.  But reading the commit logs, I like to know what's going on from the 
subject, without delving into the patch ;-)

> >> Index: qemu/softmmu_header.h
> >> ===================================================================
> >> --- qemu.orig/softmmu_header.h
> >> +++ qemu/softmmu_header.h
> >> @@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
> >>  #else
> >>  #error unsupported size
> >>  #endif
> >> +                  "pushl %%ecx\n"
> >>                    "pushl %6\n"
> >>                    "call %7\n"
> >>                    "popl %%eax\n"
> >> +                  "popl %%ecx\n"
> >>     
> >
> > Could you guard this so that this code is only included for gcc4?  (I 
> > am assuming that it is not needed for gcc3, as it worked so far...)
>
> You're right on that one. Thanks for the catch.

Hey, I'm glad that you picked up the hard work!  And if i386 still works 
as before, am even gladder ;-)

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, Alexander Graf wrote:

> Alexander Graf wrote:
> > This patch is mostly a cleanup of Michael Matz's patch with the ideas
> > that came last time included.
>
> I didn't include one file in the previous patch, sorry. This patch also 
> includes Johannes' catch to use #ifdefs around most changes.

Thanks.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Thiemo Seufer]

Alexander Graf wrote:
[snip]
> Index: qemu/target-alpha/cpu.h
> ===================================================================
> --- qemu.orig/target-alpha/cpu.h
> +++ qemu/target-alpha/cpu.h
> @@ -275,6 +275,8 @@ struct CPUAlphaState {
>       * used to emulate 64 bits target on 32 bits hosts
>       */
>      target_ulong t0, t1, t2;
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +    target_ulong t2;
>  #endif
>      /* */
>      double ft0, ft1, ft2;
> Index: qemu/target-alpha/exec.h
> ===================================================================
> --- qemu.orig/target-alpha/exec.h
> +++ qemu/target-alpha/exec.h
> @@ -40,7 +40,11 @@ register struct CPUAlphaState *env asm(A
>  
>  register uint64_t T0 asm(AREG1);
>  register uint64_t T1 asm(AREG2);
> +#ifdef GCC_BREAKS_T_REGISTER
> +#define T2 (env->t2)
> +#else
>  register uint64_t T2 asm(AREG3);
> +#endif
>  
>  #endif /* TARGET_LONG_BITS > HOST_LONG_BITS */

Please make the #ifdef'ery in cpu.h look the same as in exec.h.

[snip]
> Index: qemu/target-sparc/exec.h
> ===================================================================
> --- qemu.orig/target-sparc/exec.h
> +++ qemu/target-sparc/exec.h
> @@ -32,10 +32,14 @@ register uint32_t T2 asm(AREG4);
>  
>  #else
>  #define REGWPTR env->regwptr
> +#ifdef HOST_I386
> +#define T2 (env->t2)
> +#else
>  register uint32_t T2 asm(AREG3);
> -#endif
>  #define reg_T2
>  #endif
> +#endif
> +#endif

Should also use GCC_BREAKS_T_REGISTER.


Thiemo

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

I get an error with sh4-linux-user and sh4eb-linux-user:

../dyngen -o op.h op.o
dyngen: ret or jmp expected at the end of op_cmp_str_T0_T1

There is a "RETURN();" statement at the end of 
target-sh4/op.c:op_cmp_str_T0_T1() though, so I am puzzled...

It also says "target-ppc/exec.h:44:26: error: #if with no expression", 
because there is an "#if GCC_BREAKS_T_REGISTER" which wants to be an 
"#ifdef".  (This is in patch 1/5.)

Off to lunch, will report on the progress later,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

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

Alexander Graf wrote:
> Jens Arm wrote:
>   
>> Hi
>>
>> With this I can compile, but qemu segfaults now when trying to start a ros image:
>>
>> Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
>> [Thread debugging using libthread_db enabled]
>> [New Thread -1212344640 (LWP 18268)]
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> [Switching to Thread -1212344640 (LWP 18268)]
>> 0x08bcb3b4 in code_gen_buffer ()
>> (gdb) bt
>> #0  0x08bcb3b4 in code_gen_buffer ()
>> #1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
>> #2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445
>>
>> Any hints?
>>
>>   
>>     
>
> Looks like it's still broken. This can only be something minor, as it
> did work for me in between, but now I can reproduce you failure here as
> well. Stay tuned.
>
> Alex
>
>
>   
Oh well, take 3 it is then.

This patch should work for most targets now. All comments so far should
be integrated too, so if you're missing something you said, please tell me.

Alex

[-- Attachment #2: qemu-gcc4-i386.patch --]
[-- Type: text/x-patch, Size: 6947 bytes --]

Index: qemu/softmmu_header.h
===================================================================
--- qemu.orig/softmmu_header.h
+++ qemu/softmmu_header.h
@@ -189,9 +189,15 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+#ifdef GCC_BREAKS_T_REGISTER
+                  "pushl %%ecx\n"
+#endif
                   "pushl %6\n"
                   "call %7\n"
                   "popl %%eax\n"
+#ifdef GCC_BREAKS_T_REGISTER
+                  "popl %%ecx\n"
+#endif
                   "jmp 2f\n"
                   "1:\n"
                   "addl 8(%%edx), %%eax\n"
@@ -209,14 +215,22 @@ static inline void glue(glue(st, SUFFIX)
                   : "r" (ptr),
 /* NOTE: 'q' would be needed as constraint, but we could not use it
    with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+                  "q" (v),
+#else
                   "r" (v),
+#endif
                   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
                   "m" (*(uint32_t *)offsetof(CPUState, tlb_table[CPU_MMU_INDEX][0].addr_write)),
                   "i" (CPU_MMU_INDEX),
                   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
+#ifdef GCC_BREAKS_T_REGISTER
+                  : "%eax", "%edx", "memory", "cc");
+#else
                   : "%eax", "%ecx", "%edx", "memory", "cc");
+#endif
 }
 
 #else
Index: qemu/target-alpha/cpu.h
===================================================================
--- qemu.orig/target-alpha/cpu.h
+++ qemu/target-alpha/cpu.h
@@ -275,6 +275,8 @@ struct CPUAlphaState {
      * used to emulate 64 bits target on 32 bits hosts
      */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     /* */
     double ft0, ft1, ft2;
Index: qemu/target-alpha/exec.h
===================================================================
--- qemu.orig/target-alpha/exec.h
+++ qemu/target-alpha/exec.h
@@ -36,6 +36,12 @@ register struct CPUAlphaState *env asm(A
 #define T1 (env->t1)
 #define T2 (env->t2)
 
+#elif defined(GCC_BREAKS_T_REGISTER)
+
+register uint64_t T0 asm(AREG1);
+register uint64_t T1 asm(AREG2);
+#define T2 (env->t2)
+
 #else
 
 register uint64_t T0 asm(AREG1);
Index: qemu/target-arm/cpu.h
===================================================================
--- qemu.orig/target-arm/cpu.h
+++ qemu/target-arm/cpu.h
@@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
  */
 
 typedef struct CPUARMState {
+#if defined(GCC_BREAKS_T_REGISTER)
+    uint32_t t2;
+#endif
     /* Regs for current mode.  */
     uint32_t regs[16];
     /* Frequently accessed CPSR bits are stored separately for efficiently.
Index: qemu/target-arm/exec.h
===================================================================
--- qemu.orig/target-arm/exec.h
+++ qemu/target-arm/exec.h
@@ -23,7 +23,12 @@
 register struct CPUARMState *env asm(AREG0);
 register uint32_t T0 asm(AREG1);
 register uint32_t T1 asm(AREG2);
+#if defined(GCC_BREAKS_T_REGISTER)
+#define T2 (env->t2)
+#else
 register uint32_t T2 asm(AREG3);
+#endif
+
 
 /* TODO: Put these in FP regs on targets that have such things.  */
 /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.  */
Index: qemu/target-i386/cpu.h
===================================================================
--- qemu.orig/target-i386/cpu.h
+++ qemu/target-i386/cpu.h
@@ -470,6 +470,8 @@ typedef struct CPUX86State {
 #if TARGET_LONG_BITS > HOST_LONG_BITS
     /* temporaries if we cannot store them in host registers */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t1;
 #endif
 
     /* standard registers */
Index: qemu/target-i386/exec.h
===================================================================
--- qemu.orig/target-i386/exec.h
+++ qemu/target-i386/exec.h
@@ -39,6 +39,12 @@ register struct CPUX86State *env asm(ARE
 #define T1 (env->t1)
 #define T2 (env->t2)
 
+#elif defined(GCC_BREAKS_T_REGISTER)
+
+register target_ulong T0 asm(AREG1);
+#define T1 (env->t1)
+register target_ulong T2 asm(AREG3);
+
 #else
 
 /* XXX: use unsigned long instead of target_ulong - better code will
Index: qemu/target-mips/cpu.h
===================================================================
--- qemu.orig/target-mips/cpu.h
+++ qemu/target-mips/cpu.h
@@ -149,6 +149,8 @@ struct CPUMIPSState {
     target_ulong t0;
     target_ulong t1;
     target_ulong t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
     target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
Index: qemu/target-mips/exec.h
===================================================================
--- qemu.orig/target-mips/exec.h
+++ qemu/target-mips/exec.h
@@ -14,11 +14,16 @@ register struct CPUMIPSState *env asm(AR
 #define T0 (env->t0)
 #define T1 (env->t1)
 #define T2 (env->t2)
+#elif defined(GCC_BREAKS_T_REGISTER)
+register target_ulong T0 asm(AREG1);
+register target_ulong T1 asm(AREG2);
+#define T2 (env->t2)
 #else
 register target_ulong T0 asm(AREG1);
 register target_ulong T1 asm(AREG2);
 register target_ulong T2 asm(AREG3);
 #endif
+#endif
 
 #if defined (USE_HOST_FLOAT_REGS)
 #error "implement me."
Index: qemu/target-ppc/exec.h
===================================================================
--- qemu.orig/target-ppc/exec.h
+++ qemu/target-ppc/exec.h
@@ -38,10 +38,15 @@ register struct CPUPPCState *env asm(ARE
 #define T1 (env->t1)
 #define T2 (env->t2)
 #define TDX "%016" PRIx64
+#elif defined(GCC_BREAKS_T_REGISTER)
+register unsigned long T0 asm(AREG1);
+register unsigned long T1 asm(AREG2);
+#define T2 (env->t2)
 #else
 register unsigned long T0 asm(AREG1);
 register unsigned long T1 asm(AREG2);
 register unsigned long T2 asm(AREG3);
+#endif
 #define TDX "%016lx"
 #endif
 /* We may, sometime, need 64 bits registers on 32 bits targets */
Index: qemu/target-sparc/exec.h
===================================================================
--- qemu.orig/target-sparc/exec.h
+++ qemu/target-sparc/exec.h
@@ -32,9 +32,13 @@ register uint32_t T2 asm(AREG4);
 
 #else
 #define REGWPTR env->regwptr
+#if !defined(GCC_BREAKS_T_REGISTER)
 register uint32_t T2 asm(AREG3);
-#endif
 #define reg_T2
+#else
+#define T2 (env->t2)
+#endif
+#endif
 #endif
 
 #define FT0 (env->ft0)
Index: qemu/configure
===================================================================
--- qemu.orig/configure
+++ qemu/configure
@@ -806,6 +806,12 @@ echo "AIOLIBS=$AIOLIBS" >> $config_mak
 if test "$cpu" = "i386" ; then
   echo "ARCH=i386" >> $config_mak
   echo "#define HOST_I386 1" >> $config_h
+
+  # add check for gcc4 breakage
+  echo "#if (__GNUC__ > 3)" >> $config_h
+  echo "#define GCC_BREAKS_T_REGISTER" >> $config_h
+  echo "#endif" >> $config_h
+
 elif test "$cpu" = "x86_64" ; then
   echo "ARCH=x86_64" >> $config_mak
   echo "#define HOST_X86_64 1" >> $config_h

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

Johannes Schindelin wrote:
> Hi,
>
> I get an error with sh4-linux-user and sh4eb-linux-user:
>
> ../dyngen -o op.h op.o
> dyngen: ret or jmp expected at the end of op_cmp_str_T0_T1
>
>   

Yes, and it's mostly right about this. This specific opcode ends with a
jmp, which dyngen does not allow. I leave this to the SH4 developers, as
they know way better what this opcode is supposed to do.

> There is a "RETURN();" statement at the end of 
> target-sh4/op.c:op_cmp_str_T0_T1() though, so I am puzzled...
>   

It simply doesn't help ;-).

> It also says "target-ppc/exec.h:44:26: error: #if with no expression", 
> because there is an "#if GCC_BREAKS_T_REGISTER" which wants to be an 
> "#ifdef".  (This is in patch 1/5.)
>
>   

Fixed. Thank you.

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, Alexander Graf wrote:

> Alexander Graf wrote:
> > Jens Arm wrote:
> >   
> >> With this I can compile, but qemu segfaults now when trying to start a ros image:
> >>
> >> Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
> >> [Thread debugging using libthread_db enabled]
> >> [New Thread -1212344640 (LWP 18268)]
> >>
> >> Program received signal SIGSEGV, Segmentation fault.
> >> [Switching to Thread -1212344640 (LWP 18268)]
> >> 0x08bcb3b4 in code_gen_buffer ()
> >> (gdb) bt
> >> #0  0x08bcb3b4 in code_gen_buffer ()
> >> #1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
> >> #2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445
> >>
> >> Any hints?
> >
> > Looks like it's still broken. This can only be something minor, as it 
> > did work for me in between, but now I can reproduce you failure here 
> > as well. Stay tuned.
>
> Oh well, take 3 it is then.
> 
> This patch should work for most targets now. All comments so far should 
> be integrated too, so if you're missing something you said, please tell 
> me.

It fixed it for me, but only after a "make distclean && ./configure && 
make".  Apparently there are still some dependencies which are not right.

Thanks,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Jens Arm]

Hi


OK, now I can boot Reactos, haiku, helenos, dsl, knoppix, freedos, kubuntu
It works with and without kqemu.

If all is correctly emulated or if the speed of qemu is slower or quicker now I have not tested.


Jens




On Thu, 17 Jan 2008 15:26:23 +0100
Alexander Graf <alex@csgraf.de> wrote:

> Alexander Graf wrote:
> > Jens Arm wrote:
> >   
> >> Hi
> >>
> >> With this I can compile, but qemu segfaults now when trying to start a ros image:
> >>
> >> Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
> >> [Thread debugging using libthread_db enabled]
> >> [New Thread -1212344640 (LWP 18268)]
> >>
> >> Program received signal SIGSEGV, Segmentation fault.
> >> [Switching to Thread -1212344640 (LWP 18268)]
> >> 0x08bcb3b4 in code_gen_buffer ()
> >> (gdb) bt
> >> #0  0x08bcb3b4 in code_gen_buffer ()
> >> #1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
> >> #2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445
> >>
> >> Any hints?
> >>
> >>   
> >>     
> >
> > Looks like it's still broken. This can only be something minor, as it
> > did work for me in between, but now I can reproduce you failure here as
> > well. Stay tuned.
> >
> > Alex
> >
> >
> >   
> Oh well, take 3 it is then.
> 
> This patch should work for most targets now. All comments so far should
> be integrated too, so if you're missing something you said, please tell me.
> 
> Alex
> 

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, Johannes Schindelin wrote:

> It fixed it for me, but only after a "make distclean && ./configure && 
> make".  Apparently there are still some dependencies which are not 
> right.

Spoke too soon...  It introduces an extra #endif in target-mips/exec.h

FWIW I am pushing it (with fixes) to the "gcc4" branch of 
http://repo.or.cz/w/qemu/dscho.git/

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

Johannes Schindelin wrote:
> Hi,
>
> On Thu, 17 Jan 2008, Alexander Graf wrote:
>
>   
>> Alexander Graf wrote:
>>     
>>> Jens Arm wrote:
>>>   
>>>       
>>>> With this I can compile, but qemu segfaults now when trying to start a ros image:
>>>>
>>>> Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
>>>> [Thread debugging using libthread_db enabled]
>>>> [New Thread -1212344640 (LWP 18268)]
>>>>
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> [Switching to Thread -1212344640 (LWP 18268)]
>>>> 0x08bcb3b4 in code_gen_buffer ()
>>>> (gdb) bt
>>>> #0  0x08bcb3b4 in code_gen_buffer ()
>>>> #1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
>>>> #2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445
>>>>
>>>> Any hints?
>>>>         
>>> Looks like it's still broken. This can only be something minor, as it 
>>> did work for me in between, but now I can reproduce you failure here 
>>> as well. Stay tuned.
>>>       
>> Oh well, take 3 it is then.
>>
>> This patch should work for most targets now. All comments so far should 
>> be integrated too, so if you're missing something you said, please tell 
>> me.
>>     
>
> It fixed it for me, but only after a "make distclean && ./configure && 
> make".  Apparently there are still some dependencies which are not right.
>
> Thanks,
> Dscho
>   
I changed the configure script, so this is quite reasonable.

Alex

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

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

Johannes Schindelin wrote:
> Hi,
>
> On Thu, 17 Jan 2008, Johannes Schindelin wrote:
>
>   
>> It fixed it for me, but only after a "make distclean && ./configure && 
>> make".  Apparently there are still some dependencies which are not 
>> right.
>>     
>
> Spoke too soon...  It introduces an extra #endif in target-mips/exec.h
>
>   

Right, thanks.

New patch attached.

Alex

[-- Attachment #2: qemu-gcc4-i386.patch --]
[-- Type: text/x-patch, Size: 6831 bytes --]

Index: qemu/softmmu_header.h
===================================================================
--- qemu.orig/softmmu_header.h
+++ qemu/softmmu_header.h
@@ -189,9 +189,15 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+#ifdef GCC_BREAKS_T_REGISTER
+                  "pushl %%ecx\n"
+#endif
                   "pushl %6\n"
                   "call %7\n"
                   "popl %%eax\n"
+#ifdef GCC_BREAKS_T_REGISTER
+                  "popl %%ecx\n"
+#endif
                   "jmp 2f\n"
                   "1:\n"
                   "addl 8(%%edx), %%eax\n"
@@ -209,14 +215,22 @@ static inline void glue(glue(st, SUFFIX)
                   : "r" (ptr),
 /* NOTE: 'q' would be needed as constraint, but we could not use it
    with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+                  "q" (v),
+#else
                   "r" (v),
+#endif
                   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
                   "m" (*(uint32_t *)offsetof(CPUState, tlb_table[CPU_MMU_INDEX][0].addr_write)),
                   "i" (CPU_MMU_INDEX),
                   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
+#ifdef GCC_BREAKS_T_REGISTER
+                  : "%eax", "%edx", "memory", "cc");
+#else
                   : "%eax", "%ecx", "%edx", "memory", "cc");
+#endif
 }
 
 #else
Index: qemu/target-alpha/cpu.h
===================================================================
--- qemu.orig/target-alpha/cpu.h
+++ qemu/target-alpha/cpu.h
@@ -275,6 +275,8 @@ struct CPUAlphaState {
      * used to emulate 64 bits target on 32 bits hosts
      */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     /* */
     double ft0, ft1, ft2;
Index: qemu/target-alpha/exec.h
===================================================================
--- qemu.orig/target-alpha/exec.h
+++ qemu/target-alpha/exec.h
@@ -36,6 +36,12 @@ register struct CPUAlphaState *env asm(A
 #define T1 (env->t1)
 #define T2 (env->t2)
 
+#elif defined(GCC_BREAKS_T_REGISTER)
+
+register uint64_t T0 asm(AREG1);
+register uint64_t T1 asm(AREG2);
+#define T2 (env->t2)
+
 #else
 
 register uint64_t T0 asm(AREG1);
Index: qemu/target-arm/cpu.h
===================================================================
--- qemu.orig/target-arm/cpu.h
+++ qemu/target-arm/cpu.h
@@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
  */
 
 typedef struct CPUARMState {
+#if defined(GCC_BREAKS_T_REGISTER)
+    uint32_t t2;
+#endif
     /* Regs for current mode.  */
     uint32_t regs[16];
     /* Frequently accessed CPSR bits are stored separately for efficiently.
Index: qemu/target-arm/exec.h
===================================================================
--- qemu.orig/target-arm/exec.h
+++ qemu/target-arm/exec.h
@@ -23,7 +23,12 @@
 register struct CPUARMState *env asm(AREG0);
 register uint32_t T0 asm(AREG1);
 register uint32_t T1 asm(AREG2);
+#if defined(GCC_BREAKS_T_REGISTER)
+#define T2 (env->t2)
+#else
 register uint32_t T2 asm(AREG3);
+#endif
+
 
 /* TODO: Put these in FP regs on targets that have such things.  */
 /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.  */
Index: qemu/target-i386/cpu.h
===================================================================
--- qemu.orig/target-i386/cpu.h
+++ qemu/target-i386/cpu.h
@@ -470,6 +470,8 @@ typedef struct CPUX86State {
 #if TARGET_LONG_BITS > HOST_LONG_BITS
     /* temporaries if we cannot store them in host registers */
     target_ulong t0, t1, t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t1;
 #endif
 
     /* standard registers */
Index: qemu/target-i386/exec.h
===================================================================
--- qemu.orig/target-i386/exec.h
+++ qemu/target-i386/exec.h
@@ -39,6 +39,12 @@ register struct CPUX86State *env asm(ARE
 #define T1 (env->t1)
 #define T2 (env->t2)
 
+#elif defined(GCC_BREAKS_T_REGISTER)
+
+register target_ulong T0 asm(AREG1);
+#define T1 (env->t1)
+register target_ulong T2 asm(AREG3);
+
 #else
 
 /* XXX: use unsigned long instead of target_ulong - better code will
Index: qemu/target-mips/cpu.h
===================================================================
--- qemu.orig/target-mips/cpu.h
+++ qemu/target-mips/cpu.h
@@ -149,6 +149,8 @@ struct CPUMIPSState {
     target_ulong t0;
     target_ulong t1;
     target_ulong t2;
+#elif defined(GCC_BREAKS_T_REGISTER)
+    target_ulong t2;
 #endif
     target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
     target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
Index: qemu/target-mips/exec.h
===================================================================
--- qemu.orig/target-mips/exec.h
+++ qemu/target-mips/exec.h
@@ -14,6 +14,10 @@ register struct CPUMIPSState *env asm(AR
 #define T0 (env->t0)
 #define T1 (env->t1)
 #define T2 (env->t2)
+#elif defined(GCC_BREAKS_T_REGISTER)
+register target_ulong T0 asm(AREG1);
+register target_ulong T1 asm(AREG2);
+#define T2 (env->t2)
 #else
 register target_ulong T0 asm(AREG1);
 register target_ulong T1 asm(AREG2);
Index: qemu/target-ppc/exec.h
===================================================================
--- qemu.orig/target-ppc/exec.h
+++ qemu/target-ppc/exec.h
@@ -38,10 +38,15 @@ register struct CPUPPCState *env asm(ARE
 #define T1 (env->t1)
 #define T2 (env->t2)
 #define TDX "%016" PRIx64
+#elif defined(GCC_BREAKS_T_REGISTER)
+register unsigned long T0 asm(AREG1);
+register unsigned long T1 asm(AREG2);
+#define T2 (env->t2)
 #else
 register unsigned long T0 asm(AREG1);
 register unsigned long T1 asm(AREG2);
 register unsigned long T2 asm(AREG3);
+#endif
 #define TDX "%016lx"
 #endif
 /* We may, sometime, need 64 bits registers on 32 bits targets */
Index: qemu/target-sparc/exec.h
===================================================================
--- qemu.orig/target-sparc/exec.h
+++ qemu/target-sparc/exec.h
@@ -32,9 +32,13 @@ register uint32_t T2 asm(AREG4);
 
 #else
 #define REGWPTR env->regwptr
+#if !defined(GCC_BREAKS_T_REGISTER)
 register uint32_t T2 asm(AREG3);
-#endif
 #define reg_T2
+#else
+#define T2 (env->t2)
+#endif
+#endif
 #endif
 
 #define FT0 (env->ft0)
Index: qemu/configure
===================================================================
--- qemu.orig/configure
+++ qemu/configure
@@ -806,6 +806,12 @@ echo "AIOLIBS=$AIOLIBS" >> $config_mak
 if test "$cpu" = "i386" ; then
   echo "ARCH=i386" >> $config_mak
   echo "#define HOST_I386 1" >> $config_h
+
+  # add check for gcc4 breakage
+  echo "#if (__GNUC__ > 3)" >> $config_h
+  echo "#define GCC_BREAKS_T_REGISTER" >> $config_h
+  echo "#endif" >> $config_h
+
 elif test "$cpu" = "x86_64" ; then
   echo "ARCH=x86_64" >> $config_mak
   echo "#define HOST_X86_64 1" >> $config_h

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, Alexander Graf wrote:

> Johannes Schindelin wrote:
> > Hi,
> >
> > On Thu, 17 Jan 2008, Alexander Graf wrote:
> >
> >   
> >> Alexander Graf wrote:
> >>     
> >>> Jens Arm wrote:
> >>>   
> >>>       
> >>>> With this I can compile, but qemu segfaults now when trying to start a ros image:
> >>>>
> >>>> Starting program: /home/tux/QEMU/qemu -m 192 ros/ros
> >>>> [Thread debugging using libthread_db enabled]
> >>>> [New Thread -1212344640 (LWP 18268)]
> >>>>
> >>>> Program received signal SIGSEGV, Segmentation fault.
> >>>> [Switching to Thread -1212344640 (LWP 18268)]
> >>>> 0x08bcb3b4 in code_gen_buffer ()
> >>>> (gdb) bt
> >>>> #0  0x08bcb3b4 in code_gen_buffer ()
> >>>> #1  0x080de65b in cpu_x86_exec (env1=0x9c497e8) at /home/tux/compile/qemu/cpu-exec.c:679
> >>>> #2  0x0805738f in main (argc=119537671, argv=0x20000720) at /home/tux/compile/qemu/vl.c:7445
> >>>>
> >>>> Any hints?
> >>>>         
> >>> Looks like it's still broken. This can only be something minor, as it 
> >>> did work for me in between, but now I can reproduce you failure here 
> >>> as well. Stay tuned.
> >>>       
> >> Oh well, take 3 it is then.
> >>
> >> This patch should work for most targets now. All comments so far should 
> >> be integrated too, so if you're missing something you said, please tell 
> >> me.
> >>     
> >
> > It fixed it for me, but only after a "make distclean && ./configure && 
> > make".  Apparently there are still some dependencies which are not right.
> >
> > Thanks,
> > Dscho
> >   
> I changed the configure script, so this is quite reasonable.

I found two extra #endifs, in target-mips/exec.h and target-ppc/exec.h.  I 
updated my "gcc4" branch.

BTW the problem with sh4 seems to be specific to the function 
op_cmp_str_T0_T1() (commenting out the cond_t() call makes the funny 
"ret;andl;jmp" go away).

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, Alexander Graf wrote:

> Johannes Schindelin wrote:
> > Hi,
> >
> > On Thu, 17 Jan 2008, Johannes Schindelin wrote:
> >
> >   
> >> It fixed it for me, but only after a "make distclean && ./configure && 
> >> make".  Apparently there are still some dependencies which are not 
> >> right.
> >>     
> >
> > Spoke too soon...  It introduces an extra #endif in target-mips/exec.h
> >
> >   
> 
> Right, thanks.
> 
> New patch attached.

I still need this on top, to compile the user targets:

-- snipsnap --

 target-ppc/exec.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index 3387663..0ab4143 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -38,8 +38,7 @@ register struct CPUPPCState *env asm(AREG0);
 #define T1 (env->t1)
 #define T2 (env->t2)
 #define TDX "%016" PRIx64
-#else
-#if defined(GCC_BREAKS_T_REGISTER)
+#elif defined(GCC_BREAKS_T_REGISTER)
 register unsigned long T0 asm(AREG1);
 register unsigned long T1 asm(AREG2);
 #define T2 (env->t2)

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Andreas Färber]

Hi,

Am 17.01.2008 um 16:29 schrieb Johannes Schindelin:

> I found two extra #endifs, in target-mips/exec.h and target-ppc/ 
> exec.h.  I
> updated my "gcc4" branch.

I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu on  
Linux/i386 from your gcc4 branch. It compiles okay on gcc (GCC) 4.1.3  
20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). Runtime behavior was  
identical to CVS+gcc-3.4.

Andreas

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

On Jan 17, 2008, at 6:11 PM, Andreas Färber wrote:

> Hi,
>
> Am 17.01.2008 um 16:29 schrieb Johannes Schindelin:
>
>> I found two extra #endifs, in target-mips/exec.h and target-ppc/ 
>> exec.h.  I
>> updated my "gcc4" branch.
>
> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu on  
> Linux/i386 from your gcc4 branch. It compiles okay on gcc (GCC)  
> 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). Runtime  
> behavior was identical to CVS+gcc-3.4.
>
> Andreas
>
>

Could you please test gcc4 for PowerPC as well? I don't see a reason  
this should not work on OSX/PPC.

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Andreas Färber]

Am 17.01.2008 um 18:34 schrieb Alexander Graf:

>> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu on  
>> Linux/i386 from your gcc4 branch. It compiles okay on gcc (GCC)  
>> 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). Runtime  
>> behavior was identical to CVS+gcc-3.4.
>
> Could you please test gcc4 for PowerPC as well? I don't see a reason  
> this should not work on OSX/PPC.

As mentioned earlier, on OSX/ppc QEMU worked even without this patch  
mostly (with always_inline fix applied), only when running Q I got  
occasional crashes.

The gcc4 git branch plus always_inline fix compiles sparc-softmmu fine  
on powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build  
5465). As usual there are some signedness warnings. It ran as fine as  
without the fix. The other softmmus and the combination with Q I  
haven't checked yet.

Is this patch supposed to work on OSX/i386 as well as is, or does that  
require some of the existing patches?

Andreas

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Mike Kronenberg]

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


On 18.01.2008, at 00:25, Andreas Färber wrote:

>
> Am 17.01.2008 um 18:34 schrieb Alexander Graf:
>
>>> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu  
>>> on Linux/i386 from your gcc4 branch. It compiles okay on gcc (GCC)  
>>> 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). Runtime  
>>> behavior was identical to CVS+gcc-3.4.
>>
>> Could you please test gcc4 for PowerPC as well? I don't see a  
>> reason this should not work on OSX/PPC.
>
> As mentioned earlier, on OSX/ppc QEMU worked even without this patch  
> mostly (with always_inline fix applied), only when running Q I got  
> occasional crashes.
>
> The gcc4 git branch plus always_inline fix compiles sparc-softmmu  
> fine on powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc.  
> build 5465). As usual there are some signedness warnings. It ran as  
> fine as without the fix. The other softmmus and the combination with  
> Q I haven't checked yet.
>
> Is this patch supposed to work on OSX/i386 as well as is, or does  
> that require some of the existing patches?
>
> Andreas
>
>

OS X/i386 needs at least a fix for "relocation_info".
You find a stab at it in qemu-0.9.1-osx-intel-port.patch[1]... put it  
must first be adapted to Alex's patches.

Mike

[1] http://www.kju-app.org/proj/browser/branches/0_9_1/patches/qemu-0.9.1-osx-intel-port.patch?format=raw

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2111 bytes --]

[Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[consul]

It broke mingw build with gcc-3.4.2

gcc -Wall -O2 -g -fno-strict-aliasing  -fno-reorder-blocks  -fno-gcse    -fno-optimize-sibling-calls 
  -fno-crossjumping  -fno-align-labels  -fno-align-jumps  -fno-align-functions 
   -mpreferred-stack-boundary=2 -fomit-frame-pointer   -I. -I.. -I/c/qemu/target-i386 
 -I/c/qemu -MMD -MP -DNEED_CPU_H -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE 
 -I/c/qemu/fpu -DHAS_AUDIO -DHAS_AUDIO_CHOICE -I/c/qemu/slirp -c -o op.o 
/c/qemu/target-i386/op.c
c:/qemu/target-i386/ops_mem.h: In function `op_stw_kernel_T1_A0':
../softmmu_header.h:174: error: can't find a register in class `Q_REGS' 
while reloading `asm'
make[1]: *** [op.o] Error 1
make[1]: Leaving directory `/c/qemu/i386-softmmu'
make: *** [subdir-i386-softmmu] Error 2

Alex.

"Alexander Graf" <alex@csgraf.de> wrote in message 
news:478F6F71.1000408@csgraf.de...
> Johannes Schindelin wrote:
>> Hi,
>>
>> On Thu, 17 Jan 2008, Johannes Schindelin wrote:
>>
>>
>>> It fixed it for me, but only after a "make distclean && ./configure &&
>>> make".  Apparently there are still some dependencies which are not
>>> right.
>>>
>>
>> Spoke too soon...  It introduces an extra #endif in target-mips/exec.h
>>
>>
>
> Right, thanks.
>
> New patch attached.
>
> Alex
>


--------------------------------------------------------------------------------


> Index: qemu/softmmu_header.h
> ===================================================================
> --- qemu.orig/softmmu_header.h
> +++ qemu/softmmu_header.h
> @@ -189,9 +189,15 @@ static inline void glue(glue(st, SUFFIX)
> #else
> #error unsupported size
> #endif
> +#ifdef GCC_BREAKS_T_REGISTER
> +                  "pushl %%ecx\n"
> +#endif
>                   "pushl %6\n"
>                   "call %7\n"
>                   "popl %%eax\n"
> +#ifdef GCC_BREAKS_T_REGISTER
> +                  "popl %%ecx\n"
> +#endif
>                   "jmp 2f\n"
>                   "1:\n"
>                   "addl 8(%%edx), %%eax\n"
> @@ -209,14 +215,22 @@ static inline void glue(glue(st, SUFFIX)
>                   : "r" (ptr),
> /* NOTE: 'q' would be needed as constraint, but we could not use it
>    with T1 ! */
> +#if DATA_SIZE == 1 || DATA_SIZE == 2
> +                  "q" (v),
> +#else
>                   "r" (v),
> +#endif
>                   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
>                   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
>                   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
>                   "m" (*(uint32_t *)offsetof(CPUState, 
> tlb_table[CPU_MMU_INDEX][0].addr_write)),
>                   "i" (CPU_MMU_INDEX),
>                   "m" (*(uint8_t *)&glue(glue(__st, SUFFIX), MMUSUFFIX))
> +#ifdef GCC_BREAKS_T_REGISTER
> +                  : "%eax", "%edx", "memory", "cc");
> +#else
>                   : "%eax", "%ecx", "%edx", "memory", "cc");
> +#endif
> }
>
> #else
> Index: qemu/target-alpha/cpu.h
> ===================================================================
> --- qemu.orig/target-alpha/cpu.h
> +++ qemu/target-alpha/cpu.h
> @@ -275,6 +275,8 @@ struct CPUAlphaState {
>      * used to emulate 64 bits target on 32 bits hosts
>      */
>     target_ulong t0, t1, t2;
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +    target_ulong t2;
> #endif
>     /* */
>     double ft0, ft1, ft2;
> Index: qemu/target-alpha/exec.h
> ===================================================================
> --- qemu.orig/target-alpha/exec.h
> +++ qemu/target-alpha/exec.h
> @@ -36,6 +36,12 @@ register struct CPUAlphaState *env asm(A
> #define T1 (env->t1)
> #define T2 (env->t2)
>
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +
> +register uint64_t T0 asm(AREG1);
> +register uint64_t T1 asm(AREG2);
> +#define T2 (env->t2)
> +
> #else
>
> register uint64_t T0 asm(AREG1);
> Index: qemu/target-arm/cpu.h
> ===================================================================
> --- qemu.orig/target-arm/cpu.h
> +++ qemu/target-arm/cpu.h
> @@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
>  */
>
> typedef struct CPUARMState {
> +#if defined(GCC_BREAKS_T_REGISTER)
> +    uint32_t t2;
> +#endif
>     /* Regs for current mode.  */
>     uint32_t regs[16];
>     /* Frequently accessed CPSR bits are stored separately for 
> efficiently.
> Index: qemu/target-arm/exec.h
> ===================================================================
> --- qemu.orig/target-arm/exec.h
> +++ qemu/target-arm/exec.h
> @@ -23,7 +23,12 @@
> register struct CPUARMState *env asm(AREG0);
> register uint32_t T0 asm(AREG1);
> register uint32_t T1 asm(AREG2);
> +#if defined(GCC_BREAKS_T_REGISTER)
> +#define T2 (env->t2)
> +#else
> register uint32_t T2 asm(AREG3);
> +#endif
> +
>
> /* TODO: Put these in FP regs on targets that have such things.  */
> /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.  */
> Index: qemu/target-i386/cpu.h
> ===================================================================
> --- qemu.orig/target-i386/cpu.h
> +++ qemu/target-i386/cpu.h
> @@ -470,6 +470,8 @@ typedef struct CPUX86State {
> #if TARGET_LONG_BITS > HOST_LONG_BITS
>     /* temporaries if we cannot store them in host registers */
>     target_ulong t0, t1, t2;
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +    target_ulong t1;
> #endif
>
>     /* standard registers */
> Index: qemu/target-i386/exec.h
> ===================================================================
> --- qemu.orig/target-i386/exec.h
> +++ qemu/target-i386/exec.h
> @@ -39,6 +39,12 @@ register struct CPUX86State *env asm(ARE
> #define T1 (env->t1)
> #define T2 (env->t2)
>
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +
> +register target_ulong T0 asm(AREG1);
> +#define T1 (env->t1)
> +register target_ulong T2 asm(AREG3);
> +
> #else
>
> /* XXX: use unsigned long instead of target_ulong - better code will
> Index: qemu/target-mips/cpu.h
> ===================================================================
> --- qemu.orig/target-mips/cpu.h
> +++ qemu/target-mips/cpu.h
> @@ -149,6 +149,8 @@ struct CPUMIPSState {
>     target_ulong t0;
>     target_ulong t1;
>     target_ulong t2;
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +    target_ulong t2;
> #endif
>     target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
>     target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
> Index: qemu/target-mips/exec.h
> ===================================================================
> --- qemu.orig/target-mips/exec.h
> +++ qemu/target-mips/exec.h
> @@ -14,6 +14,10 @@ register struct CPUMIPSState *env asm(AR
> #define T0 (env->t0)
> #define T1 (env->t1)
> #define T2 (env->t2)
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +register target_ulong T0 asm(AREG1);
> +register target_ulong T1 asm(AREG2);
> +#define T2 (env->t2)
> #else
> register target_ulong T0 asm(AREG1);
> register target_ulong T1 asm(AREG2);
> Index: qemu/target-ppc/exec.h
> ===================================================================
> --- qemu.orig/target-ppc/exec.h
> +++ qemu/target-ppc/exec.h
> @@ -38,10 +38,15 @@ register struct CPUPPCState *env asm(ARE
> #define T1 (env->t1)
> #define T2 (env->t2)
> #define TDX "%016" PRIx64
> +#elif defined(GCC_BREAKS_T_REGISTER)
> +register unsigned long T0 asm(AREG1);
> +register unsigned long T1 asm(AREG2);
> +#define T2 (env->t2)
> #else
> register unsigned long T0 asm(AREG1);
> register unsigned long T1 asm(AREG2);
> register unsigned long T2 asm(AREG3);
> +#endif
> #define TDX "%016lx"
> #endif
> /* We may, sometime, need 64 bits registers on 32 bits targets */
> Index: qemu/target-sparc/exec.h
> ===================================================================
> --- qemu.orig/target-sparc/exec.h
> +++ qemu/target-sparc/exec.h
> @@ -32,9 +32,13 @@ register uint32_t T2 asm(AREG4);
>
> #else
> #define REGWPTR env->regwptr
> +#if !defined(GCC_BREAKS_T_REGISTER)
> register uint32_t T2 asm(AREG3);
> -#endif
> #define reg_T2
> +#else
> +#define T2 (env->t2)
> +#endif
> +#endif
> #endif
>
> #define FT0 (env->ft0)
> Index: qemu/configure
> ===================================================================
> --- qemu.orig/configure
> +++ qemu/configure
> @@ -806,6 +806,12 @@ echo "AIOLIBS=$AIOLIBS" >> $config_mak
> if test "$cpu" = "i386" ; then
>   echo "ARCH=i386" >> $config_mak
>   echo "#define HOST_I386 1" >> $config_h
> +
> +  # add check for gcc4 breakage
> +  echo "#if (__GNUC__ > 3)" >> $config_h
> +  echo "#define GCC_BREAKS_T_REGISTER" >> $config_h
> +  echo "#endif" >> $config_h
> +
> elif test "$cpu" = "x86_64" ; then
>   echo "ARCH=x86_64" >> $config_mak
>   echo "#define HOST_X86_64 1" >> $config_h
> 

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Thu, 17 Jan 2008, consul wrote:

> It broke mingw build with gcc-3.4.2

Now that's funny, since the last incarnation Alexander should have 
_exactly_ the same code as before for gcc < 4.  Are you sure that you do 
not have applied the patch that exchanges at least one "#ifdef HOST_I386" 
for "#ifdef GCC_BREAKS_T_REGISTER"?

Ciao,
Dscho

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Johannes Schindelin wrote:

> On Thu, 17 Jan 2008, consul wrote:
> 
> > It broke mingw build with gcc-3.4.2
> 
> Now that's funny, since the last incarnation Alexander should have 
> _exactly_ the same code as before for gcc < 4.  Are you sure that you do 
> not have applied the patch that exchanges at least one "#ifdef 
> HOST_I386" for "#ifdef GCC_BREAKS_T_REGISTER"?

Okay, I see it, too.  Seems this is the culprit:

-- snip --
diff --git a/softmmu_header.h b/softmmu_header.h
index 80eefa8..7e4bc03 100644
--- a/softmmu_header.h
+++ b/softmmu_header.h
@@ -209,7 +215,11 @@ static inline void glue(glue(st, SUFFIX), 
MEMSUFFIX)(target_ulong ptr, RES_TYPE
                   : "r" (ptr),
 /* NOTE: 'q' would be needed as constraint, but we could not use it
    with T1 ! */
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+                  "q" (v),
+#else
                   "r" (v),
+#endif
                   "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
                   "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
-- snap --

Michael, Alexander, what is this hunk supposed to do?

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Mike Kronenberg]

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


On 18.01.2008, at 01:40, Mike Kronenberg wrote:

>
> On 18.01.2008, at 00:25, Andreas Färber wrote:
>
>>
>> Am 17.01.2008 um 18:34 schrieb Alexander Graf:
>>
>>>> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu  
>>>> on Linux/i386 from your gcc4 branch. It compiles okay on gcc  
>>>> (GCC) 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2).  
>>>> Runtime behavior was identical to CVS+gcc-3.4.
>>>
>>> Could you please test gcc4 for PowerPC as well? I don't see a  
>>> reason this should not work on OSX/PPC.
>>
>> As mentioned earlier, on OSX/ppc QEMU worked even without this  
>> patch mostly (with always_inline fix applied), only when running Q  
>> I got occasional crashes.
>>
>> The gcc4 git branch plus always_inline fix compiles sparc-softmmu  
>> fine on powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc.  
>> build 5465). As usual there are some signedness warnings. It ran as  
>> fine as without the fix. The other softmmus and the combination  
>> with Q I haven't checked yet.
>>
>> Is this patch supposed to work on OSX/i386 as well as is, or does  
>> that require some of the existing patches?
>>
>> Andreas
>>
>>
>
> OS X/i386 needs at least a fix for "relocation_info".
> You find a stab at it in qemu-0.9.1-osx-intel-port.patch[1]... put  
> it must first be adapted to Alex's patches.
>
> Mike
>
> [1] http://www.kju-app.org/proj/browser/branches/0_9_1/patches/qemu-0.9.1-osx-intel-port.patch?format=raw


I tried it quick and dirty with Alex osx-patch (without exec-all.h  
part) on a x86_64.
everything except ppc-softmmu (/tmp/qemu/target-ppc/exec.h:135:2:  
error: #endif without #if) compiled fine. I'm going to make some more  
test and try to use Alex relocation fix on host i386.

Mike

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2111 bytes --]

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

On Jan 18, 2008, at 12:25 AM, Andreas Färber wrote:

>
> Am 17.01.2008 um 18:34 schrieb Alexander Graf:
>
>>> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu  
>>> on Linux/i386 from your gcc4 branch. It compiles okay on gcc (GCC)  
>>> 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). Runtime  
>>> behavior was identical to CVS+gcc-3.4.
>>
>> Could you please test gcc4 for PowerPC as well? I don't see a  
>> reason this should not work on OSX/PPC.
>
> As mentioned earlier, on OSX/ppc QEMU worked even without this patch  
> mostly (with always_inline fix applied), only when running Q I got  
> occasional crashes.

Using gcc4 on OSX/ppc still is not that bad of an idea. I don't know  
when Apple is going to drop support for gcc3, but I'm pretty sure they  
will. And since this is necessary for Linux/ppc anyway, why not check  
if it works on all platforms?

The performance hit (3% without kqemu as Michael measured) you get on  
i386 by using gcc4 does not affect ppc. So the only thing you get is  
more intelligent optimization, which might even make the gcc4 version  
faster.

>
>
> The gcc4 git branch plus always_inline fix compiles sparc-softmmu  
> fine on powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc.  
> build 5465). As usual there are some signedness warnings. It ran as  
> fine as without the fix. The other softmmus and the combination with  
> Q I haven't checked yet.
>
> Is this patch supposed to work on OSX/i386 as well as is, or does  
> that require some of the existing patches?
>

You still need support for Mach-O on i386. I'll leave that part to  
Mike Kronenberg though ;-).

Alex

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Alexander Graf]

On Jan 18, 2008, at 2:14 AM, consul wrote:

> It broke mingw build with gcc-3.4.2

Does it work if you change the lines I'll comment on below?

>
>
> gcc -Wall -O2 -g -fno-strict-aliasing  -fno-reorder-blocks  -fno- 
> gcse    -fno-optimize-sibling-calls
>  -fno-crossjumping  -fno-align-labels  -fno-align-jumps  -fno-align- 
> functions
>   -mpreferred-stack-boundary=2 -fomit-frame-pointer   -I. -I.. -I/c/ 
> qemu/target-i386
> -I/c/qemu -MMD -MP -DNEED_CPU_H -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64  
> -D_LARGEFILE_SOURCE
> -I/c/qemu/fpu -DHAS_AUDIO -DHAS_AUDIO_CHOICE -I/c/qemu/slirp -c -o  
> op.o
> /c/qemu/target-i386/op.c
> c:/qemu/target-i386/ops_mem.h: In function `op_stw_kernel_T1_A0':
> ../softmmu_header.h:174: error: can't find a register in class  
> `Q_REGS'
> while reloading `asm'
> make[1]: *** [op.o] Error 1
> make[1]: Leaving directory `/c/qemu/i386-softmmu'
> make: *** [subdir-i386-softmmu] Error 2
>
> Alex.
>
> "Alexander Graf" <alex@csgraf.de> wrote in message
> news:478F6F71.1000408@csgraf.de...
>> Johannes Schindelin wrote:
>>> Hi,
>>>
>>> On Thu, 17 Jan 2008, Johannes Schindelin wrote:
>>>
>>>
>>>> It fixed it for me, but only after a "make distclean && ./ 
>>>> configure &&
>>>> make".  Apparently there are still some dependencies which are not
>>>> right.
>>>>
>>>
>>> Spoke too soon...  It introduces an extra #endif in target-mips/ 
>>> exec.h
>>>
>>>
>>
>> Right, thanks.
>>
>> New patch attached.
>>
>> Alex
>>
>
>
> --------------------------------------------------------------------------------
>
>
>> Index: qemu/softmmu_header.h
>> ===================================================================
>> --- qemu.orig/softmmu_header.h
>> +++ qemu/softmmu_header.h
>> @@ -189,9 +189,15 @@ static inline void glue(glue(st, SUFFIX)
>> #else
>> #error unsupported size
>> #endif
>> +#ifdef GCC_BREAKS_T_REGISTER
>> +                  "pushl %%ecx\n"
>> +#endif

Remove the ifdef and endif.

>>
>>                  "pushl %6\n"
>>                  "call %7\n"
>>                  "popl %%eax\n"
>> +#ifdef GCC_BREAKS_T_REGISTER
>> +                  "popl %%ecx\n"
>> +#endif

remove the ifdef and endif.

>>
>>                  "jmp 2f\n"
>>                  "1:\n"
>>                  "addl 8(%%edx), %%eax\n"
>> @@ -209,14 +215,22 @@ static inline void glue(glue(st, SUFFIX)
>>                  : "r" (ptr),
>> /* NOTE: 'q' would be needed as constraint, but we could not use it
>>   with T1 ! */
>> +#if DATA_SIZE == 1 || DATA_SIZE == 2
>> +                  "q" (v),
>> +#else
>>                  "r" (v),
>> +#endif
>>                  "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
>>                  "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
>>                  "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
>>                  "m" (*(uint32_t *)offsetof(CPUState,
>> tlb_table[CPU_MMU_INDEX][0].addr_write)),
>>                  "i" (CPU_MMU_INDEX),
>>                  "m" (*(uint8_t *)&glue(glue(__st, SUFFIX),  
>> MMUSUFFIX))
>> +#ifdef GCC_BREAKS_T_REGISTER
>> +                  : "%eax", "%edx", "memory", "cc");
>> +#else
>>                  : "%eax", "%ecx", "%edx", "memory", "cc");
>> +#endif

remove the ifdef, else, the line after else and endif.

>>
>> }
>>
>> #else
>> Index: qemu/target-alpha/cpu.h
>> ===================================================================
>> --- qemu.orig/target-alpha/cpu.h
>> +++ qemu/target-alpha/cpu.h
>> @@ -275,6 +275,8 @@ struct CPUAlphaState {
>>     * used to emulate 64 bits target on 32 bits hosts
>>     */
>>    target_ulong t0, t1, t2;
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +    target_ulong t2;
>> #endif
>>    /* */
>>    double ft0, ft1, ft2;
>> Index: qemu/target-alpha/exec.h
>> ===================================================================
>> --- qemu.orig/target-alpha/exec.h
>> +++ qemu/target-alpha/exec.h
>> @@ -36,6 +36,12 @@ register struct CPUAlphaState *env asm(A
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>>
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +
>> +register uint64_t T0 asm(AREG1);
>> +register uint64_t T1 asm(AREG2);
>> +#define T2 (env->t2)
>> +
>> #else
>>
>> register uint64_t T0 asm(AREG1);
>> Index: qemu/target-arm/cpu.h
>> ===================================================================
>> --- qemu.orig/target-arm/cpu.h
>> +++ qemu/target-arm/cpu.h
>> @@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
>> */
>>
>> typedef struct CPUARMState {
>> +#if defined(GCC_BREAKS_T_REGISTER)
>> +    uint32_t t2;
>> +#endif
>>    /* Regs for current mode.  */
>>    uint32_t regs[16];
>>    /* Frequently accessed CPSR bits are stored separately for
>> efficiently.
>> Index: qemu/target-arm/exec.h
>> ===================================================================
>> --- qemu.orig/target-arm/exec.h
>> +++ qemu/target-arm/exec.h
>> @@ -23,7 +23,12 @@
>> register struct CPUARMState *env asm(AREG0);
>> register uint32_t T0 asm(AREG1);
>> register uint32_t T1 asm(AREG2);
>> +#if defined(GCC_BREAKS_T_REGISTER)
>> +#define T2 (env->t2)
>> +#else
>> register uint32_t T2 asm(AREG3);
>> +#endif
>> +
>>
>> /* TODO: Put these in FP regs on targets that have such things.  */
>> /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.   
>> */
>> Index: qemu/target-i386/cpu.h
>> ===================================================================
>> --- qemu.orig/target-i386/cpu.h
>> +++ qemu/target-i386/cpu.h
>> @@ -470,6 +470,8 @@ typedef struct CPUX86State {
>> #if TARGET_LONG_BITS > HOST_LONG_BITS
>>    /* temporaries if we cannot store them in host registers */
>>    target_ulong t0, t1, t2;
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +    target_ulong t1;
>> #endif
>>
>>    /* standard registers */
>> Index: qemu/target-i386/exec.h
>> ===================================================================
>> --- qemu.orig/target-i386/exec.h
>> +++ qemu/target-i386/exec.h
>> @@ -39,6 +39,12 @@ register struct CPUX86State *env asm(ARE
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>>
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +
>> +register target_ulong T0 asm(AREG1);
>> +#define T1 (env->t1)
>> +register target_ulong T2 asm(AREG3);
>> +
>> #else
>>
>> /* XXX: use unsigned long instead of target_ulong - better code will
>> Index: qemu/target-mips/cpu.h
>> ===================================================================
>> --- qemu.orig/target-mips/cpu.h
>> +++ qemu/target-mips/cpu.h
>> @@ -149,6 +149,8 @@ struct CPUMIPSState {
>>    target_ulong t0;
>>    target_ulong t1;
>>    target_ulong t2;
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +    target_ulong t2;
>> #endif
>>    target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
>>    target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
>> Index: qemu/target-mips/exec.h
>> ===================================================================
>> --- qemu.orig/target-mips/exec.h
>> +++ qemu/target-mips/exec.h
>> @@ -14,6 +14,10 @@ register struct CPUMIPSState *env asm(AR
>> #define T0 (env->t0)
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +register target_ulong T0 asm(AREG1);
>> +register target_ulong T1 asm(AREG2);
>> +#define T2 (env->t2)
>> #else
>> register target_ulong T0 asm(AREG1);
>> register target_ulong T1 asm(AREG2);
>> Index: qemu/target-ppc/exec.h
>> ===================================================================
>> --- qemu.orig/target-ppc/exec.h
>> +++ qemu/target-ppc/exec.h
>> @@ -38,10 +38,15 @@ register struct CPUPPCState *env asm(ARE
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>> #define TDX "%016" PRIx64
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +register unsigned long T0 asm(AREG1);
>> +register unsigned long T1 asm(AREG2);
>> +#define T2 (env->t2)
>> #else
>> register unsigned long T0 asm(AREG1);
>> register unsigned long T1 asm(AREG2);
>> register unsigned long T2 asm(AREG3);
>> +#endif
>> #define TDX "%016lx"
>> #endif
>> /* We may, sometime, need 64 bits registers on 32 bits targets */
>> Index: qemu/target-sparc/exec.h
>> ===================================================================
>> --- qemu.orig/target-sparc/exec.h
>> +++ qemu/target-sparc/exec.h
>> @@ -32,9 +32,13 @@ register uint32_t T2 asm(AREG4);
>>
>> #else
>> #define REGWPTR env->regwptr
>> +#if !defined(GCC_BREAKS_T_REGISTER)
>> register uint32_t T2 asm(AREG3);
>> -#endif
>> #define reg_T2
>> +#else
>> +#define T2 (env->t2)
>> +#endif
>> +#endif
>> #endif
>>
>> #define FT0 (env->ft0)
>> Index: qemu/configure
>> ===================================================================
>> --- qemu.orig/configure
>> +++ qemu/configure
>> @@ -806,6 +806,12 @@ echo "AIOLIBS=$AIOLIBS" >> $config_mak
>> if test "$cpu" = "i386" ; then
>>  echo "ARCH=i386" >> $config_mak
>>  echo "#define HOST_I386 1" >> $config_h
>> +
>> +  # add check for gcc4 breakage
>> +  echo "#if (__GNUC__ > 3)" >> $config_h
>> +  echo "#define GCC_BREAKS_T_REGISTER" >> $config_h
>> +  echo "#endif" >> $config_h
>> +
>> elif test "$cpu" = "x86_64" ; then
>>  echo "ARCH=x86_64" >> $config_mak
>>  echo "#define HOST_X86_64 1" >> $config_h
>>
>
>
>
>
>

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Andreas Färber]

Am 18.01.2008 um 07:19 schrieb Alexander Graf:

>
> On Jan 18, 2008, at 12:25 AM, Andreas Färber wrote:
>
>>
>> Am 17.01.2008 um 18:34 schrieb Alexander Graf:
>>
>>>> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu  
>>>> on Linux/i386 from your gcc4 branch. It compiles okay on gcc  
>>>> (GCC) 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2).  
>>>> Runtime behavior was identical to CVS+gcc-3.4.
>>>
>>> Could you please test gcc4 for PowerPC as well? I don't see a  
>>> reason this should not work on OSX/PPC.
>>
>> As mentioned earlier, on OSX/ppc QEMU worked even without this  
>> patch mostly (with always_inline fix applied), only when running Q  
>> I got occasional crashes.
>
> Using gcc4 on OSX/ppc still is not that bad of an idea. I don't know  
> when Apple is going to drop support for gcc3, but I'm pretty sure  
> they will. And since this is necessary for Linux/ppc anyway, why not  
> check if it works on all platforms?
>
> The performance hit (3% without kqemu as Michael measured) you get  
> on i386 by using gcc4 does not affect ppc. So the only thing you get  
> is more intelligent optimization, which might even make the gcc4  
> version faster.

Which is what I did, sorry if I didn't make that clear: I was already  
using gcc4 on OSX/ppc because Leopard's header files seem to have  
dropped gcc-3.3 support. Only on OSX/i386 it doesn't work right away.

Andreas

[Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Michael Matz]

Hi,

[answering to a forwarded mail, hence breaking thread, sorry]
[and keep me CCed :) ]

On Fri, 18 Jan 2008, Alexander Graf wrote:

> >From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> >Date: January 18, 2008 3:05:58 AM GMT+01:00
> >To: consul <consul@collegeclub.com>
> >Cc: qemu-devel@nongnu.org
> >Subject: Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host
> >Reply-To: qemu-devel@nongnu.org
> >
> >Hi,
> >
> >On Fri, 18 Jan 2008, Johannes Schindelin wrote:
> >
> > >On Thu, 17 Jan 2008, consul wrote:
> > >
> > > >It broke mingw build with gcc-3.4.2
> > >
> > >Now that's funny, since the last incarnation Alexander should have
> >>_exactly_ the same code as before for gcc < 4.  Are you sure that you do
> > >not have applied the patch that exchanges at least one "#ifdef
> > >HOST_I386" for "#ifdef GCC_BREAKS_T_REGISTER"?
> >
> >Okay, I see it, too.  Seems this is the culprit:
> >
> >-- snip --
> >diff --git a/softmmu_header.h b/softmmu_header.h
> >index 80eefa8..7e4bc03 100644
> >--- a/softmmu_header.h
> >+++ b/softmmu_header.h
> >@@ -209,7 +215,11 @@ static inline void glue(glue(st, SUFFIX),
> >MEMSUFFIX)(target_ulong ptr, RES_TYPE
> > : "r" (ptr),
> >/* NOTE: 'q' would be needed as constraint, but we could not use it
> >   with T1 ! */
> >+#if DATA_SIZE == 1 || DATA_SIZE == 2
> >+                  "q" (v),
> >+#else
> >                  "r" (v),
> >+#endif
> >                  "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
> >                  "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
> >                  "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
> >-- snap --
> >
> >Michael, Alexander, what is this hunk supposed to do?

This is required to generate valid assembler code.  Without that hunk, the 
interesting parts of the asm look like so (for DATA_SIZE == 1):

asm (" ... movzbl %b1, %%edx\n ... " : : "r" (blubb), "r" (bla) );

I.e. an instruction which requires a byte register in operand 1.  Now 
constraint "r" only guarantees a free integer register, including e.g. 
%edi, for which no low 8bit part exists (on i386).  The constraints have 
to match the use in the asm template, so in this case must mention an 
integer register for which low parts are constructible, %eax,%ebx,%ecx or 
%edx, i.e. constraint "q".

The comment above this constraint is true in the sense that T1 (== AREG2 
== %esi on i386) can not be used in this operand for DATA_SIZE being 1 or 
2.  But if it were ever used before the patch in that place it would have 
generated invalid assembler code already, and as it didn't the conclusion 
must be, that T1 simply isn't used with this datasize.  So the comment is 
true, but harmless (especially if T1 is defined to env->t1, instead of 
AREG2 to reduce register pressure).  What problems do you have with this 
hunk?


Ciao,
Michael.

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Alexander Graf]

I am merely resending this mail so Michael knows the exact error  
message.
Please have him CCed when replying to this thread.

On Jan 18, 2008, at 2:14 AM, consul wrote:

> It broke mingw build with gcc-3.4.2
>
> gcc -Wall -O2 -g -fno-strict-aliasing  -fno-reorder-blocks  -fno- 
> gcse    -fno-optimize-sibling-calls
>  -fno-crossjumping  -fno-align-labels  -fno-align-jumps  -fno-align- 
> functions
>   -mpreferred-stack-boundary=2 -fomit-frame-pointer   -I. -I.. -I/c/ 
> qemu/target-i386
> -I/c/qemu -MMD -MP -DNEED_CPU_H -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64  
> -D_LARGEFILE_SOURCE
> -I/c/qemu/fpu -DHAS_AUDIO -DHAS_AUDIO_CHOICE -I/c/qemu/slirp -c -o  
> op.o
> /c/qemu/target-i386/op.c
> c:/qemu/target-i386/ops_mem.h: In function `op_stw_kernel_T1_A0':
> ../softmmu_header.h:174: error: can't find a register in class  
> `Q_REGS'
> while reloading `asm'
> make[1]: *** [op.o] Error 1
> make[1]: Leaving directory `/c/qemu/i386-softmmu'
> make: *** [subdir-i386-softmmu] Error 2
>
> Alex.
>
> "Alexander Graf" <alex@csgraf.de> wrote in message
> news:478F6F71.1000408@csgraf.de...
>> Johannes Schindelin wrote:
>>> Hi,
>>>
>>> On Thu, 17 Jan 2008, Johannes Schindelin wrote:
>>>
>>>
>>>> It fixed it for me, but only after a "make distclean && ./ 
>>>> configure &&
>>>> make".  Apparently there are still some dependencies which are not
>>>> right.
>>>>
>>>
>>> Spoke too soon...  It introduces an extra #endif in target-mips/ 
>>> exec.h
>>>
>>>
>>
>> Right, thanks.
>>
>> New patch attached.
>>
>> Alex
>>
>
>
> --------------------------------------------------------------------------------
>
>
>> Index: qemu/softmmu_header.h
>> ===================================================================
>> --- qemu.orig/softmmu_header.h
>> +++ qemu/softmmu_header.h
>> @@ -189,9 +189,15 @@ static inline void glue(glue(st, SUFFIX)
>> #else
>> #error unsupported size
>> #endif
>> +#ifdef GCC_BREAKS_T_REGISTER
>> +                  "pushl %%ecx\n"
>> +#endif
>>                  "pushl %6\n"
>>                  "call %7\n"
>>                  "popl %%eax\n"
>> +#ifdef GCC_BREAKS_T_REGISTER
>> +                  "popl %%ecx\n"
>> +#endif
>>                  "jmp 2f\n"
>>                  "1:\n"
>>                  "addl 8(%%edx), %%eax\n"
>> @@ -209,14 +215,22 @@ static inline void glue(glue(st, SUFFIX)
>>                  : "r" (ptr),
>> /* NOTE: 'q' would be needed as constraint, but we could not use it
>>   with T1 ! */
>> +#if DATA_SIZE == 1 || DATA_SIZE == 2
>> +                  "q" (v),
>> +#else
>>                  "r" (v),
>> +#endif
>>                  "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
>>                  "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
>>                  "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
>>                  "m" (*(uint32_t *)offsetof(CPUState,
>> tlb_table[CPU_MMU_INDEX][0].addr_write)),
>>                  "i" (CPU_MMU_INDEX),
>>                  "m" (*(uint8_t *)&glue(glue(__st, SUFFIX),  
>> MMUSUFFIX))
>> +#ifdef GCC_BREAKS_T_REGISTER
>> +                  : "%eax", "%edx", "memory", "cc");
>> +#else
>>                  : "%eax", "%ecx", "%edx", "memory", "cc");
>> +#endif
>> }
>>
>> #else
>> Index: qemu/target-alpha/cpu.h
>> ===================================================================
>> --- qemu.orig/target-alpha/cpu.h
>> +++ qemu/target-alpha/cpu.h
>> @@ -275,6 +275,8 @@ struct CPUAlphaState {
>>     * used to emulate 64 bits target on 32 bits hosts
>>     */
>>    target_ulong t0, t1, t2;
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +    target_ulong t2;
>> #endif
>>    /* */
>>    double ft0, ft1, ft2;
>> Index: qemu/target-alpha/exec.h
>> ===================================================================
>> --- qemu.orig/target-alpha/exec.h
>> +++ qemu/target-alpha/exec.h
>> @@ -36,6 +36,12 @@ register struct CPUAlphaState *env asm(A
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>>
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +
>> +register uint64_t T0 asm(AREG1);
>> +register uint64_t T1 asm(AREG2);
>> +#define T2 (env->t2)
>> +
>> #else
>>
>> register uint64_t T0 asm(AREG1);
>> Index: qemu/target-arm/cpu.h
>> ===================================================================
>> --- qemu.orig/target-arm/cpu.h
>> +++ qemu/target-arm/cpu.h
>> @@ -66,6 +66,9 @@ typedef uint32_t ARMReadCPFunc(void *opa
>> */
>>
>> typedef struct CPUARMState {
>> +#if defined(GCC_BREAKS_T_REGISTER)
>> +    uint32_t t2;
>> +#endif
>>    /* Regs for current mode.  */
>>    uint32_t regs[16];
>>    /* Frequently accessed CPSR bits are stored separately for
>> efficiently.
>> Index: qemu/target-arm/exec.h
>> ===================================================================
>> --- qemu.orig/target-arm/exec.h
>> +++ qemu/target-arm/exec.h
>> @@ -23,7 +23,12 @@
>> register struct CPUARMState *env asm(AREG0);
>> register uint32_t T0 asm(AREG1);
>> register uint32_t T1 asm(AREG2);
>> +#if defined(GCC_BREAKS_T_REGISTER)
>> +#define T2 (env->t2)
>> +#else
>> register uint32_t T2 asm(AREG3);
>> +#endif
>> +
>>
>> /* TODO: Put these in FP regs on targets that have such things.  */
>> /* It is ok for FT0s and FT0d to overlap.  Likewise FT1s and FT1d.   
>> */
>> Index: qemu/target-i386/cpu.h
>> ===================================================================
>> --- qemu.orig/target-i386/cpu.h
>> +++ qemu/target-i386/cpu.h
>> @@ -470,6 +470,8 @@ typedef struct CPUX86State {
>> #if TARGET_LONG_BITS > HOST_LONG_BITS
>>    /* temporaries if we cannot store them in host registers */
>>    target_ulong t0, t1, t2;
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +    target_ulong t1;
>> #endif
>>
>>    /* standard registers */
>> Index: qemu/target-i386/exec.h
>> ===================================================================
>> --- qemu.orig/target-i386/exec.h
>> +++ qemu/target-i386/exec.h
>> @@ -39,6 +39,12 @@ register struct CPUX86State *env asm(ARE
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>>
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +
>> +register target_ulong T0 asm(AREG1);
>> +#define T1 (env->t1)
>> +register target_ulong T2 asm(AREG3);
>> +
>> #else
>>
>> /* XXX: use unsigned long instead of target_ulong - better code will
>> Index: qemu/target-mips/cpu.h
>> ===================================================================
>> --- qemu.orig/target-mips/cpu.h
>> +++ qemu/target-mips/cpu.h
>> @@ -149,6 +149,8 @@ struct CPUMIPSState {
>>    target_ulong t0;
>>    target_ulong t1;
>>    target_ulong t2;
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +    target_ulong t2;
>> #endif
>>    target_ulong HI[MIPS_DSP_ACC][MIPS_TC_MAX];
>>    target_ulong LO[MIPS_DSP_ACC][MIPS_TC_MAX];
>> Index: qemu/target-mips/exec.h
>> ===================================================================
>> --- qemu.orig/target-mips/exec.h
>> +++ qemu/target-mips/exec.h
>> @@ -14,6 +14,10 @@ register struct CPUMIPSState *env asm(AR
>> #define T0 (env->t0)
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +register target_ulong T0 asm(AREG1);
>> +register target_ulong T1 asm(AREG2);
>> +#define T2 (env->t2)
>> #else
>> register target_ulong T0 asm(AREG1);
>> register target_ulong T1 asm(AREG2);
>> Index: qemu/target-ppc/exec.h
>> ===================================================================
>> --- qemu.orig/target-ppc/exec.h
>> +++ qemu/target-ppc/exec.h
>> @@ -38,10 +38,15 @@ register struct CPUPPCState *env asm(ARE
>> #define T1 (env->t1)
>> #define T2 (env->t2)
>> #define TDX "%016" PRIx64
>> +#elif defined(GCC_BREAKS_T_REGISTER)
>> +register unsigned long T0 asm(AREG1);
>> +register unsigned long T1 asm(AREG2);
>> +#define T2 (env->t2)
>> #else
>> register unsigned long T0 asm(AREG1);
>> register unsigned long T1 asm(AREG2);
>> register unsigned long T2 asm(AREG3);
>> +#endif
>> #define TDX "%016lx"
>> #endif
>> /* We may, sometime, need 64 bits registers on 32 bits targets */
>> Index: qemu/target-sparc/exec.h
>> ===================================================================
>> --- qemu.orig/target-sparc/exec.h
>> +++ qemu/target-sparc/exec.h
>> @@ -32,9 +32,13 @@ register uint32_t T2 asm(AREG4);
>>
>> #else
>> #define REGWPTR env->regwptr
>> +#if !defined(GCC_BREAKS_T_REGISTER)
>> register uint32_t T2 asm(AREG3);
>> -#endif
>> #define reg_T2
>> +#else
>> +#define T2 (env->t2)
>> +#endif
>> +#endif
>> #endif
>>
>> #define FT0 (env->ft0)
>> Index: qemu/configure
>> ===================================================================
>> --- qemu.orig/configure
>> +++ qemu/configure
>> @@ -806,6 +806,12 @@ echo "AIOLIBS=$AIOLIBS" >> $config_mak
>> if test "$cpu" = "i386" ; then
>>  echo "ARCH=i386" >> $config_mak
>>  echo "#define HOST_I386 1" >> $config_h
>> +
>> +  # add check for gcc4 breakage
>> +  echo "#if (__GNUC__ > 3)" >> $config_h
>> +  echo "#define GCC_BREAKS_T_REGISTER" >> $config_h
>> +  echo "#endif" >> $config_h
>> +
>> elif test "$cpu" = "x86_64" ; then
>>  echo "ARCH=x86_64" >> $config_mak
>>  echo "#define HOST_X86_64 1" >> $config_h
>>
>
>
>
>
>

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Mike Kronenberg wrote:

> I tried it quick and dirty with Alex osx-patch (without exec-all.h part) 
> on a x86_64. everything except ppc-softmmu 
> (/tmp/qemu/target-ppc/exec.h:135:2: error: #endif without #if) compiled 
> fine. I'm going to make some more test and try to use Alex relocation 
> fix on host i386.

I think I have this on top of Alexander's patch (for my current version, 
see 

http://repo.or.cz/w/qemu/dscho.git?a=commitdiff;h=fe49802591f40c7fbdb06e1301bb82dab95f9a74

it even has a "raw" link so you can directly "git am" it):

-- snipsnap --
diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index 0ab4143..3387663 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -38,7 +38,8 @@ register struct CPUPPCState *env asm(AREG0);
 #define T1 (env->t1)
 #define T2 (env->t2)
 #define TDX "%016" PRIx64
-#elif defined(GCC_BREAKS_T_REGISTER)
+#else
+#if defined(GCC_BREAKS_T_REGISTER)
 register unsigned long T0 asm(AREG1);
 register unsigned long T1 asm(AREG2);
 #define T2 (env->t2)

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Michael Matz wrote:

> [answering to a forwarded mail, hence breaking thread, sorry]
> [and keep me CCed :) ]

Sorry, I never break Cc: list, but always reply-to-all.

> On Fri, 18 Jan 2008, Alexander Graf wrote:
> 
> > >On Fri, 18 Jan 2008, Johannes Schindelin wrote:
> > >
> > > >On Thu, 17 Jan 2008, consul wrote:
> > > >
> > > > >It broke mingw build with gcc-3.4.2
> > > >
> > > >Now that's funny, since the last incarnation Alexander should have 
> > > >_exactly_ the same code as before for gcc < 4.  Are you sure that 
> > > >you do not have applied the patch that exchanges at least one 
> > > >"#ifdef HOST_I386" for "#ifdef GCC_BREAKS_T_REGISTER"?
> > >
> > >Okay, I see it, too.  Seems this is the culprit:
> > >
> > >-- snip --
> > >diff --git a/softmmu_header.h b/softmmu_header.h
> > >index 80eefa8..7e4bc03 100644
> > >--- a/softmmu_header.h
> > >+++ b/softmmu_header.h
> > >@@ -209,7 +215,11 @@ static inline void glue(glue(st, SUFFIX),
> > >MEMSUFFIX)(target_ulong ptr, RES_TYPE
> > > : "r" (ptr),
> > >/* NOTE: 'q' would be needed as constraint, but we could not use it
> > >   with T1 ! */
> > >+#if DATA_SIZE == 1 || DATA_SIZE == 2
> > >+                  "q" (v),
> > >+#else
> > >                  "r" (v),
> > >+#endif
> > >                  "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
> > >                  "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
> > >                  "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
> > >-- snap --
> > >
> > >Michael, Alexander, what is this hunk supposed to do?
> 
> This is required to generate valid assembler code.  Without that hunk, the 
> interesting parts of the asm look like so (for DATA_SIZE == 1):
> 
> asm (" ... movzbl %b1, %%edx\n ... " : : "r" (blubb), "r" (bla) );

Okay, but this only concerns gcc4, apparently.  Can't we guard it with yet 
another "defined(GCC...)"?

> I.e. an instruction which requires a byte register in operand 1.  Now 
> constraint "r" only guarantees a free integer register, including e.g. 
> %edi, for which no low 8bit part exists (on i386).  The constraints have 
> to match the use in the asm template, so in this case must mention an 
> integer register for which low parts are constructible, %eax,%ebx,%ecx 
> or %edx, i.e. constraint "q".
> 
> The comment above this constraint is true in the sense that T1 (== AREG2 
> == %esi on i386) can not be used in this operand for DATA_SIZE being 1 or 
> 2.  But if it were ever used before the patch in that place it would have 
> generated invalid assembler code already, and as it didn't the conclusion 
> must be, that T1 simply isn't used with this datasize.  So the comment is 
> true, but harmless (especially if T1 is defined to env->t1, instead of 
> AREG2 to reduce register pressure).  What problems do you have with this 
> hunk?

The problem is as described:

c:/qemu/target-i386/ops_mem.h: In function `op_stw_kernel_T1_A0':
../softmmu_header.h:174: error: can't find a register in class `Q_REGS' 
while reloading `asm'

For details, please see

	http://article.gmane.org/gmane.comp.emulators.qemu/22743

Ciao,
Dscho

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Michael Matz]

Hi,

On Fri, 18 Jan 2008, Johannes Schindelin wrote:

> > > >+#if DATA_SIZE == 1 || DATA_SIZE == 2
> > > >+                  "q" (v),
> > > >+#else
> > > >                  "r" (v),
> > > >+#endif
> > > >                  "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
> > > >                  "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
> > > >                  "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
> > > >-- snap --
> > > >
> > > >Michael, Alexander, what is this hunk supposed to do?
> > 
> > This is required to generate valid assembler code.  Without that hunk, the 
> > interesting parts of the asm look like so (for DATA_SIZE == 1):
> > 
> > asm (" ... movzbl %b1, %%edx\n ... " : : "r" (blubb), "r" (bla) );
> 
> Okay, but this only concerns gcc4, apparently.

No, it's nothing to do with GCC.  The instruction itself (movzbl) requires 
an 8-bit register, so it must be made sure by the constraints the that 
operand indeed is one of those four.  If it also works with "r" then this 
is just pure luck (in that GCC chooses one of the four good registes, and 
not one of the three bad ones allowable with "r").

> Can't we guard it with yet another "defined(GCC...)"?

Only if you want to trust your luck.  I fear I don't have gcc 3.4.2 lying 
around anywhere, so I can't really help debugging this reload breakage in 
that GCC version.  It might help to introduce a temporary to guide GCC 
through this problematic reload case by detaching the global register 
variable from the asm operand.  For cases where it's no problem this 
should be optimized away, so doesn't inhibit a performance cost.  What I 
mean is something like the below.  If someone with gcc 3.4.2 could test 
that ...


Ciao,
Michael.
-- 
--- softmmu_header.h.mm	2008-01-18 14:15:46.000000000 +0100
+++ softmmu_header.h	2008-01-18 14:14:49.000000000 +0100
@@ -212,6 +212,9 @@ static inline int glue(glue(lds, SUFFIX)
 
 static inline void glue(glue(st, SUFFIX), MEMSUFFIX)(target_ulong ptr, RES_TYPE v)
 {
+#if DATA_SIZE == 1 || DATA_SIZE == 2
+    RES_TYPE vtmp = v;
+#endif
     asm volatile ("movl %0, %%edx\n"
                   "movl %0, %%eax\n"
                   "shrl %3, %%edx\n"
@@ -253,7 +256,7 @@ static inline void glue(glue(st, SUFFIX)
 /* NOTE: 'q' would be needed as constraint, but we could not use it
    with T1 ! */
 #if DATA_SIZE == 1 || DATA_SIZE == 2
-		  "q" (v),
+		  "q" (vtmp),
 #else
                   "r" (v), 
 #endif

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Michael Matz wrote:

> On Fri, 18 Jan 2008, Johannes Schindelin wrote:
> 
> > > > >+#if DATA_SIZE == 1 || DATA_SIZE == 2
> > > > >+                  "q" (v),
> > > > >+#else
> > > > >                  "r" (v),
> > > > >+#endif
> > > > >                  "i" ((CPU_TLB_SIZE - 1) << CPU_TLB_ENTRY_BITS),
> > > > >                  "i" (TARGET_PAGE_BITS - CPU_TLB_ENTRY_BITS),
> > > > >                  "i" (TARGET_PAGE_MASK | (DATA_SIZE - 1)),
> > > > >-- snap --
> > > > >
> > > > >Michael, Alexander, what is this hunk supposed to do?
> > > 
> > > This is required to generate valid assembler code.  Without that hunk, the 
> > > interesting parts of the asm look like so (for DATA_SIZE == 1):
> > > 
> > > asm (" ... movzbl %b1, %%edx\n ... " : : "r" (blubb), "r" (bla) );
> > 
> > Okay, but this only concerns gcc4, apparently.
> 
> No, it's nothing to do with GCC.

But apparently it has!  With gcc < 4 I did never get the error.  Which 
probably means that gcc < 4 did _not_ use ecx, and therefore it does not 
have to be pushed and popped.  Which -- judging from how commonly glue() 
is called in op.c -- could mean a performance hit.

I am all for supporting gcc > 3, but please, please not at the cost of 
having a performance hit for _existing_ users.

> > Can't we guard it with yet another "defined(GCC...)"?
> 
> Only if you want to trust your luck.  I fear I don't have gcc 3.4.2 
> lying around anywhere, so I can't really help debugging this reload 
> breakage in that GCC version.  It might help to introduce a temporary to 
> guide GCC through this problematic reload case by detaching the global 
> register variable from the asm operand.  For cases where it's no problem 
> this should be optimized away, so doesn't inhibit a performance cost.  
> What I mean is something like the below.  If someone with gcc 3.4.2 
> could test that ...

I do ask myself how gcc would optimise away instructions that are 
explicitely written in the asm() statement.  If it does so, I consider 
this a serious bug in gcc.

Ciao,
Dscho

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Alexander Graf wrote:

> On Jan 18, 2008, at 2:14 AM, consul wrote:
> 
> > It broke mingw build with gcc-3.4.2
> 
> Does it work if you change the lines I'll comment on below?

Yes, if I force the pushing and popping of ecx, it compiles 
(unfortunately, I could not test the result yet).  But does

> > > +#ifdef GCC_BREAKS_T_REGISTER
> > > +                  : "%eax", "%edx", "memory", "cc");
> > > +#else
> > >                 : "%eax", "%ecx", "%edx", "memory", "cc");
> > > +#endif

not make

> > > +#ifdef GCC_BREAKS_T_REGISTER
> > > +                  "pushl %%ecx\n"
> > > +#endif

and

> > > +#ifdef GCC_BREAKS_T_REGISTER
> > > +                  "popl %%ecx\n"
> > > +#endif

obsolete (it's been a long time since I did assembler stuff, so forgive 
me this stupid question)?

Ciao,
Dscho

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Michael Matz]

Hi,

On Fri, 18 Jan 2008, Johannes Schindelin wrote:

> > > > asm (" ... movzbl %b1, %%edx\n ... " : : "r" (blubb), "r" (bla) );
> > > 
> > > Okay, but this only concerns gcc4, apparently.
> > 
> > No, it's nothing to do with GCC.
> 
> But apparently it has!  With gcc < 4 I did never get the error.

As I tried to explain, this is pure luck.

> Which probably means that gcc < 4 did _not_ use ecx, and therefore it 
> does not have to be pushed and popped.

We are talking about the hunk using the "q" constraint for operand 1 in 
st[bw]_kernel.  The change in the clobber list (and the associated 
saving/restoring of %ecx around the call) is something entirely different.

> Which -- judging from how commonly glue() is called in op.c -- could 
> mean a performance hit.

glue() is a macro, the function called is stw_kernel (inline function).

> I am all for supporting gcc > 3, but please, please not at the cost of 
> having a performance hit for _existing_ users.

Have you measured this?  This function actually does a call to stw_mmu, a 
rather slow and big function, the overhead of one register store more or 
less is probably zero.

But that point is mood anyway.  When it works without the "q" constraint 
in gcc 3.4.2 it only does so, because GCC allocates one of the ax-dx 
registers to that operand (by luck, not by design).  As T1 is coming in in 
esi there anyway existed a reg-reg move already, so you pay that 
performance hit (if you like to call it such) already.

> > Only if you want to trust your luck.  I fear I don't have gcc 3.4.2 
> > lying around anywhere, so I can't really help debugging this reload 
> > breakage in that GCC version.  It might help to introduce a temporary to 
> > guide GCC through this problematic reload case by detaching the global 
> > register variable from the asm operand.  For cases where it's no problem 
> > this should be optimized away, so doesn't inhibit a performance cost.  
> > What I mean is something like the below.  If someone with gcc 3.4.2 
> > could test that ...
> 
> I do ask myself how gcc would optimise away instructions that are 
> explicitely written in the asm() statement.  If it does so, I consider 
> this a serious bug in gcc.

My patch in the last mail introduces a copy in C (to vtmp), _that_ can be 
optimized away under the right circumstances.  Of course GCC does not 
change the asm template in any way.


Ciao,
Michael.

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Michael Matz wrote:

> On Fri, 18 Jan 2008, Johannes Schindelin wrote:
> 
> > > > > asm (" ... movzbl %b1, %%edx\n ... " : : "r" (blubb), "r" (bla) );
> > > > 
> > > > Okay, but this only concerns gcc4, apparently.
> > > 
> > > No, it's nothing to do with GCC.
> > 
> > But apparently it has!  With gcc < 4 I did never get the error.
> 
> As I tried to explain, this is pure luck.

Maybe.

> > Which probably means that gcc < 4 did _not_ use ecx, and therefore it 
> > does not have to be pushed and popped.
> 
> We are talking about the hunk using the "q" constraint for operand 1 in 
> st[bw]_kernel.  The change in the clobber list (and the associated 
> saving/restoring of %ecx around the call) is something entirely different.

It cannot be, because just changing the clobber list makes the code 
compile again!

Of course, you are now going to say that this is pure luck, too...

> > Which -- judging from how commonly glue() is called in op.c -- could 
> > mean a performance hit.
> 
> glue() is a macro, the function called is stw_kernel (inline function).

Okay... I did not look closely enough (lack of time).

> > I am all for supporting gcc > 3, but please, please not at the cost of 
> > having a performance hit for _existing_ users.
> 
> Have you measured this?  This function actually does a call to stw_mmu, a 
> rather slow and big function, the overhead of one register store more or 
> less is probably zero.

No, lack of time.

> My patch in the last mail introduces a copy in C (to vtmp), _that_ can 
> be optimized away under the right circumstances.  Of course GCC does not 
> change the asm template in any way.

I must have missed that mail.

Ciao,
Dscho

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Michael Matz]

Hi,

On Fri, 18 Jan 2008, Johannes Schindelin wrote:

> > > But apparently it has!  With gcc < 4 I did never get the error.
> > 
> > As I tried to explain, this is pure luck.
> 
> Maybe.
> 
> > > Which probably means that gcc < 4 did _not_ use ecx, and therefore it 
> > > does not have to be pushed and popped.
> > 
> > We are talking about the hunk using the "q" constraint for operand 1 in 
> > st[bw]_kernel.  The change in the clobber list (and the associated 
> > saving/restoring of %ecx around the call) is something entirely different.
> 
> It cannot be, because just changing the clobber list makes the code 
> compile again!

But I'm not talking about the clobber list at all.  I reacted to the first 
mail forwarded to me, which was a question specifically about the hunk 
adding the "q" constraint, whose purpose I explained.  Are you now also 
asking about the changes regarding %ecx and the clobber list?


Ciao,
Michael.

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Michael Matz wrote:

> But I'm not talking about the clobber list at all.  I reacted to the 
> first mail forwarded to me, which was a question specifically about the 
> hunk adding the "q" constraint, whose purpose I explained.  Are you now 
> also asking about the changes regarding %ecx and the clobber list?

Okay, look, I do not have enough time to fix it myself.  But there _is_ a 
breakage with gcc 3.4.2 on MinGW.  If that is not fixed, I will openly 
oppose that patch going into CVS.

All I tried was to get an understanding why the current patch "Fix i386" 
(which is still misnamed) broke on MinGW.

Seems like I am not able to get that understanding, but at least it 
compiles again with those "#ifdef GCC..." removed.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Andreas Färber]

Am 17.01.2008 um 18:11 schrieb Andreas Färber:

> Hi,
>
> Am 17.01.2008 um 16:29 schrieb Johannes Schindelin:
>
>> I found two extra #endifs, in target-mips/exec.h and target-ppc/ 
>> exec.h.  I
>> updated my "gcc4" branch.
>
> I tested sparc-softmmu, ppc-softmmu, ppc64-softmmu, i386-softmmu on  
> Linux/i386 from your gcc4 branch. It compiles okay on gcc (GCC)  
> 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). Runtime  
> behavior was identical to CVS+gcc-3.4.

This starts to have the feeling of a Bug Week. ;) Cheers to everyone  
else testing or fixing.

To complete my test report from OSX/ppc as requested:

It turns out that I can compile QEMU with gcc-3.3 on Leopard/ppc with  
the always_inline fix applied: QEMU's recursive redefinition of inline/ 
always_inline seems to break Leopard's headers rather than gcc3 itself.

i386-darwin-user and ppc-darwin-user failed to compile (same from CVS):

gcc -Wall -O2 -g -fno-strict-aliasing -I. -I.. -I/Users/andreas/Q/ 
dscho/target-i386 -I/Users/andreas/Q/dscho -MMD -MP -DNEED_CPU_H -I/ 
Users/andreas/Q/dscho/darwin-user -I/Users/andreas/Q/dscho/darwin-user/ 
i386 -D__powerpc__ -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 - 
D_LARGEFILE_SOURCE -I/Users/andreas/Q/dscho/fpu -DHAS_AUDIO - 
DHAS_AUDIO_CHOICE -I/Users/andreas/Q/dscho/slirp  -mdynamic-no-pic  -c  
-o main.o /Users/andreas/Q/dscho/darwin-user/main.c
In file included from /Users/andreas/Q/dscho/darwin-user/qemu.h:9,
                  from /Users/andreas/Q/dscho/darwin-user/main.c:31:
../thunk.h: In function ‘thunk_type_size’:
../thunk.h:104: error: ‘TARGET_ABI_BITS’ undeclared (first use in this  
function)
../thunk.h:104: error: (Each undeclared identifier is reported only once
../thunk.h:104: error: for each function it appears in.)
../thunk.h: In function ‘thunk_type_align’:
../thunk.h:141: error: ‘TARGET_ABI_BITS’ undeclared (first use in this  
function)
In file included from /Users/andreas/Q/dscho/darwin-user/main.c:31:
/Users/andreas/Q/dscho/darwin-user/qemu.h: At top level:
/Users/andreas/Q/dscho/darwin-user/qemu.h:100: warning: ‘struct  
sigaltstack’ declared inside parameter list
/Users/andreas/Q/dscho/darwin-user/qemu.h:100: warning: its scope is  
only this definition or declaration, which is probably not what you want
make[1]: *** [main.o] Error 1
make: *** [subdir-i386-darwin-user] Error 2

ppc64-softmmu fails to compile (also from CVS):

gcc -Wall -O2 -g -fno-strict-aliasing  -fno-reorder-blocks  -fno-gcse   
-fno-tree-ch  -fno-optimize-sibling-calls  -fno-crossjumping  -fno- 
align-labels  -fno-align-jumps  -fno-align-functions   -mdynamic-no- 
pic  -I. -I.. -I/Users/andreas/Q/dscho/target-ppc -I/Users/andreas/Q/ 
dscho -MMD -MP -DNEED_CPU_H -D__powerpc__ -D_GNU_SOURCE - 
D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/Users/andreas/Q/dscho/fpu  
-DHAS_AUDIO -DHAS_AUDIO_CHOICE -I/Users/andreas/Q/dscho/slirp -c -o  
op.o /Users/andreas/Q/dscho/target-ppc/op.c
/Users/andreas/Q/dscho/target-ppc/op_mem.h: In function  
‘op_vr_stvx_raw’:
/Users/andreas/Q/dscho/target-ppc/op_mem.h:934: error: unrecognizable  
insn:
(insn:HI 16 18 17 0 ../cpu-all.h:561 (set (subreg:SI (reg:DI 122) 4)
         (plus:SI (mem/s/j:SI (plus:SI (reg/f:SI 119 [ env.1205 ])
                     (const_int 4 [0x4])) [0 <variable>.t0+4 S4 A32])
             (const_int 8 [0x8]))) -1 (insn_list:REG_DEP_TRUE 18 (nil))
     (nil))
/Users/andreas/Q/dscho/target-ppc/op_mem.h:934: internal compiler  
error: in extract_insn, at recog.c:2037
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://developer.apple.com/bugreporter> for instructions.
make[1]: *** [op.o] Error 1
make: *** [subdir-ppc64-softmmu] Error 2

sh4-softmmu and sh4eb-softmmu fail to compile (only from git):

gcc -Wall -O2 -g -fno-strict-aliasing  -fno-reorder-blocks  -fno-gcse   
-fno-tree-ch  -fno-optimize-sibling-calls  -fno-crossjumping  -fno- 
align-labels  -fno-align-jumps  -fno-align-functions   -mdynamic-no- 
pic  -I. -I.. -I/Users/andreas/Q/dscho/target-sh4 -I/Users/andreas/Q/ 
dscho -MMD -MP -DNEED_CPU_H -D__powerpc__ -D_GNU_SOURCE - 
D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/Users/andreas/Q/dscho/fpu  
-I/Users/andreas/Q/dscho/slirp -c -o op.o /Users/andreas/Q/dscho/ 
target-sh4/op.c
../dyngen-exec.h:236: warning: ‘__op_param3’ defined but not used
../dyngen -o op.h op.o
dyngen: blr expected at the end of op_cmp_str_T0_T1
make[1]: *** [op.h] Error 1
make: *** [subdir-sh4-softmmu] Error 2

cris-softmmu compiles from git but fails from CVS:

make -C cris-softmmu all
gcc-3.3 -Wall -O2 -g -fno-strict-aliasing -I. -I.. -I/Users/andreas/Q/ 
myqemu/target-cris -I/Users/andreas/Q/myqemu -MMD -MP -DNEED_CPU_H - 
D__powerpc__ -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE - 
I/Users/andreas/Q/myqemu/fpu -I/Users/andreas/Q/myqemu/slirp  - 
mdynamic-no-pic  -c -o vl.o /Users/andreas/Q/myqemu/vl.c
In file included from /Users/andreas/Q/myqemu/hw/hw.h:5,
                  from /Users/andreas/Q/myqemu/vl.c:24:
../qemu-common.h:62:17: cpu.h: No such file or directory
[...]
make[1]: *** [vl.o] Error 1
make: *** [subdir-cris-softmmu] Error 2

My usual suspects sparc-softmmu, ppc-softmmu, i386-softmmu run from  
git as before.
powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5465)

Andreas

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Michael Matz]

Hi,

On Fri, 18 Jan 2008, Johannes Schindelin wrote:

> > But I'm not talking about the clobber list at all.  I reacted to the 
> > first mail forwarded to me, which was a question specifically about the 
> > hunk adding the "q" constraint, whose purpose I explained.  Are you now 
> > also asking about the changes regarding %ecx and the clobber list?
> 
> Okay, look, I do not have enough time to fix it myself.  But there _is_ a 
> breakage with gcc 3.4.2 on MinGW.  If that is not fixed, I will openly 
> oppose that patch going into CVS.

That's fine and is why I sent a test patch which might fix the 
breakage when applied on top of the constraint patch.

> All I tried was to get an understanding why the current patch "Fix i386" 
> (which is still misnamed) broke on MinGW.

Well, I can tell you why, but it doesn't help you: the 3.4.2 compiler has 
different deficiencies in reload than the 4.x line of compilers.  To make 
the whole thingy work on all compilers trying and testing is required to 
avoid all these different deficiencies.  My patch is partly real bug fixes 
(the "q" constraint part for instance) and partly changes helping to 
lessen the register pressure which reload needs to fix (the %ecx thingy 
for instance).  Both of these might expose the bugs in 3.4.2, in which 
case we need to hack around those as well.  That's what I tried to do with 
the patch from http://article.gmane.org/gmane.comp.emulators.qemu/22762 , 
but it needs of course testing by someone who actually uses 3.4.2.


Ciao,
Michael.

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Andreas Färber]

Am 18.01.2008 um 15:43 schrieb Johannes Schindelin:

> On Fri, 18 Jan 2008, Michael Matz wrote:
>
>> But I'm not talking about the clobber list at all.  I reacted to the
>> first mail forwarded to me, which was a question specifically about  
>> the
>> hunk adding the "q" constraint, whose purpose I explained.  Are you  
>> now
>> also asking about the changes regarding %ecx and the clobber list?
>
> Okay, look, I do not have enough time to fix it myself.  But there  
> _is_ a
> breakage with gcc 3.4.2 on MinGW.  If that is not fixed, I will openly
> oppose that patch going into CVS.

No-one was asking for a patch that breaks things to go into CVS.

Andreas

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

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

Hi,

On Fri, 18 Jan 2008, Michael Matz wrote:

> On Fri, 18 Jan 2008, Johannes Schindelin wrote:
> 
> > > But I'm not talking about the clobber list at all.  I reacted to the 
> > > first mail forwarded to me, which was a question specifically about 
> > > the hunk adding the "q" constraint, whose purpose I explained.  Are 
> > > you now also asking about the changes regarding %ecx and the clobber 
> > > list?
> > 
> > Okay, look, I do not have enough time to fix it myself.  But there 
> > _is_ a breakage with gcc 3.4.2 on MinGW.  If that is not fixed, I will 
> > openly oppose that patch going into CVS.
> 
> That's fine and is why I sent a test patch which might fix the breakage 
> when applied on top of the constraint patch.
> 
> > All I tried was to get an understanding why the current patch "Fix 
> > i386" (which is still misnamed) broke on MinGW.
> 
> Well, I can tell you why, but it doesn't help you: the 3.4.2 compiler 
> has different deficiencies in reload than the 4.x line of compilers.  
> To make the whole thingy work on all compilers trying and testing is 
> required to avoid all these different deficiencies.  My patch is partly 
> real bug fixes (the "q" constraint part for instance) and partly changes 
> helping to lessen the register pressure which reload needs to fix (the 
> %ecx thingy for instance).  Both of these might expose the bugs in 
> 3.4.2, in which case we need to hack around those as well.  That's what 
> I tried to do with the patch from 
> http://article.gmane.org/gmane.comp.emulators.qemu/22762 , but it needs 
> of course testing by someone who actually uses 3.4.2.

I just downloaded it, and tested it on MinGW with 3.4.2, and I still get 
the "can't find register in class `Q_REGS´ while reloading `asm´" error.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Fabrice Bellard]

Alexander Graf wrote:
> This patch is mostly a cleanup of Michael Matz's patch with the ideas
> that came last time included.

I must say I don't like such patches because they are likely to break 
with every new GCC version.

Moreover, I will commit in the next few days a new code generator in 
QEMU which will utimately solve the compilation problems. In its current 
form it still relies on "dyngen" so that legacy micro operations still 
work, but in the end dyngen will be completely suppressed.

A side effect of these commits will be that only the x86 and x86_64 
hosts will be supported during some time.

Regards,

Fabrice.

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Michael Matz]

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

Hi,

On Fri, 18 Jan 2008, Johannes Schindelin wrote:

> > Well, I can tell you why, but it doesn't help you: the 3.4.2 compiler 
> > has different deficiencies in reload than the 4.x line of compilers.  
> > To make the whole thingy work on all compilers trying and testing is 
> > required to avoid all these different deficiencies.  My patch is 
> > partly real bug fixes (the "q" constraint part for instance) and 
> > partly changes helping to lessen the register pressure which reload 
> > needs to fix (the %ecx thingy for instance).  Both of these might 
> > expose the bugs in 3.4.2, in which case we need to hack around those 
> > as well.  That's what I tried to do with the patch from 
> > http://article.gmane.org/gmane.comp.emulators.qemu/22762 , but it 
> > needs of course testing by someone who actually uses 3.4.2.
> 
> I just downloaded it, and tested it on MinGW with 3.4.2, and I still get 
> the "can't find register in class `Q_REGS´ while reloading `asm´" error.

Bummer.  As a test of theory (I'm not proposing this as patch), can you 
see what happens if you make vtmp volatile, i.e. make it look like so ? :

#if DATA_SIZE == 1 || DATA_SIZE == 2
    volatile RES_TYPE vtmp = v;
#endif

Bah, remote work-arounding of reload sucks :-)


Ciao,
Michael.

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Fabrice Bellard wrote:

> Moreover, I will commit in the next few days a new code generator in 
> QEMU which will utimately solve the compilation problems. In its current 
> form it still relies on "dyngen" so that legacy micro operations still 
> work, but in the end dyngen will be completely suppressed.

Great!

> A side effect of these commits will be that only the x86 and x86_64 
> hosts will be supported during some time.

But didn't you say you depend on dyngen to counter that?  Well, I guess 
I'll see.

Ciao,
Dscho

Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

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

Hi,

On Fri, 18 Jan 2008, Michael Matz wrote:

> On Fri, 18 Jan 2008, Johannes Schindelin wrote:
> 
> > > Well, I can tell you why, but it doesn't help you: the 3.4.2 
> > > compiler has different deficiencies in reload than the 4.x line of 
> > > compilers.  To make the whole thingy work on all compilers trying 
> > > and testing is required to avoid all these different deficiencies.  
> > > My patch is partly real bug fixes (the "q" constraint part for 
> > > instance) and partly changes helping to lessen the register pressure 
> > > which reload needs to fix (the %ecx thingy for instance).  Both of 
> > > these might expose the bugs in 3.4.2, in which case we need to hack 
> > > around those as well.  That's what I tried to do with the patch from 
> > > http://article.gmane.org/gmane.comp.emulators.qemu/22762 , but it 
> > > needs of course testing by someone who actually uses 3.4.2.
> > 
> > I just downloaded it, and tested it on MinGW with 3.4.2, and I still 
> > get the "can't find register in class `Q_REGS´ while reloading `asm´" 
> > error.
> 
> Bummer.  As a test of theory (I'm not proposing this as patch), can you 
> see what happens if you make vtmp volatile, i.e. make it look like so ? :
> 
> #if DATA_SIZE == 1 || DATA_SIZE == 2
>     volatile RES_TYPE vtmp = v;
> #endif

Still no joy.  Exactly the same error.

> Bah, remote work-arounding of reload sucks :-)

Heh.  I am so looking forward to Fabrice's code generator, now that I know 
of it!

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Alexander Graf]

On Jan 18, 2008, at 4:41 PM, Fabrice Bellard wrote:

> Alexander Graf wrote:
>> This patch is mostly a cleanup of Michael Matz's patch with the ideas
>> that came last time included.
>
> I must say I don't like such patches because they are likely to  
> break with every new GCC version.
>

Basically these patches only fixed what was broken, so I don't see any  
way they make anything worse than what we have now. Especially because  
the only intrusive patches are the i386 ones, which are conditional  
for gcc4, so there really is nothing it breaks here. It merely removes  
the assumption that if registers can be used for temporary variables,  
there are 3 spare ones and fixes a bug which was officially marked as  
XXX.

> Moreover, I will commit in the next few days a new code generator in  
> QEMU which will utimately solve the compilation problems. In its  
> current form it still relies on "dyngen" so that legacy micro  
> operations still work, but in the end dyngen will be completely  
> suppressed.
>

I don't really like the idea of throwing away all the micro-ops we  
have so far. Quite a lot of effort was put into writing this code and  
I don't know if the people who committed all those targets actually  
want to do the same work once again because there is a new approach  
again. Maybe if it's way superior and faster, but if everything it  
does is fixing the gcc dependency I'm not that enthusiastic.

> A side effect of these commits will be that only the x86 and x86_64  
> hosts will be supported during some time.
>

Great. Please make it optional then ;-).

Regards,

Alex

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Alexander Graf wrote:

> On Jan 18, 2008, at 4:41 PM, Fabrice Bellard wrote:
> 
> > Alexander Graf wrote:
> > > This patch is mostly a cleanup of Michael Matz's patch with the 
> > > ideas that came last time included.
> > 
> > I must say I don't like such patches because they are likely to break 
> > with every new GCC version.
> 
> Basically these patches only fixed what was broken, so I don't see any 
> way they make anything worse than what we have now. Especially because 
> the only intrusive patches are the i386 ones, which are conditional for 
> gcc4, so there really is nothing it breaks here. It merely removes the 
> assumption that if registers can be used for temporary variables, there 
> are 3 spare ones and fixes a bug which was officially marked as XXX.

But that broke a previously working system, and that's why I agree with 
Fabrice.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Markus Hitter]

Am 18.01.2008 um 19:10 schrieb Johannes Schindelin:

> But that broke a previously working system, and that's why I agree  
> with
> Fabrice.

At the same time it made a more modern system work. Refusing a patch  
because it exposes existing bugs isn't exactly intelligent.


Markus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Markus Hitter wrote:

> Am 18.01.2008 um 19:10 schrieb Johannes Schindelin:
> 
> > But that broke a previously working system, and that's why I agree 
> > with Fabrice.
> 
> At the same time it made a more modern system work. Refusing a patch 
> because it exposes existing bugs isn't exactly intelligent.

In the world I live, introducing regressions is bad.  Even if another 
system starts working, if you break existing users, you did something 
wrong.  And if you don't care, and don't mind giving existing users a hard 
time, you cannot be helped and should go somewhere else.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Markus Hitter]

Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:

> Even if another system starts working, if you break existing users,  
> you did something
> wrong.  And if you don't care, and don't mind giving existing users  
> a hard time, you cannot be helped and should go somewhere else.

So you have to be backwards oriented and willing to live with bugs to  
join your world. Good to know.


Markus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Sat, 19 Jan 2008, Markus Hitter wrote:

> Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:
> 
> > Even if another system starts working, if you break existing users, 
> > you did something wrong.  And if you don't care, and don't mind giving 
> > existing users a hard time, you cannot be helped and should go 
> > somewhere else.
> 
> So you have to be backwards oriented and willing to live with bugs to 
> join your world. Good to know.

How dare you misrepresenting my words like that?  Cutting out the first 
sentence which read something like "supporting new systems is good"?

Pissed,
Dscho

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Markus Hitter]

Am 19.01.2008 um 12:16 schrieb Johannes Schindelin:

> Hi,
>
> On Sat, 19 Jan 2008, Markus Hitter wrote:
>
>> Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:
>>
>>> Even if another system starts working, if you break existing users,
>>> you did something wrong.  And if you don't care, and don't mind  
>>> giving
>>> existing users a hard time, you cannot be helped and should go
>>> somewhere else.
>>
>> So you have to be backwards oriented and willing to live with bugs to
>> join your world. Good to know.
>
> How dare you misrepresenting my words like that?

Because for me it's what you essentially said.

In search of a friendly agreement I'll continue off list.


Markus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/

Re: [Qemu-devel] [PATCH 1/5] Fix i386 Host

[Johannes Schindelin]

Hi,

On Sat, 19 Jan 2008, Markus Hitter wrote:

> Am 19.01.2008 um 12:16 schrieb Johannes Schindelin:
> 
> > On Sat, 19 Jan 2008, Markus Hitter wrote:
> > 
> > > Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:
> > > 
> > > > Even if another system starts working, if you break existing 
> > > > users, you did something wrong.  And if you don't care, and don't 
> > > > mind giving existing users a hard time, you cannot be helped and 
> > > > should go somewhere else.
> > > 
> > > So you have to be backwards oriented and willing to live with bugs 
> > > to join your world. Good to know.
> > 
> > How dare you misrepresenting my words like that?
> 
> Because for me it's what you essentially said.
> 
> In search of a friendly agreement I'll continue off list.

I think the open assumption that I am "backwards oriented" just for 
claiming that breaking existing setups made a friendly agreement rather 
hard to reach.

So this discussion is no longer wasting my time from...

right now