diff mbox

tests: Fix 'make test' for i686 hosts (build regression)

Message ID 1394187082-31986-1-git-send-email-sw@weilnetz.de
State Accepted
Headers show

Commit Message

Stefan Weil March 7, 2014, 10:11 a.m. UTC
'make test' is broken at least since commit
baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
to util/, and some of them there split, so add the missing prefix and new
files to fix the compiler and linker errors.

There remain more issues, but these changes allow running the test on a
Linux i686 host.

Cc: qemu-stable@nongnu.org
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

'make test' shows several problems where the results from native
execution and user mode emulation differ. Obviously the TCG code
or at least one helper function don't work as expected.

The patch might be useful for QEMU stable, too. I did not use it
with older versions, so I still don't know whether there was a TCG
regression and when it occurred.

Running 'make test' on an x86_64 host is currently not possible.
There is at least one -m32 compiler option missing, and a 32 bit
version of glib2.0 must be installed (not available for Debian
wheezy, so I had to stop here).

Regards
Stefan

 tests/tcg/test_path.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Alex Bennée March 7, 2014, 12:06 p.m. UTC | #1
Stefan Weil <sw@weilnetz.de> writes:

> 'make test' is broken at least since commit
> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
> to util/, and some of them there split, so add the missing prefix and new
> files to fix the compiler and linker errors.

I'm curious as why the Travis builds didn't pick this up?

https://travis-ci.org/qemu/qemu/builds

Does it require more libraries/features enabled to cause the breakage?

>
> There remain more issues, but these changes allow running the test on a
> Linux i686 host.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>
> 'make test' shows several problems where the results from native
> execution and user mode emulation differ. Obviously the TCG code
> or at least one helper function don't work as expected.
>
> The patch might be useful for QEMU stable, too. I did not use it
> with older versions, so I still don't know whether there was a TCG
> regression and when it occurred.
>
> Running 'make test' on an x86_64 host is currently not possible.
> There is at least one -m32 compiler option missing, and a 32 bit
> version of glib2.0 must be installed (not available for Debian
> wheezy, so I had to stop here).
>
> Regards
> Stefan
>
>  tests/tcg/test_path.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
> index a064eea..f8dd36a 100644
> --- a/tests/tcg/test_path.c
> +++ b/tests/tcg/test_path.c
> @@ -1,12 +1,15 @@
>  /* Test path override code */
>  #define _GNU_SOURCE
>  #include "config-host.h"
> -#include "iov.c"
> -#include "cutils.c"
> -#include "path.c"
> -#include "trace.c"
> +#include "util/cutils.c"
> +#include "util/hexdump.c"
> +#include "util/iov.c"
> +#include "util/path.c"
> +#include "util/qemu-timer-common.c"
> +#include "trace/control.c"
> +#include "../trace/generated-events.c"
>  #ifdef CONFIG_TRACE_SIMPLE
> -#include "../trace/simple.c"
> +#include "trace/simple.c"
>  #endif
>  
>  #include <stdarg.h>
Stefan Weil March 7, 2014, 12:40 p.m. UTC | #2
Am 07.03.2014 13:06, schrieb Alex Bennée:
> 
> Stefan Weil <sw@weilnetz.de> writes:
> 
>> 'make test' is broken at least since commit
>> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
>> to util/, and some of them there split, so add the missing prefix and new
>> files to fix the compiler and linker errors.
> 
> I'm curious as why the Travis builds didn't pick this up?
> 
> https://travis-ci.org/qemu/qemu/builds
> 
> Does it require more libraries/features enabled to cause the breakage?

Neither a simple 'make' nor a 'make check' will trigger the breakage
because no other target depends on 'test', so you have to add that
target explicitly.

In the meantime I tried older versions. Even version 1.1 already shows
unwanted differences between native and QEMU emulated FPU operations.

The test binary is now available from this URL:
http://qemu.weilnetz.de/test/user/i386/test-i386

Run it native and with qemu-i386 and compare the output of both runs
to see the problems.

Stefan
Andreas Färber March 7, 2014, 12:56 p.m. UTC | #3
Am 07.03.2014 13:40, schrieb Stefan Weil:
> Am 07.03.2014 13:06, schrieb Alex Bennée:
>>
>> Stefan Weil <sw@weilnetz.de> writes:
>>
>>> 'make test' is broken at least since commit
>>> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
>>> to util/, and some of them there split, so add the missing prefix and new
>>> files to fix the compiler and linker errors.
[...]
> In the meantime I tried older versions. Even version 1.1 already shows
> unwanted differences between native and QEMU emulated FPU operations.
> 
> The test binary is now available from this URL:
> http://qemu.weilnetz.de/test/user/i386/test-i386
> 
> Run it native and with qemu-i386 and compare the output of both runs
> to see the problems.

So... is QEMU or the test mistaken? :)

