Fix i386 Host

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

Hi,

Quoted 3 lines · Alexander Graf

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

Quoted 9 lines

Index: qemu/softmmu_header.h
===================================================================

softmmu_header.h +1 -0

@@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+                  "pushl %%ecx\n"

softmmu_header.h static inline void glue(glue(st, SUFFIX)

Quoted 5 lines

softmmu_header.h +1 -0

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

Quoted 12 lines · Johannes Schindelin

Hi,

Quoted 3 lines · Alexander Graf

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

Quoted 9 lines

Quoted 9 lines

Index: qemu/softmmu_header.h
===================================================================

softmmu_header.h +1 -0

@@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+                  "pushl %%ecx\n"

Quoted 10 lines

softmmu_header.h static inline void glue(glue(st, SUFFIX)

Quoted 5 lines

softmmu_header.h +1 -0

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

Quoted 8 lines · Johannes Schindelin

Hi,

Quoted 2 lines · Alexander Graf

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

Quoted 9 lines

Quoted 9 lines

Index: qemu/softmmu_header.h
===================================================================

softmmu_header.h +1 -0

@@ -189,9 +189,11 @@ static inline void glue(glue(st, SUFFIX)
 #else
 #error unsupported size
 #endif
+                  "pushl %%ecx\n"

Quoted 8 lines

softmmu_header.h static inline void glue(glue(st, SUFFIX)

Quoted 5 lines

softmmu_header.h +1 -0

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

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

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

Quoted 4 lines · Alexander Graf

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.

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?

Quoted 12 lines · 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

Quoted 8 lines

#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

Quoted 16 lines · Alexander Graf

Quoted 14 lines · Jens Arm

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

Quoted 5 lines

Quoted 1 line

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.

Hi,

On Thu, 17 Jan 2008, Johannes Schindelin wrote:

Quoted 4 lines

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

Quoted 10 lines · Johannes Schindelin

Hi,

Quoted 4 lines · Johannes Schindelin

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

Quoted 3 lines

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

Right, thanks.

New patch attached.

Quoted 5 lines · Johannes Schindelin

Hi,

On Thu, 17 Jan 2008, Johannes Schindelin wrote:

Quoted 8 lines

Quoted 4 lines

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
===================================================================

softmmu_header.h +14 -0

@@ -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
===================================================================

target-alpha/cpu.h +2 -0

@@ -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
===================================================================

target-alpha/exec.h +6 -0

@@ -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
===================================================================

target-arm/cpu.h +3 -0

@@ -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
===================================================================

target-arm/exec.h +5 -0

@@ -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
===================================================================

target-i386/cpu.h +2 -0

@@ -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
===================================================================

target-i386/exec.h +6 -0

@@ -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
===================================================================

target-mips/cpu.h +2 -0

@@ -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
===================================================================

target-mips/exec.h +4 -0

@@ -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
===================================================================

target-ppc/exec.h +5 -0

@@ -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
===================================================================

target-sparc/exec.h +5 -1

@@ -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
===================================================================

configure +0 -0

@@ -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

configure +6 -0

+
+  # 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

It broke mingw build with gcc-3.4.2

Quoted 1 line · consul

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"?

Quoted 42 lines

From: Johannes Schindelin <[email protected]>
Date: January 18, 2008 3:05:58 AM GMT+01:00
To: consul <[email protected]>
Cc: [email protected]
Subject: Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host
Reply-To: [email protected]

Hi,

Quoted 8 lines · Johannes Schindelin

Quoted 1 line · consul

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

softmmu_header.h +0 -0

index 80eefa8..7e4bc03 100644
@@ -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 ! */

softmmu_header.h +4 -1

+#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 --

Quoted 1 line

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

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

Quoted 34 lines

Quoted 34 lines · Johannes Schindelin

Quoted 9 lines · consul

Quoted 2 lines

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

softmmu_header.h +0 -0

index 80eefa8..7e4bc03 100644
@@ -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 ! */

softmmu_header.h +4 -1

+#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 --

Quoted 1 line

Quoted 1 line

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?

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

(quoted diff) +4 -1

+#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 --

Quoted 6 lines

Quoted 1 line

Quoted 1 line

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

Quoted 15 lines · Johannes Schindelin

Quoted 15 lines

Quoted 11 lines

Quoted 11 lines

(quoted diff) +4 -1

+#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):

Quoted 3 lines

Quoted 1 line

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

Okay, but this only concerns gcc4, apparently.

No, it's nothing to do with GCC.

Quoted 1 line

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

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

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.

Quoted 8 lines

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.

Quoted 7 lines · Johannes Schindelin

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 2 lines

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

Quoted 2 lines

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.

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.

Quoted 3 lines

Quoted 1 line

But apparently it has! With gcc < 4 I did never get the error.

As I tried to explain, this is pure luck.

Maybe.

Quoted 3 lines

Quoted 2 lines

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

Quoted 3 lines

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?

Quoted 4 lines

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.

Quoted 9 lines · Johannes Schindelin

Quoted 5 lines

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.

Quoted 2 lines

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.

Quoted 11 lines

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.

Quoted 12 lines · Johannes Schindelin

Quoted 11 lines

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.

Quoted 3 lines

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

Quoted 5 lines

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.

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

(quoted diff) +0 -3

-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" <[email protected]> wrote in message news:[email protected]...

Quoted 15 lines

Quoted 5 lines · Johannes Schindelin

Hi,

On Thu, 17 Jan 2008, Johannes Schindelin wrote:

Quoted 8 lines

Quoted 4 lines

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

Quoted 6 lines

Right, thanks.

New patch attached.

Alex

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

softmmu_header.h static inline void glue(glue(st, SUFFIX) · line 194

Quoted 11 lines

Index: qemu/softmmu_header.h
===================================================================

softmmu_header.h +3 -0

@@ -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

softmmu_header.h static inline void glue(glue(st, SUFFIX) · line 200

Quoted 7 lines

softmmu_header.h +3 -0

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

Quoted 18 lines

softmmu_header.h +4 -0

                 "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)),

softmmu_header.h static inline void glue(glue(st, SUFFIX) · line 233

Quoted 8 lines

softmmu_header.h +4 -0

                 "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

Quoted 181 lines

softmmu_header.h +0 -0

 }

#else
Index: qemu/target-alpha/cpu.h
===================================================================

target-alpha/cpu.h +2 -0

@@ -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
===================================================================

target-alpha/exec.h +6 -0

@@ -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
===================================================================

target-arm/cpu.h +3 -0

@@ -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
===================================================================

target-arm/exec.h +5 -0

@@ -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
===================================================================

target-i386/cpu.h +2 -0

@@ -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
===================================================================

target-i386/exec.h +6 -0

@@ -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
===================================================================

target-mips/cpu.h +2 -0

@@ -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
===================================================================

target-mips/exec.h +4 -0

@@ -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
===================================================================

target-ppc/exec.h +5 -0

@@ -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
===================================================================

target-sparc/exec.h +5 -1

@@ -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
===================================================================

configure +0 -0

@@ -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

configure +6 -0

+
+  # 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

Quoted 1 line · consul

It broke mingw build with gcc-3.4.2

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

Quoted 5 lines

softmmu_header.h static inline void glue(glue(st, SUFFIX) · line 233

Quoted 5 lines

softmmu_header.h +4 -0

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

Quoted 3 lines

softmmu_header.h static inline void glue(glue(st, SUFFIX) · line 194

Quoted 3 lines

softmmu_header.h +3 -0

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

Quoted 3 lines

softmmu_header.h static inline void glue(glue(st, SUFFIX) · line 200

Quoted 3 lines

softmmu_header.h +3 -0

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

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

(quoted diff) +0 -3

-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" <[email protected]> wrote in message news:[email protected]...

Quoted 15 lines

Quoted 5 lines · Johannes Schindelin

Hi,

On Thu, 17 Jan 2008, Johannes Schindelin wrote:

Quoted 8 lines

Quoted 4 lines

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

Quoted 6 lines

Right, thanks.

New patch attached.

Alex

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

Quoted 222 lines

Index: qemu/softmmu_header.h
===================================================================

softmmu_header.h +14 -0

@@ -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
===================================================================

target-alpha/cpu.h +2 -0

@@ -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
===================================================================

target-alpha/exec.h +6 -0

@@ -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
===================================================================

target-arm/cpu.h +3 -0

@@ -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
===================================================================

target-arm/exec.h +5 -0

@@ -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
===================================================================

target-i386/cpu.h +2 -0

@@ -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
===================================================================

target-i386/exec.h +6 -0

@@ -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
===================================================================

target-mips/cpu.h +2 -0

@@ -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
===================================================================

target-mips/exec.h +4 -0

@@ -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
===================================================================

target-ppc/exec.h +5 -0

@@ -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
===================================================================

target-sparc/exec.h +5 -1

@@ -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
===================================================================

configure +0 -0

@@ -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

configure +6 -0

+
+  # 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

Hi,

Quoted 25 lines · Alexander Graf

Quoted 17 lines · Alexander Graf

Quoted 14 lines · Jens Arm

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

Quoted 6 lines

Quoted 2 lines

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.

Quoted 6 lines

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

Quoted 37 lines · Johannes Schindelin

Hi,

Quoted 31 lines · Alexander Graf

Quoted 17 lines · Alexander Graf

Quoted 14 lines · Jens Arm

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

Quoted 6 lines

Quoted 2 lines

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.

Quoted 6 lines

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.

Hi,

Am 17.01.2008 um 16:29 schrieb Johannes Schindelin:

Quoted 2 lines

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

Quoted 1 line

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.

Am 17.01.2008 um 18:34 schrieb Alexander Graf:

Quoted 3 lines

Quoted 1 line

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

Quoted 7 lines · Andreas Färber

Am 17.01.2008 um 18:34 schrieb Alexander Graf:

Quoted 3 lines

Quoted 1 line

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.

Quoted 9 lines

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.

Am 17.01.2008 um 18:34 schrieb Alexander Graf:

Quoted 3 lines

Quoted 1 line

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?

Quoted 6 lines

Am 17.01.2008 um 18:34 schrieb Alexander Graf:

Quoted 2 lines

Quoted 1 line

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.

Quoted 3 lines

Quoted 1 line

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.

Hi,

Am 17.01.2008 um 16:29 schrieb Johannes Schindelin:

Quoted 2 lines

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.

Quoted 23 lines · Alexander Graf

Quoted 12 lines · 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

Quoted 8 lines

#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?

Quoted 8 lines

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

Quoted 2 lines · Alexander Graf

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.

Index: qemu/target-alpha/cpu.h
===================================================================

target-alpha/cpu.h +2 -0

@@ -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
===================================================================

Index: qemu/target-sparc/exec.h
===================================================================

target-sparc/exec.h +0 -0

@@ -32,10 +32,14 @@ register uint32_t T2 asm(AREG4);
 
 #else

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

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.

Quoted 2 lines · Alexander Graf

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.

Quoted 6 lines · Fabrice Bellard

Quoted 2 lines · Alexander Graf

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.

Am 18.01.2008 um 19:10 schrieb Johannes Schindelin:

Quoted 2 lines

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.

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.

Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:

Quoted 4 lines

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.

Hi,

Quoted 7 lines · Markus Hitter

Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:

Quoted 4 lines

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.

Quoted 2 lines

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?

Am 19.01.2008 um 12:16 schrieb Johannes Schindelin:

Quoted 9 lines · Markus Hitter

Quoted 8 lines

Am 18.01.2008 um 20:28 schrieb Johannes Schindelin:

Quoted 4 lines

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.

Quoted 4 lines

Quoted 2 lines

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.