Make use of temporaries as registers conditional (fixes gcc4 i386 host)

PATCH RESEND In review Alexander Graf 2 participants 4 messages Jan 21, 2008

lore

Share & tools

lore

Alexander Graf

author

2008-01-21 07:38 UTC #0 lore raw

PATCH RESEND 1/1 Make use of temporaries as registers conditional (fixes gcc4 i386 host)

Hi,

this is a mostly a resend of the very same patch I sent to the list a few days ago. I believe I fixed most issues (wrong endif, gcc3 breakage). Please tell me if this patch breaks already working setups, as it should only make _more_ things work than before.

If anybody has a really valid point for this to not be included into qemu, please tell me. I don't really see "I don't like fixing gcc dependencies" as such, as that's what had to be done for gcc3 too already.

Regards,

Alex

softmmu_header.h +14 -0

Index: 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) && GCC_BREAKS_T_REGISTER
+                  "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

target-alpha/cpu.h +2 -0

Index: 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;

target-alpha/exec.h +6 -0

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

target-arm/cpu.h +3 -0

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

target-arm/exec.h +5 -0

Index: 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.  */

target-i386/cpu.h +2 -0

Index: 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 */

target-i386/exec.h +6 -0

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

target-mips/cpu.h +2 -0

Index: 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];

target-mips/exec.h +4 -0

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

target-ppc/exec.h +4 -0

Index: qemu/target-ppc/exec.h
===================================================================
@@ -38,10 +38,14 @@ 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);
 #define TDX "%016lx"
 #endif
 /* We may, sometime, need 64 bits registers on 32 bits targets */

target-sparc/exec.h +5 -1

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

configure +6 -0

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

Alexander Graf

author

2008-01-21 07:57 UTC #1 lore raw

Quoted 7 lines · Alexander Graf

Hi,
this is a mostly a resend of the very same patch I sent to the list a few days ago. I believe I fixed most issues (wrong endif, gcc3 breakage). Please tell me if this patch breaks already working setups, as it should only make _more_ things work than before.

Quoted 8 lines

If anybody has a really valid point for this to not be included into qemu, please tell me. I don't really see "I don't like fixing gcc dependencies" as such, as that's what had to be done for gcc3 too already.
Regards,
Alex

Oops, wrong version :-)

Here we go again...

Alex

softmmu_header.h +14 -0

Index: 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) && defined(GCC_BREAKS_T_REGISTER)
+                  "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

target-alpha/cpu.h +2 -0

Index: 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;

target-alpha/exec.h +6 -0

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

target-arm/cpu.h +3 -0

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

target-arm/exec.h +5 -0

Index: 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.  */

target-i386/cpu.h +2 -0

Index: 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 */

target-i386/exec.h +6 -0

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

target-mips/cpu.h +2 -0

Index: 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];

target-mips/exec.h +4 -0

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

target-ppc/exec.h +4 -0

Index: qemu/target-ppc/exec.h
===================================================================
@@ -38,10 +38,14 @@ 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);
 #define TDX "%016lx"
 #endif
 /* We may, sometime, need 64 bits registers on 32 bits targets */

target-sparc/exec.h +5 -1

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

configure +6 -0

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

Mulyadi Santosa

2008-01-21 08:19 UTC #2 lore raw

On 1/21/08, Alexander Graf <[email protected]> wrote:
Hi..

Quoted 4 lines

Alexander Graf wrote:
Oops, wrong version :-)
Here we go again...

Bummer! Almost applied that...:) Sure it's the right one? :D

regards,

Mulyadi

Alexander Graf

author

2008-01-21 09:08 UTC #3 lore raw

Quoted 9 lines · Mulyadi Santosa

On 1/21/08, Alexander Graf <[email protected]> wrote:
Hi..
Quoted 4 lines
Alexander Graf wrote:
Oops, wrong version :-)
Here we go again...
Bummer! Almost applied that...:) Sure it's the right one? :D

Pretty sure, yes ;-). It still doesn't fix x86_64 target for me though. Nevertheless it's a step in the right direction as far as I'm concerned.

Regards,

Alex