hexagon docker test failure

hexagon docker test failure

[Richard Henderson]

Hi Taylor,

One of your recent hexagon testsuite changes is incompatible with the docker image that 
we're using:

tests/tcg/hexagon/multi_result.c:79:16: error: invalid instruction

   asm volatile("%0,p0 = vminub(%2, %3)\n\t"

                ^

<inline asm>:1:2: note: instantiated into assembly here

         r3:2,p0 = vminub(r1:0, r3:2)

         ^

1 error generated.


Can we try again to update debian-hexagon-cross?  I recall that last time there was a 
compiler bug that prevented forward progress.  Perhaps that has been fixed in the interim?

I'm willing to accept such an update in the next week before rc1, but if we can't manage 
that we'll need to disable the failing test(s?).  Thanks in advance,


r~

RE: hexagon docker test failure

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Tuesday, July 26, 2022 10:41 AM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: qemu-devel <qemu-devel@nongnu.org>
> Subject: hexagon docker test failure
> 
> Hi Taylor,
> 
> One of your recent hexagon testsuite changes is incompatible with the
> docker image that we're using:
> 
> tests/tcg/hexagon/multi_result.c:79:16: error: invalid instruction
> 
>    asm volatile("%0,p0 = vminub(%2, %3)\n\t"
> 
>                 ^
> 
> <inline asm>:1:2: note: instantiated into assembly here
> 
>          r3:2,p0 = vminub(r1:0, r3:2)
> 
>          ^
> 
> 1 error generated.
> 
> 
> Can we try again to update debian-hexagon-cross?  I recall that last time
> there was a compiler bug that prevented forward progress.  Perhaps that has
> been fixed in the interim?
> 
> I'm willing to accept such an update in the next week before rc1, but if we
> can't manage that we'll need to disable the failing test(s?).  Thanks in
> advance,
> 
> 
> r~

Hi Richard,

Some of the tests require the -mv67 flag to be passed to the compiler because they have instructions that are new in v67.

This patch
commit cd362defbbd09cbbc08b3bb465141542887b8cef
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Fri May 27 16:35:48 2022 +0100

    tests/tcg: merge configure.sh back into main configure script

Moved this line from tests/tcg/configure.sh to the main configure script
: ${cross_cc_cflags_hexagon="-mv67 -O2 -static"}


However, those flags aren't actually passed to the compiler any more - at least by default.

The gitlab builder is passing
https://gitlab.com/qemu-project/qemu/-/jobs/2771528066
So, there must be something in $MAKE_CHECK_ARGS

I use the following when I run
make EXTRA_CFLAGS='-mv67 -O2' check-tcg


So, we probably don't need a new docker image.  Do other targets have their cross_cc_cflags?  Please advise.

Thanks,
Taylor

Re: hexagon docker test failure

[Richard Henderson]

On 7/26/22 10:23, Taylor Simpson wrote:
> 
>> -----Original Message-----
>> From: Richard Henderson <richard.henderson@linaro.org>
>> Sent: Tuesday, July 26, 2022 10:41 AM
>> To: Taylor Simpson <tsimpson@quicinc.com>
>> Cc: qemu-devel <qemu-devel@nongnu.org>
>> Subject: hexagon docker test failure
>>
>> Hi Taylor,
>>
>> One of your recent hexagon testsuite changes is incompatible with the
>> docker image that we're using:
>>
>> tests/tcg/hexagon/multi_result.c:79:16: error: invalid instruction
>>
>>     asm volatile("%0,p0 = vminub(%2, %3)\n\t"
>>
>>                  ^
>>
>> <inline asm>:1:2: note: instantiated into assembly here
>>
>>           r3:2,p0 = vminub(r1:0, r3:2)
>>
>>           ^
>>
>> 1 error generated.
>>
>>
>> Can we try again to update debian-hexagon-cross?  I recall that last time
>> there was a compiler bug that prevented forward progress.  Perhaps that has
>> been fixed in the interim?
>>
>> I'm willing to accept such an update in the next week before rc1, but if we
>> can't manage that we'll need to disable the failing test(s?).  Thanks in
>> advance,
>>
>>
>> r~
> 
> Hi Richard,
> 
> Some of the tests require the -mv67 flag to be passed to the compiler because they have instructions that are new in v67.
> 
> This patch
> commit cd362defbbd09cbbc08b3bb465141542887b8cef
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date:   Fri May 27 16:35:48 2022 +0100
> 
>      tests/tcg: merge configure.sh back into main configure script
> 
> Moved this line from tests/tcg/configure.sh to the main configure script
> : ${cross_cc_cflags_hexagon="-mv67 -O2 -static"}
> 
> 
> However, those flags aren't actually passed to the compiler any more - at least by default.
> 
> The gitlab builder is passing
> https://gitlab.com/qemu-project/qemu/-/jobs/2771528066
> So, there must be something in $MAKE_CHECK_ARGS
> 
> I use the following when I run
> make EXTRA_CFLAGS='-mv67 -O2' check-tcg
> 
> 
> So, we probably don't need a new docker image.  Do other targets have their cross_cc_cflags?  Please advise.

Oooh, that's easy.  Just modify CFLAGS in tests/tcg/hexagon/Makefile.target.

I've just tested

--- a/tests/tcg/hexagon/Makefile.target

+++ b/tests/tcg/hexagon/Makefile.target

@@ -19,7 +19,7 @@

  EXTRA_RUNS =



  CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal

-CFLAGS += -fno-unroll-loops

+CFLAGS += -fno-unroll-loops -mv67 -O2



  HEX_SRC=$(SRC_PATH)/tests/tcg/hexagon

  VPATH += $(HEX_SRC)


and it now builds, but I see a runtime error:

timeout --foreground 90  /home/rth/qemu/bld/qemu-hexagon  misc >  misc.out

make[1]: *** [../Makefile.target:158: run-misc] Error 1

$ cat ./tests/tcg/hexagon-linux-user/misc.out

ERROR: 0x0007 != 0x001f

FAIL


Any ideas there?


r~

RE: hexagon docker test failure

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Tuesday, July 26, 2022 11:42 AM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: qemu-devel <qemu-devel@nongnu.org>
> Subject: Re: hexagon docker test failure
> 
> On 7/26/22 10:23, Taylor Simpson wrote:
> >
> >> -----Original Message-----
> >> From: Richard Henderson <richard.henderson@linaro.org>
> >> Sent: Tuesday, July 26, 2022 10:41 AM
> >> To: Taylor Simpson <tsimpson@quicinc.com>
> >> Cc: qemu-devel <qemu-devel@nongnu.org>
> >> Subject: hexagon docker test failure
> >>
> >> Hi Taylor,
> >>
> >> One of your recent hexagon testsuite changes is incompatible with the
> >> docker image that we're using:
> >>
> >> tests/tcg/hexagon/multi_result.c:79:16: error: invalid instruction
> >>
> >>     asm volatile("%0,p0 = vminub(%2, %3)\n\t"
> >>
> >>                  ^
> >>
> >> <inline asm>:1:2: note: instantiated into assembly here
> >>
> >>           r3:2,p0 = vminub(r1:0, r3:2)
> >>
> >>           ^
> >>
> >> 1 error generated.
> >>
> >>
> >> Can we try again to update debian-hexagon-cross?  I recall that last
> >> time there was a compiler bug that prevented forward progress.
> >> Perhaps that has been fixed in the interim?
> >>
> >> I'm willing to accept such an update in the next week before rc1, but
> >> if we can't manage that we'll need to disable the failing test(s?).
> >> Thanks in advance,
> >>
> >>
> >> r~
> >
> > Hi Richard,
> >
> > Some of the tests require the -mv67 flag to be passed to the compiler
> because they have instructions that are new in v67.
> >
> > This patch
> > commit cd362defbbd09cbbc08b3bb465141542887b8cef
> > Author: Paolo Bonzini <pbonzini@redhat.com>
> > Date:   Fri May 27 16:35:48 2022 +0100
> >
> >      tests/tcg: merge configure.sh back into main configure script
> >
> > Moved this line from tests/tcg/configure.sh to the main configure
> > script
> > : ${cross_cc_cflags_hexagon="-mv67 -O2 -static"}
> >
> >
> > However, those flags aren't actually passed to the compiler any more - at
> least by default.
> >
> > The gitlab builder is passing
> > https://gitlab.com/qemu-project/qemu/-/jobs/2771528066
> > So, there must be something in $MAKE_CHECK_ARGS
> >
> > I use the following when I run
> > make EXTRA_CFLAGS='-mv67 -O2' check-tcg
> >
> >
> > So, we probably don't need a new docker image.  Do other targets have
> their cross_cc_cflags?  Please advise.
> 
> Oooh, that's easy.  Just modify CFLAGS in
> tests/tcg/hexagon/Makefile.target.
> 
> I've just tested
> 
> --- a/tests/tcg/hexagon/Makefile.target
> 
> +++ b/tests/tcg/hexagon/Makefile.target
> 
> @@ -19,7 +19,7 @@
> 
>   EXTRA_RUNS =
> 
> 
> 
>   CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
> 
> -CFLAGS += -fno-unroll-loops
> 
> +CFLAGS += -fno-unroll-loops -mv67 -O2
> 
> 
> 
>   HEX_SRC=$(SRC_PATH)/tests/tcg/hexagon
> 
>   VPATH += $(HEX_SRC)
> 
> 
> and it now builds, but I see a runtime error:
> 
> timeout --foreground 90  /home/rth/qemu/bld/qemu-hexagon  misc >
> misc.out
> 
> make[1]: *** [../Makefile.target:158: run-misc] Error 1
> 
> $ cat ./tests/tcg/hexagon-linux-user/misc.out
> 
> ERROR: 0x0007 != 0x001f
> 
> FAIL
> 
> 
> Any ideas there?

That's because misc requires -O2 (don't remember why).  When you put -O2 in CFLAGS, it gets overridden.  Here's the command that gets used - notice the -O0 after the -O2.
/local/mnt/workspace/qemu-series/qemu/tests/docker/docker.py --engine auto cc --cc hexagon-unknown-linux-musl-clang -i qemu/debian-hexagon-cross -s /local/mnt/workspace/qemu-series/qemu --  -Wno-incompatible-pointer-types -Wno-undefined-internal -fno-unroll-loops -mv68 -O2 -Wall -Werror -O0 -g -fno-strict-aliasing  /local/mnt/workspace/qemu-series/qemu/tests/tcg/hexagon/misc.c -o misc  -static


So, instead of putting those in CFLAGS, put them in EXTRA_CFLAGS.

--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -20,6 +20,7 @@ EXTRA_RUNS =
 
 CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
 CFLAGS += -fno-unroll-loops
+EXTRA_CFLAGS += -mv67 -O2
 
 HEX_SRC=$(SRC_PATH)/tests/tcg/hexagon
 VPATH += $(HEX_SRC)


Do I understand correctly that putting the flags in Makefile.target is the proper way and cross_cc_cflags is obsolete?  If so, I'll send the above as a patch.

Thanks,
Taylor

RE: hexagon docker test failure

[Brian Cain]

> -----Original Message-----
> From: Qemu-devel <qemu-devel-bounces+bcain=quicinc.com@nongnu.org>
> On Behalf Of Richard Henderson
> Sent: Tuesday, July 26, 2022 12:42 PM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: qemu-devel <qemu-devel@nongnu.org>
> Subject: Re: hexagon docker test failure
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
> 
> On 7/26/22 10:23, Taylor Simpson wrote:
> >
> >> -----Original Message-----
> >> From: Richard Henderson <richard.henderson@linaro.org>
> >> Sent: Tuesday, July 26, 2022 10:41 AM
> >> To: Taylor Simpson <tsimpson@quicinc.com>
> >> Cc: qemu-devel <qemu-devel@nongnu.org>
> >> Subject: hexagon docker test failure
> >>
> >> Hi Taylor,
> >>
> >> One of your recent hexagon testsuite changes is incompatible with the
> >> docker image that we're using:
> >>
> >> tests/tcg/hexagon/multi_result.c:79:16: error: invalid instruction
> >>
> >>     asm volatile("%0,p0 = vminub(%2, %3)\n\t"
> >>
> >>                  ^
> >>
> >> <inline asm>:1:2: note: instantiated into assembly here
> >>
> >>           r3:2,p0 = vminub(r1:0, r3:2)
> >>
> >>           ^
> >>
> >> 1 error generated.
> >>
> >>
> >> Can we try again to update debian-hexagon-cross?  I recall that last time
> >> there was a compiler bug that prevented forward progress.  Perhaps that
> has
> >> been fixed in the interim?
> >>
> >> I'm willing to accept such an update in the next week before rc1, but if we
> >> can't manage that we'll need to disable the failing test(s?).  Thanks in
> >> advance,
> >>
> >>
> >> r~
> >
> > Hi Richard,
> >
> > Some of the tests require the -mv67 flag to be passed to the compiler
> because they have instructions that are new in v67.
> >
> > This patch
> > commit cd362defbbd09cbbc08b3bb465141542887b8cef
> > Author: Paolo Bonzini <pbonzini@redhat.com>
> > Date:   Fri May 27 16:35:48 2022 +0100
> >
> >      tests/tcg: merge configure.sh back into main configure script
> >
> > Moved this line from tests/tcg/configure.sh to the main configure script
> > : ${cross_cc_cflags_hexagon="-mv67 -O2 -static"}
> >
> >
> > However, those flags aren't actually passed to the compiler any more - at
> least by default.
> >
> > The gitlab builder is passing
> > https://gitlab.com/qemu-project/qemu/-/jobs/2771528066
> > So, there must be something in $MAKE_CHECK_ARGS
> >
> > I use the following when I run
> > make EXTRA_CFLAGS='-mv67 -O2' check-tcg
> >
> >
> > So, we probably don't need a new docker image.  Do other targets have their
> cross_cc_cflags?  Please advise.
> 
> Oooh, that's easy.  Just modify CFLAGS in tests/tcg/hexagon/Makefile.target.
> 
> I've just tested
> 
> --- a/tests/tcg/hexagon/Makefile.target
> 
> +++ b/tests/tcg/hexagon/Makefile.target
> 
> @@ -19,7 +19,7 @@
> 
>   EXTRA_RUNS =
> 
> 
> 
>   CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
> 
> -CFLAGS += -fno-unroll-loops
> 
> +CFLAGS += -fno-unroll-loops -mv67 -O2
> 
> 
> 
>   HEX_SRC=$(SRC_PATH)/tests/tcg/hexagon
> 
>   VPATH += $(HEX_SRC)
> 
> 
> and it now builds, but I see a runtime error:
> 
> timeout --foreground 90  /home/rth/qemu/bld/qemu-hexagon  misc >
> misc.out
> 
> make[1]: *** [../Makefile.target:158: run-misc] Error 1
> 
> $ cat ./tests/tcg/hexagon-linux-user/misc.out
> 
> ERROR: 0x0007 != 0x001f
> 
> FAIL
> 
> 
> Any ideas there?

I don't think I've seen this one fail before.  We can analyze the failure and/or bisect to see if it was introduced by a particular commit.

-Brian

Re: hexagon docker test failure

[Richard Henderson]

On 7/26/22 11:00, Taylor Simpson wrote:
> So, instead of putting those in CFLAGS, put them in EXTRA_CFLAGS.
> 
> --- a/tests/tcg/hexagon/Makefile.target
> +++ b/tests/tcg/hexagon/Makefile.target
> @@ -20,6 +20,7 @@ EXTRA_RUNS =
>   
>   CFLAGS += -Wno-incompatible-pointer-types -Wno-undefined-internal
>   CFLAGS += -fno-unroll-loops
> +EXTRA_CFLAGS += -mv67 -O2
>   
>   HEX_SRC=$(SRC_PATH)/tests/tcg/hexagon
>   VPATH += $(HEX_SRC)

Ah, ok.

> Do I understand correctly that putting the flags in Makefile.target is the proper way and cross_cc_cflags is obsolete?

cross_cc_flags is intended to handle using one compiler for multiple targets, e.g. arm vs 
armbe.

Which is not what you're attempting to do; you're trying to test a particular isa. 
Compare tests/tcg/aarch64/Makefile.target:

bti-1 bti-3: CFLAGS += -mbranch-protection=standard

pauth-%: CFLAGS += -march=armv8.3-a

mte-%: CFLAGS += -march=armv8.5-a+memtag


where we set specific isa extensions for specific tests.


r~

Re: hexagon docker test failure

[Philippe Mathieu-Daudé via]

(Cc'ing Paolo for commit cd362defb)

On 26/7/22 19:23, Taylor Simpson wrote:
> 
>> -----Original Message-----
>> From: Richard Henderson <richard.henderson@linaro.org>
>> Sent: Tuesday, July 26, 2022 10:41 AM
>> To: Taylor Simpson <tsimpson@quicinc.com>
>> Cc: qemu-devel <qemu-devel@nongnu.org>
>> Subject: hexagon docker test failure
>>
>> Hi Taylor,
>>
>> One of your recent hexagon testsuite changes is incompatible with the
>> docker image that we're using:
>>
>> tests/tcg/hexagon/multi_result.c:79:16: error: invalid instruction
>>
>>     asm volatile("%0,p0 = vminub(%2, %3)\n\t"
>>
>>                  ^
>>
>> <inline asm>:1:2: note: instantiated into assembly here
>>
>>           r3:2,p0 = vminub(r1:0, r3:2)
>>
>>           ^
>>
>> 1 error generated.
>>
>>
>> Can we try again to update debian-hexagon-cross?  I recall that last time
>> there was a compiler bug that prevented forward progress.  Perhaps that has
>> been fixed in the interim?
>>
>> I'm willing to accept such an update in the next week before rc1, but if we
>> can't manage that we'll need to disable the failing test(s?).  Thanks in
>> advance,
>>
>>
>> r~
> 
> Hi Richard,
> 
> Some of the tests require the -mv67 flag to be passed to the compiler because they have instructions that are new in v67.
> 
> This patch
> commit cd362defbbd09cbbc08b3bb465141542887b8cef
> Author: Paolo Bonzini <pbonzini@redhat.com>
> Date:   Fri May 27 16:35:48 2022 +0100
> 
>      tests/tcg: merge configure.sh back into main configure script
> 
> Moved this line from tests/tcg/configure.sh to the main configure script
> : ${cross_cc_cflags_hexagon="-mv67 -O2 -static"}
> 
> 
> However, those flags aren't actually passed to the compiler any more - at least by default.
> 
> The gitlab builder is passing
> https://gitlab.com/qemu-project/qemu/-/jobs/2771528066
> So, there must be something in $MAKE_CHECK_ARGS
> 
> I use the following when I run
> make EXTRA_CFLAGS='-mv67 -O2' check-tcg
> 
> 
> So, we probably don't need a new docker image.  Do other targets have their cross_cc_cflags?  Please advise.
> 
> Thanks,
> Taylor
>