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

[Michael Matz <matz@suse.de>]

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