Regards,
Andreas
Stefan Weil March 7, 2014, 1:19 p.m. UTC | #4
Am 07.03.2014 13:56, schrieb Andreas Färber:
> Am 07.03.2014 13:40, schrieb Stefan Weil:
>> Am 07.03.2014 13:06, schrieb Alex Bennée:
>>>
>>> Stefan Weil <sw@weilnetz.de> writes:
>>>
>>>> 'make test' is broken at least since commit
>>>> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
>>>> to util/, and some of them there split, so add the missing prefix and new
>>>> files to fix the compiler and linker errors.
> [...]
>> In the meantime I tried older versions. Even version 1.1 already shows
>> unwanted differences between native and QEMU emulated FPU operations.
>>
>> The test binary is now available from this URL:
>> http://qemu.weilnetz.de/test/user/i386/test-i386
>>
>> Run it native and with qemu-i386 and compare the output of both runs
>> to see the problems.
> 
> So... is QEMU or the test mistaken? :)
> 
> Regards,
> Andreas


Hi Andreas,

test-i386 does some calculations and prints the results (see source code
tests/tcg/test-i386.c). If the user mode emulation of QEMU works, it
should not matter whether that executable runs native or emulated and
both outputs be identical. They aren't - that's why I think we have a
TCG problem to solve.

Regards
Stefan
Peter Maydell March 7, 2014, 1:35 p.m. UTC | #5
On 7 March 2014 13:19, Stefan Weil <sw@weilnetz.de> wrote:
> test-i386 does some calculations and prints the results (see source code
> tests/tcg/test-i386.c). If the user mode emulation of QEMU works, it
> should not matter whether that executable runs native or emulated and
> both outputs be identical. They aren't - that's why I think we have a
> TCG problem to solve.

I think TCG x86 FPU emulation has been a bit dodgy since
forever; it's a fair amount of work to go through and
fix everything up to be bitwise exact results versus
hardware and I think that nobody's cared enough about x86
emulation to do that...

thanks
-- PMM
Alex Bennée March 7, 2014, 4:17 p.m. UTC | #6
Peter Maydell <peter.maydell@linaro.org> writes:

> On 7 March 2014 13:19, Stefan Weil <sw@weilnetz.de> wrote:
>> test-i386 does some calculations and prints the results (see source code
>> tests/tcg/test-i386.c). If the user mode emulation of QEMU works, it
>> should not matter whether that executable runs native or emulated and
>> both outputs be identical. They aren't - that's why I think we have a
>> TCG problem to solve.
>
> I think TCG x86 FPU emulation has been a bit dodgy since
> forever; it's a fair amount of work to go through and
> fix everything up to be bitwise exact results versus
> hardware and I think that nobody's cared enough about x86
> emulation to do that...

IIRC the behaviour is different depending on how much x87 vs SIMD FP you
go through. FWIW the Transitive translator was able to do most FP ops
with generated code (certainly on SPARC->x86_64) and only go to softfloat
for some things. But you did need to disable the x87 to do it.

But I get the impression that FP performance is currently "good enough"
for what QEMU gets used for.
Peter Maydell March 7, 2014, 4:37 p.m. UTC | #7
On 7 March 2014 16:17, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> I think TCG x86 FPU emulation has been a bit dodgy since
>> forever; it's a fair amount of work to go through and
>> fix everything up to be bitwise exact results versus
>> hardware and I think that nobody's cared enough about x86
>> emulation to do that...
>
> IIRC the behaviour is different depending on how much x87 vs SIMD FP you
> go through. FWIW the Transitive translator was able to do most FP ops
> with generated code (certainly on SPARC->x86_64) and only go to softfloat
> for some things. But you did need to disable the x87 to do it.
>
> But I get the impression that FP performance is currently "good enough"
> for what QEMU gets used for.

This is true but not particularly relevant to whether our
current implementation is actually buggy in the sense
of producing wrong results. We ripped out the code that
tried to implement FP natively a long time back, so
pure emulation is all we do now.

thanks
-- PMM
Michael Tokarev March 14, 2014, 4:28 p.m. UTC | #8
07.03.2014 14:11, Stefan Weil wrote:
> 'make test' is broken at least since commit
> baacf04799ace72a9c735dd9306a1ceaf305e7cf. Several source files were moved
> to util/, and some of them there split, so add the missing prefix and new
> files to fix the compiler and linker errors.
> 
> There remain more issues, but these changes allow running the test on a
> Linux i686 host.

Thanks, applied to -trivial.

/mjt
diff mbox

Patch

diff --git a/tests/tcg/test_path.c b/tests/tcg/test_path.c
index a064eea..f8dd36a 100644
--- a/tests/tcg/test_path.c
+++ b/tests/tcg/test_path.c
@@ -1,12 +1,15 @@ 
 /* Test path override code */
 #define _GNU_SOURCE
 #include "config-host.h"
-#include "iov.c"
-#include "cutils.c"
-#include "path.c"
-#include "trace.c"
+#include "util/cutils.c"
+#include "util/hexdump.c"
+#include "util/iov.c"
+#include "util/path.c"
+#include "util/qemu-timer-common.c"
+#include "trace/control.c"
+#include "../trace/generated-events.c"
 #ifdef CONFIG_TRACE_SIMPLE
-#include "../trace/simple.c"
+#include "trace/simple.c"
 #endif
 
 #include <stdarg.h>