Re: [PATCH 3/3] tests/tcg/s390x: Test VSTRS
On Thu, Aug 17, 2023 at 11:37:29AM +0200, Claudio Fontana wrote: > On 8/5/23 01:03, Ilya Leoshkevich wrote: > > Add a small test to prevent regressions. > > > > Signed-off-by: Ilya Leoshkevich > > Something seems off in the wiring of the make check target? > > I built with: > > ./configure --target-list=s390x-linux-user,s390x-softmmu > > make -j > make -j check-help > > ... > > Individual test suites: > make check-qtest-TARGET Run qtest tests for given target > make check-qtestRun qtest tests > make check-unit Run qobject tests > make check-qapi-schema Run QAPI schema tests > make check-blockRun block tests > make check-tcg Run TCG tests > > > ... > > make -j check-tcg > > changing dir to build for make "check-tcg"... > make[1]: Entering directory '/root/git/qemu/build' > make[1]: Nothing to be done for 'check-tcg'. > make[1]: Leaving directory '/root/git/qemu/build' > > > Why is this not running any tests for tcg? > > I tried also to run the general make check, > but even in this case the tcg tests do not seem to trigger. > > Thanks, > > Claudio Hi, I believe you need either s390x-linux-gnu-gcc or docker/podman to run the tcg tests. Best regards, Ilya
Re: [PATCH 3/3] tests/tcg/s390x: Test VSTRS
On 8/5/23 01:03, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
>
> Signed-off-by: Ilya Leoshkevich
Something seems off in the wiring of the make check target?
I built with:
./configure --target-list=s390x-linux-user,s390x-softmmu
make -j
make -j check-help
...
Individual test suites:
make check-qtest-TARGET Run qtest tests for given target
make check-qtestRun qtest tests
make check-unit Run qobject tests
make check-qapi-schema Run QAPI schema tests
make check-blockRun block tests
make check-tcg Run TCG tests
...
make -j check-tcg
changing dir to build for make "check-tcg"...
make[1]: Entering directory '/root/git/qemu/build'
make[1]: Nothing to be done for 'check-tcg'.
make[1]: Leaving directory '/root/git/qemu/build'
Why is this not running any tests for tcg?
I tried also to run the general make check,
but even in this case the tcg tests do not seem to trigger.
Thanks,
Claudio
> ---
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/vxeh2_vstrs.c | 88 +
> 2 files changed, 89 insertions(+)
> create mode 100644 tests/tcg/s390x/vxeh2_vstrs.c
>
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 1fc98099070..8ba36e5985b 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -73,6 +73,7 @@ ifneq ($(CROSS_CC_HAS_Z15),)
> Z15_TESTS=vxeh2_vs
> Z15_TESTS+=vxeh2_vcvt
> Z15_TESTS+=vxeh2_vlstr
> +Z15_TESTS+=vxeh2_vstrs
> $(Z15_TESTS): CFLAGS+=-march=z15 -O2
> TESTS+=$(Z15_TESTS)
> endif
> diff --git a/tests/tcg/s390x/vxeh2_vstrs.c b/tests/tcg/s390x/vxeh2_vstrs.c
> new file mode 100644
> index 000..313ec1d728f
> --- /dev/null
> +++ b/tests/tcg/s390x/vxeh2_vstrs.c
> @@ -0,0 +1,88 @@
> +/*
> + * Test the VSTRS instruction.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include
> +#include
> +#include
> +#include
> +#include "vx.h"
> +
> +static inline __attribute__((__always_inline__)) int
> +vstrs(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
> + const S390Vector *v4, const uint8_t m5, const uint8_t m6)
> +{
> +int cc;
> +
> +asm("vstrs %[v1],%[v2],%[v3],%[v4],%[m5],%[m6]\n"
> +"ipm %[cc]"
> +: [v1] "=v" (v1->v)
> +, [cc] "=r" (cc)
> +: [v2] "v" (v2->v)
> +, [v3] "v" (v3->v)
> +, [v4] "v" (v4->v)
> +, [m5] "i" (m5)
> +, [m6] "i" (m6)
> +: "cc");
> +
> +return (cc >> 28) & 3;
> +}
> +
> +static void test_ignored_match(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0x222000205e41ULL, .d[1] = 0};
> +S390Vector v3 = {.d[0] = 0x205e4100ULL, .d[1] = 0};
> +S390Vector v4 = {.d[0] = 3, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 2) == 1);
> +assert(v1.d[0] == 16);
> +assert(v1.d[1] == 0);
> +}
> +
> +static void test_empty_needle(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0x5300ULL, .d[1] = 0};
> +S390Vector v3 = {.d[0] = 0, .d[1] = 0};
> +S390Vector v4 = {.d[0] = 0, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 0) == 2);
> +assert(v1.d[0] == 0);
> +assert(v1.d[1] == 0);
> +}
> +
> +static void test_max_length(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0x1122334455667700ULL, .d[1] = 0};
> +S390Vector v3 = {.d[0] = 0, .d[1] = 0};
> +S390Vector v4 = {.d[0] = 16, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 0) == 3);
> +assert(v1.d[0] == 7);
> +assert(v1.d[1] == 0);
> +}
> +
> +static void test_no_match(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0xff000f00ULL, .d[1] = 0x82b};
> +S390Vector v3 = {.d[0] = 0xfffeULL,
> + .d[1] = 0xULL};
> +S390Vector v4 = {.d[0] = 11, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 2) == 1);
> +assert(v1.d[0] == 16);
> +assert(v1.d[1] == 0);
> +}
> +
> +int main(void)
> +{
> +test_ignored_match();
> +test_empty_needle();
> +test_max_length();
> +test_no_match();
> +return EXIT_SUCCESS;
> +}
Re: [PATCH 3/3] tests/tcg/s390x: Test VSTRS
On 8/7/23 10:08, Ilya Leoshkevich wrote:
> On Sun, 2023-08-06 at 13:05 +0200, Claudio Fontana wrote:
>> On 8/5/23 01:03, Ilya Leoshkevich wrote:
>>> Add a small test to prevent regressions.
>>>
>>> Signed-off-by: Ilya Leoshkevich
>>> ---
>>> tests/tcg/s390x/Makefile.target | 1 +
>>> tests/tcg/s390x/vxeh2_vstrs.c | 88
>>> +
>>> 2 files changed, 89 insertions(+)
>>> create mode 100644 tests/tcg/s390x/vxeh2_vstrs.c
>>>
>>> diff --git a/tests/tcg/s390x/Makefile.target
>>> b/tests/tcg/s390x/Makefile.target
>>> index 1fc98099070..8ba36e5985b 100644
>>> --- a/tests/tcg/s390x/Makefile.target
>>> +++ b/tests/tcg/s390x/Makefile.target
>>> @@ -73,6 +73,7 @@ ifneq ($(CROSS_CC_HAS_Z15),)
>>> Z15_TESTS=vxeh2_vs
>>> Z15_TESTS+=vxeh2_vcvt
>>> Z15_TESTS+=vxeh2_vlstr
>>> +Z15_TESTS+=vxeh2_vstrs
>>> $(Z15_TESTS): CFLAGS+=-march=z15 -O2
>>> TESTS+=$(Z15_TESTS)
>>> endif
>>> diff --git a/tests/tcg/s390x/vxeh2_vstrs.c
>>> b/tests/tcg/s390x/vxeh2_vstrs.c
>>> new file mode 100644
>>> index 000..313ec1d728f
>>> --- /dev/null
>>> +++ b/tests/tcg/s390x/vxeh2_vstrs.c
>>> @@ -0,0 +1,88 @@
>>> +/*
>>> + * Test the VSTRS instruction.
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include "vx.h"
>>> +
>>> +static inline __attribute__((__always_inline__)) int
>>> +vstrs(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
>>> + const S390Vector *v4, const uint8_t m5, const uint8_t m6)
>>> +{
>>> + int cc;
>>> +
>>> + asm("vstrs %[v1],%[v2],%[v3],%[v4],%[m5],%[m6]\n"
>>> + "ipm %[cc]"
>>> + : [v1] "=v" (v1->v)
>>> + , [cc] "=r" (cc)
>>> + : [v2] "v" (v2->v)
>>> + , [v3] "v" (v3->v)
>>> + , [v4] "v" (v4->v)
>>> + , [m5] "i" (m5)
>>> + , [m6] "i" (m6)
>>> + : "cc");
>>> +
>>> + return (cc >> 28) & 3;
>> Following the POp, I am puzzled by the use of an "int" to contain the
>> register result of the IPM instruction, should it not be a 64bit
>> variable?
>> bits 0-31 are left unchanged / are uninteresting, is that enough to
>> avoid having to use a properly sized variable?
>
> The compiler understands that if the type is int, then the asm block
> will not touch the upper 32 bits of the register that's assigned to it.
> This observation is used not only in the QEMU tests, but also all over
> arch/s390 in the Linux kernel.
Thank you,
Claudio
>>
>> I see that this is done elsewhere in the tests (sometimes a 64bit
>> variable is used, sometimes just 'int'), so I assume it's probably
>> fine.
>>
>> Otherwise lgtm,
>>
>> Claudio
>
> [...]
>>
>
Re: [PATCH 3/3] tests/tcg/s390x: Test VSTRS
On Sun, 2023-08-06 at 13:05 +0200, Claudio Fontana wrote:
> On 8/5/23 01:03, Ilya Leoshkevich wrote:
> > Add a small test to prevent regressions.
> >
> > Signed-off-by: Ilya Leoshkevich
> > ---
> > tests/tcg/s390x/Makefile.target | 1 +
> > tests/tcg/s390x/vxeh2_vstrs.c | 88
> > +
> > 2 files changed, 89 insertions(+)
> > create mode 100644 tests/tcg/s390x/vxeh2_vstrs.c
> >
> > diff --git a/tests/tcg/s390x/Makefile.target
> > b/tests/tcg/s390x/Makefile.target
> > index 1fc98099070..8ba36e5985b 100644
> > --- a/tests/tcg/s390x/Makefile.target
> > +++ b/tests/tcg/s390x/Makefile.target
> > @@ -73,6 +73,7 @@ ifneq ($(CROSS_CC_HAS_Z15),)
> > Z15_TESTS=vxeh2_vs
> > Z15_TESTS+=vxeh2_vcvt
> > Z15_TESTS+=vxeh2_vlstr
> > +Z15_TESTS+=vxeh2_vstrs
> > $(Z15_TESTS): CFLAGS+=-march=z15 -O2
> > TESTS+=$(Z15_TESTS)
> > endif
> > diff --git a/tests/tcg/s390x/vxeh2_vstrs.c
> > b/tests/tcg/s390x/vxeh2_vstrs.c
> > new file mode 100644
> > index 000..313ec1d728f
> > --- /dev/null
> > +++ b/tests/tcg/s390x/vxeh2_vstrs.c
> > @@ -0,0 +1,88 @@
> > +/*
> > + * Test the VSTRS instruction.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "vx.h"
> > +
> > +static inline __attribute__((__always_inline__)) int
> > +vstrs(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
> > + const S390Vector *v4, const uint8_t m5, const uint8_t m6)
> > +{
> > + int cc;
> > +
> > + asm("vstrs %[v1],%[v2],%[v3],%[v4],%[m5],%[m6]\n"
> > + "ipm %[cc]"
> > + : [v1] "=v" (v1->v)
> > + , [cc] "=r" (cc)
> > + : [v2] "v" (v2->v)
> > + , [v3] "v" (v3->v)
> > + , [v4] "v" (v4->v)
> > + , [m5] "i" (m5)
> > + , [m6] "i" (m6)
> > + : "cc");
> > +
> > + return (cc >> 28) & 3;
> Following the POp, I am puzzled by the use of an "int" to contain the
> register result of the IPM instruction, should it not be a 64bit
> variable?
> bits 0-31 are left unchanged / are uninteresting, is that enough to
> avoid having to use a properly sized variable?
The compiler understands that if the type is int, then the asm block
will not touch the upper 32 bits of the register that's assigned to it.
This observation is used not only in the QEMU tests, but also all over
arch/s390 in the Linux kernel.
>
> I see that this is done elsewhere in the tests (sometimes a 64bit
> variable is used, sometimes just 'int'), so I assume it's probably
> fine.
>
> Otherwise lgtm,
>
> Claudio
[...]
>
Re: [PATCH 3/3] tests/tcg/s390x: Test VSTRS
On 8/5/23 01:03, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
>
> Signed-off-by: Ilya Leoshkevich
> ---
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/vxeh2_vstrs.c | 88 +
> 2 files changed, 89 insertions(+)
> create mode 100644 tests/tcg/s390x/vxeh2_vstrs.c
>
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 1fc98099070..8ba36e5985b 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -73,6 +73,7 @@ ifneq ($(CROSS_CC_HAS_Z15),)
> Z15_TESTS=vxeh2_vs
> Z15_TESTS+=vxeh2_vcvt
> Z15_TESTS+=vxeh2_vlstr
> +Z15_TESTS+=vxeh2_vstrs
> $(Z15_TESTS): CFLAGS+=-march=z15 -O2
> TESTS+=$(Z15_TESTS)
> endif
> diff --git a/tests/tcg/s390x/vxeh2_vstrs.c b/tests/tcg/s390x/vxeh2_vstrs.c
> new file mode 100644
> index 000..313ec1d728f
> --- /dev/null
> +++ b/tests/tcg/s390x/vxeh2_vstrs.c
> @@ -0,0 +1,88 @@
> +/*
> + * Test the VSTRS instruction.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include
> +#include
> +#include
> +#include
> +#include "vx.h"
> +
> +static inline __attribute__((__always_inline__)) int
> +vstrs(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
> + const S390Vector *v4, const uint8_t m5, const uint8_t m6)
> +{
> +int cc;
> +
> +asm("vstrs %[v1],%[v2],%[v3],%[v4],%[m5],%[m6]\n"
> +"ipm %[cc]"
> +: [v1] "=v" (v1->v)
> +, [cc] "=r" (cc)
> +: [v2] "v" (v2->v)
> +, [v3] "v" (v3->v)
> +, [v4] "v" (v4->v)
> +, [m5] "i" (m5)
> +, [m6] "i" (m6)
> +: "cc");
> +
> +return (cc >> 28) & 3;
Following the POp, I am puzzled by the use of an "int" to contain the register
result of the IPM instruction, should it not be a 64bit variable?
bits 0-31 are left unchanged / are uninteresting, is that enough to avoid
having to use a properly sized variable?
I see that this is done elsewhere in the tests (sometimes a 64bit variable is
used, sometimes just 'int'), so I assume it's probably fine.
Otherwise lgtm,
Claudio
> +}
> +
> +static void test_ignored_match(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0x222000205e41ULL, .d[1] = 0};
> +S390Vector v3 = {.d[0] = 0x205e4100ULL, .d[1] = 0};
> +S390Vector v4 = {.d[0] = 3, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 2) == 1);
> +assert(v1.d[0] == 16);
> +assert(v1.d[1] == 0);
> +}
> +
> +static void test_empty_needle(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0x5300ULL, .d[1] = 0};
> +S390Vector v3 = {.d[0] = 0, .d[1] = 0};
> +S390Vector v4 = {.d[0] = 0, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 0) == 2);
> +assert(v1.d[0] == 0);
> +assert(v1.d[1] == 0);
> +}
> +
> +static void test_max_length(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0x1122334455667700ULL, .d[1] = 0};
> +S390Vector v3 = {.d[0] = 0, .d[1] = 0};
> +S390Vector v4 = {.d[0] = 16, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 0) == 3);
> +assert(v1.d[0] == 7);
> +assert(v1.d[1] == 0);
> +}
> +
> +static void test_no_match(void)
> +{
> +S390Vector v1;
> +S390Vector v2 = {.d[0] = 0xff000f00ULL, .d[1] = 0x82b};
> +S390Vector v3 = {.d[0] = 0xfffeULL,
> + .d[1] = 0xULL};
> +S390Vector v4 = {.d[0] = 11, .d[1] = 0};
> +
> +assert(vstrs(, , , , 0, 2) == 1);
> +assert(v1.d[0] == 16);
> +assert(v1.d[1] == 0);
> +}
> +
> +int main(void)
> +{
> +test_ignored_match();
> +test_empty_needle();
> +test_max_length();
> +test_no_match();
> +return EXIT_SUCCESS;
> +}
[PATCH 3/3] tests/tcg/s390x: Test VSTRS
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/vxeh2_vstrs.c | 88 +
2 files changed, 89 insertions(+)
create mode 100644 tests/tcg/s390x/vxeh2_vstrs.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 1fc98099070..8ba36e5985b 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -73,6 +73,7 @@ ifneq ($(CROSS_CC_HAS_Z15),)
Z15_TESTS=vxeh2_vs
Z15_TESTS+=vxeh2_vcvt
Z15_TESTS+=vxeh2_vlstr
+Z15_TESTS+=vxeh2_vstrs
$(Z15_TESTS): CFLAGS+=-march=z15 -O2
TESTS+=$(Z15_TESTS)
endif
diff --git a/tests/tcg/s390x/vxeh2_vstrs.c b/tests/tcg/s390x/vxeh2_vstrs.c
new file mode 100644
index 000..313ec1d728f
--- /dev/null
+++ b/tests/tcg/s390x/vxeh2_vstrs.c
@@ -0,0 +1,88 @@
+/*
+ * Test the VSTRS instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+#include
+#include
+#include "vx.h"
+
+static inline __attribute__((__always_inline__)) int
+vstrs(S390Vector *v1, const S390Vector *v2, const S390Vector *v3,
+ const S390Vector *v4, const uint8_t m5, const uint8_t m6)
+{
+int cc;
+
+asm("vstrs %[v1],%[v2],%[v3],%[v4],%[m5],%[m6]\n"
+"ipm %[cc]"
+: [v1] "=v" (v1->v)
+, [cc] "=r" (cc)
+: [v2] "v" (v2->v)
+, [v3] "v" (v3->v)
+, [v4] "v" (v4->v)
+, [m5] "i" (m5)
+, [m6] "i" (m6)
+: "cc");
+
+return (cc >> 28) & 3;
+}
+
+static void test_ignored_match(void)
+{
+S390Vector v1;
+S390Vector v2 = {.d[0] = 0x222000205e41ULL, .d[1] = 0};
+S390Vector v3 = {.d[0] = 0x205e4100ULL, .d[1] = 0};
+S390Vector v4 = {.d[0] = 3, .d[1] = 0};
+
+assert(vstrs(, , , , 0, 2) == 1);
+assert(v1.d[0] == 16);
+assert(v1.d[1] == 0);
+}
+
+static void test_empty_needle(void)
+{
+S390Vector v1;
+S390Vector v2 = {.d[0] = 0x5300ULL, .d[1] = 0};
+S390Vector v3 = {.d[0] = 0, .d[1] = 0};
+S390Vector v4 = {.d[0] = 0, .d[1] = 0};
+
+assert(vstrs(, , , , 0, 0) == 2);
+assert(v1.d[0] == 0);
+assert(v1.d[1] == 0);
+}
+
+static void test_max_length(void)
+{
+S390Vector v1;
+S390Vector v2 = {.d[0] = 0x1122334455667700ULL, .d[1] = 0};
+S390Vector v3 = {.d[0] = 0, .d[1] = 0};
+S390Vector v4 = {.d[0] = 16, .d[1] = 0};
+
+assert(vstrs(, , , , 0, 0) == 3);
+assert(v1.d[0] == 7);
+assert(v1.d[1] == 0);
+}
+
+static void test_no_match(void)
+{
+S390Vector v1;
+S390Vector v2 = {.d[0] = 0xff000f00ULL, .d[1] = 0x82b};
+S390Vector v3 = {.d[0] = 0xfffeULL,
+ .d[1] = 0xULL};
+S390Vector v4 = {.d[0] = 11, .d[1] = 0};
+
+assert(vstrs(, , , , 0, 2) == 1);
+assert(v1.d[0] == 16);
+assert(v1.d[1] == 0);
+}
+
+int main(void)
+{
+test_ignored_match();
+test_empty_needle();
+test_max_length();
+test_no_match();
+return EXIT_SUCCESS;
+}
--
2.41.0
