Re: [Qemu-devel] Re: Qemu 32-bit i386, gcc >= 3.4 spill error fix

["Ben Taylor" <bentaylor.solx86@gmail.com>]

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

On Sun, May 4, 2008 at 11:56 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> On Sun, May 04, 2008 at 09:13:52PM -0400, Ben Taylor wrote:
>  > On Sun, May 4, 2008 at 5:41 PM, Ben Taylor <bentaylor.solx86@gmail.com> wrote:
>  > > This patch applies some additional flags to op.c and helper.c
>  > >  for compiling on i386 32-bit systems with gcc >= 3.4.
>  > >
>  > >  This has been tested on both ubuntu 7.10/32-bit and
>  > >  Solaris SXCE/32-bit with both gcc-3.3 and gcc-3.4
>  > >  and the behavior is consistent between both.
>  > >
>  > >  This is just a short term fix til the SSE ops are converted
>  > >  to TCG.
>  > >
>  >
>  > I updated the patch to take care of -mtune options that might
>  > be set by --extra-cflags, as well as the -march flags. I added
>  > support for athlon-xp, as someone mentioned it in #qemu
>  > with the exact error that helper.c sees in this context, so I
>  > fixed it as I have an athlon-xp.
>  >
>  > Ben
>  >
>  > diff -ruN qemu.ORIG/Makefile.target qemu/Makefile.target
>  > --- qemu.ORIG/Makefile.target 2008-05-04 17:21:08.000000000 -0400
>  > +++ qemu/Makefile.target      2008-05-04 20:58:47.000000000 -0400
>  > @@ -96,6 +96,12 @@
>  >  ifeq ($(ARCH),i386)
>  >  HELPER_CFLAGS+=-fomit-frame-pointer
>  >  OP_CFLAGS+=-mpreferred-stack-boundary=2 -fomit-frame-pointer
>  > +# OP_CFLAGS needs this on 32-bit x86 system to avoid
>  > +# a compiler spill error. This can probably go away
>  > +# once the SSE ops have been converted to TCG
>  > +ifeq ($(HAVE_GT_GCC_3_3), true)
>  > +MTUNE_CFLAGS=-march=i486 -mtune=i686
>
>  MTUNE is not really a good name, given it overrides both -mtune and
>  -march. What about I386_CFLAGS?

Done.

>  > @@ -309,7 +315,7 @@
>  >       $(DYNGEN) -g -o $@ $<
>  >
>  >  op.o: op.c
>  > -     $(CC) $(OP_CFLAGS) $(CPPFLAGS) -c -o $@ $<
>  > +     $(CC) $(OP_CFLAGS) $(CPPFLAGS) $(MTUNE_CFLAGS) -c -o $@ $<
>  >
>  >  machine.o: machine.c
>  >       $(CC) $(OP_CFLAGS) $(CPPFLAGS) -c -o $@ $<
>  > @@ -317,9 +323,24 @@
>  >  # HELPER_CFLAGS is used for all the code compiled with static register
>  >  # variables
>  >  ifeq ($(TARGET_BASE_ARCH), i386)
>  > +  # if current host is i386 and gcc >= -3.4, CFLAGS requires some
>  > +  # handling as helper.c generates a compiler spill error if
>  > +  # "-march=i686 -mtune=i686" or -"march=athlon-xp -mtune=athlon-xp"
>  > +  # We set MTUNE_CFLAGS="-march=i486 -mtune=i686" here
>  > +  # Once SSE ops are converted to TCG, this can go away
>  > +  HCFLAGS=$(CFLAGS)
>  > +  ifeq ($(ARCH), i386)
>  > +    ifeq ($(HAVE_GT_GCC_3_3), true)
>  > +      # if CFLAGS has -march=i686, remove it so helper.o
>  > +      # can compile on 32-bit intel systems
>  > +      HCFLAGS=`echo $(CFLAGS) | sed 's,-march=i686,,' | sed 's,-mtune=i686,,' \
>  > +        | sed 's,-march=athlon-xp,,' |  sed 's,-mtune=athlon-xp,,'`
>  > +    endif
>  > +  endif
>  > +
>
>  This part looks overcomplicated, and probably doesn't handle all cases.
>  What's the point of trying to remove options from CFLAGS? Multiple
>  -march or -mtune should be harmless, gcc should use the latest provided
>  one. So just make sure MTUNE_CFLAGS or I386_CFLAGS is after CFLAGS.

Done.  Because I had put the flags forward, they were getting over-riden by
CFLAGS.  Now it is much simpler and we don't care what the user puts
in --extra-cflags.

Ben

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: qemu-gcc-3.4-spill-error-v3.patch --]
[-- Type: text/x-patch; name=qemu-gcc-3.4-spill-error-v3.patch, Size: 1748 bytes --]

diff -ruN qemu.ORIG/configure qemu/configure
--- qemu.ORIG/configure	2008-05-04 17:21:08.000000000 -0400
+++ qemu/configure	2008-05-04 17:36:55.000000000 -0400
@@ -1199,6 +1199,13 @@
     then
       echo "#define USE_KQEMU 1" >> $config_h
     fi
+    gcc3minver=`$cc --version 2> /dev/null| fgrep "(GCC) 3." | awk '{ print $3 }' | cut -f2 -d.`
+    if test -n "$gcc3minver" -a $gcc3minver -gt 3
+    then
+      echo "HAVE_GT_GCC_3_3=true" >> $config_mak
+    else
+      echo "HAVE_GT_GCC_3_3=false" >> $config_mak
+    fi
   ;;
   x86_64)
     echo "TARGET_ARCH=x86_64" >> $config_mak
diff -ruN qemu.ORIG/Makefile.target qemu/Makefile.target
--- qemu.ORIG/Makefile.target	2008-05-04 17:21:08.000000000 -0400
+++ qemu/Makefile.target	2008-05-05 01:15:09.000000000 -0400
@@ -96,6 +96,12 @@
 ifeq ($(ARCH),i386)
 HELPER_CFLAGS+=-fomit-frame-pointer
 OP_CFLAGS+=-mpreferred-stack-boundary=2 -fomit-frame-pointer
+# op.c and helper.c need this on 32-bit x86 system to avoid
+# a compiler spill error. This can probably go away
+# once the SSE ops have been converted to TCG
+ifeq ($(HAVE_GT_GCC_3_3), true)
+I386_CFLAGS=-march=i486 -mtune=i686
+endif
 endif
 
 ifeq ($(ARCH),ppc)
@@ -309,7 +315,7 @@
 	$(DYNGEN) -g -o $@ $<
 
 op.o: op.c
-	$(CC) $(OP_CFLAGS) $(CPPFLAGS) -c -o $@ $<
+	$(CC) $(OP_CFLAGS) $(CPPFLAGS) $(I386_CFLAGS) -c -o $@ $<
 
 machine.o: machine.c
 	$(CC) $(OP_CFLAGS) $(CPPFLAGS) -c -o $@ $<
@@ -319,7 +325,7 @@
 ifeq ($(TARGET_BASE_ARCH), i386)
 # XXX: rename helper.c to op_helper.c
 helper.o: helper.c
-	$(CC) $(HELPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<
+	$(CC) $(HELPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(I386_CFLAGS) -c -o $@ $<
 else
 op_helper.o: op_helper.c
 	$(CC) $(HELPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) -c -o $@ $<