[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 <address@hidden>
> >Date: January 18, 2008 3:05:58 AM GMT+01:00
> >To: consul <address@hidden>
> >Cc: address@hidden
> >Subject: Re: [Qemu-devel] Re: [PATCH 1/5] Fix i386 Host
> >Reply-To: address@hidden
> >
> >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.