Re: [PATCH v4 10/11] tests/tcg/s390x: Tests for Vector Enhancements Facility 2

[David Hildenbrand]

On 22.03.22 01:04, David Miller wrote:
> Signed-off-by: David Miller <dmiller423@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Checkpatch complains about three things:

ERROR: space prohibited between function name and open parenthesis '('
#262: FILE: tests/tcg/s390x/vxeh2_vlstr.c:115:
+    vler (&vd, &vs, ES16);  vtst(vd, vt_v_er16);

ERROR: space prohibited between function name and open parenthesis '('
#265: FILE: tests/tcg/s390x/vxeh2_vlstr.c:118:
+    vlbr (&vd, &vs, ES16);  vtst(vd, vt_v_br16);

ERROR: space prohibited between function name and open parenthesis '('
#383: FILE: tests/tcg/s390x/vxeh2_vs.c:84:
+    vsl (&vd, &vs, &vsi);       vtst(vd, vt_vsl);

total: 3 errors, 1 warnings, 348 lines checked


> ---
>  tests/tcg/s390x/Makefile.target |   8 ++
>  tests/tcg/s390x/vxeh2_vcvt.c    |  97 +++++++++++++++++++++
>  tests/tcg/s390x/vxeh2_vlstr.c   | 146 ++++++++++++++++++++++++++++++++
>  tests/tcg/s390x/vxeh2_vs.c      |  91 ++++++++++++++++++++
>  4 files changed, 342 insertions(+)
>  create mode 100644 tests/tcg/s390x/vxeh2_vcvt.c
>  create mode 100644 tests/tcg/s390x/vxeh2_vlstr.c
>  create mode 100644 tests/tcg/s390x/vxeh2_vs.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 8c9b6a13ce..921a056dd1 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -16,6 +16,14 @@ TESTS+=shift
>  TESTS+=trap
>  TESTS+=signals-s390x
>  
> +VECTOR_TESTS=vxeh2_vs
> +VECTOR_TESTS+=vxeh2_vcvt
> +VECTOR_TESTS+=vxeh2_vlstr
> +
> +TESTS+=$(VECTOR_TESTS)
> +
> +$(VECTOR_TESTS): CFLAGS+=-march=z15 -O2


@Thomas, will that survive our test framework already, or do we have to
wait for the debain11 changes?

> +
>  ifneq ($(HAVE_GDB_BIN),)
>  GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
>  
> diff --git a/tests/tcg/s390x/vxeh2_vcvt.c b/tests/tcg/s390x/vxeh2_vcvt.c
> new file mode 100644
> index 0000000000..71ecbd77b0
> --- /dev/null
> +++ b/tests/tcg/s390x/vxeh2_vcvt.c
> @@ -0,0 +1,97 @@
> +/*
> + * vxeh2_vcvt: vector-enhancements facility 2 vector convert *
> + */
> +#include <stdint.h>
> +
> +typedef union S390Vector {
> +    uint64_t d[2];  /* doubleword */
> +    uint32_t w[4];  /* word */
> +    uint16_t h[8];  /* halfword */
> +    uint8_t  b[16]; /* byte */
> +    float    f[4];
> +    double   fd[2];
> +    __uint128_t v;
> +} S390Vector;

Let's move that into a separate header (vx.h?) so we can reuse it.

> +
> +#define M_S 8
> +#define M4_XxC 4
> +#define M4_def M4_XxC
[...]

> diff --git a/tests/tcg/s390x/vxeh2_vlstr.c b/tests/tcg/s390x/vxeh2_vlstr.c
> new file mode 100644
> index 0000000000..bf2954e86d
> --- /dev/null
> +++ b/tests/tcg/s390x/vxeh2_vlstr.c
> @@ -0,0 +1,146 @@
> +/*
> + * vxeh2_vlstr: vector-enhancements facility 2 vector load/store reversed *
> + */
> +#include <stdint.h>
> +
> +typedef union S390Vector {
> +    uint64_t d[2];  /* doubleword */
> +    uint32_t w[4];  /* word */
> +    uint16_t h[8];  /* halfword */
> +    uint8_t  b[16]; /* byte */
> +    __uint128_t v;
> +} S390Vector;
> +
> +#define ES8  0
> +#define ES16 1
> +#define ES32 2
> +#define ES64 3

These should probably also go to the new header.

> +
> +#define vtst(v1, v2) \
> +    if (v1.d[0] != v2.d[0] || v1.d[1] != v2.d[1]) { \
> +        return 1;     \
> +    }
> +
> +static inline void vler(S390Vector *v1, const void *va, uint8_t m3)
> +{
> +    asm volatile("vler %[v1], 0(%[va]), %[m3]\n"
> +                : [v1] "+v" (v1->v)
> +                : [va]  "d" (va)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +static inline void vster(S390Vector *v1, const void *va, uint8_t m3)
> +{
> +    asm volatile("vster %[v1], 0(%[va]), %[m3]\n"
> +                : [va] "+d" (va)
> +                : [v1]  "v" (v1->v)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +static inline void vlbr(S390Vector *v1, void *va, const uint8_t m3)
> +{
> +    asm volatile("vlbr %[v1], 0(%[va]), %[m3]\n"
> +                : [v1] "+v" (v1->v)
> +                : [va]  "d" (va)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +static inline void vstbr(S390Vector *v1, void *va, const uint8_t m3)
> +{
> +    asm volatile("vstbr %[v1], 0(%[va]), %[m3]\n"
> +                : [va] "+d" (va)
> +                : [v1]  "v" (v1->v)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +
> +static inline void vlebrh(S390Vector *v1, void *va, const uint8_t m3)
> +{
> +    asm volatile("vlebrh %[v1], 0(%[va]), %[m3]\n"
> +                : [v1] "+v" (v1->v)
> +                : [va]  "d" (va)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +static inline void vstebrh(S390Vector *v1, void *va, const uint8_t m3)
> +{
> +    asm volatile("vstebrh %[v1], 0(%[va]), %[m3]\n"
> +                : [va] "+d" (va)
> +                : [v1]  "v" (v1->v)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +static inline void vllebrz(S390Vector *v1, void *va, const uint8_t m3)
> +{
> +    asm volatile("vllebrz %[v1], 0(%[va]), %[m3]\n"
> +                : [v1] "+v" (v1->v)
> +                : [va]  "d" (va)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +static inline void vlbrrep(S390Vector *v1, void *va, const uint8_t m3)
> +{
> +    asm volatile("vlbrrep %[v1], 0(%[va]), %[m3]\n"
> +                : [v1] "+v" (v1->v)
> +                : [va]  "d" (va)
> +                , [m3]  "i" (m3)
> +                : "memory");
> +}
> +
> +

Superfluous empty line.

> +int main(int argc, char *argv[])
> +{
> +    S390Vector vd = { .d[0] = 0, .d[1] = 0 };
> +    S390Vector vs = { .d[0] = 0x8FEEDDCCBBAA9988ull,
> +                      .d[1] = 0x7766554433221107ull };
> +
> +    const S390Vector vt_v_er16 = {
> +        .h[0] = 0x1107, .h[1] = 0x3322, .h[2] = 0x5544, .h[3] = 0x7766,
> +        .h[4] = 0x9988, .h[5] = 0xBBAA, .h[6] = 0xDDCC, .h[7] = 0x8FEE };
> +
> +    const S390Vector vt_v_br16 = {
> +        .h[0] = 0xEE8F, .h[1] = 0xCCDD, .h[2] = 0xAABB, .h[3] = 0x8899,
> +        .h[4] = 0x6677, .h[5] = 0x4455, .h[6] = 0x2233, .h[7] = 0x0711 };
> +
> +    int ix;
> +    uint64_t ss64 = 0xFEEDFACE0BADBEEFull, sd64 = 0;
> +
> +    vler (&vd, &vs, ES16);  vtst(vd, vt_v_er16);
> +    vster(&vs, &vd, ES16);  vtst(vd, vt_v_er16);
> +
> +    vlbr (&vd, &vs, ES16);  vtst(vd, vt_v_br16);
> +    vstbr(&vs, &vd, ES16);  vtst(vd, vt_v_br16);
> +

Please put each statement on a new line.

> +    vlebrh(&vd, &ss64, 5);
> +    if (0xEDFE != vd.h[5]) {
> +        return 1;
> +    }
> +
> +    vstebrh(&vs, (uint8_t *)&sd64 + 4, 7);
> +    if (0x0000000007110000ull != sd64) {
> +        return 1;
> +    }
> +
> +    vllebrz(&vd, (uint8_t *)&ss64 + 3, 2);
> +    for (ix = 0; ix < 4; ix++) {
> +        if (vd.w[ix] != (ix != 1 ? 0 : 0xBEAD0BCE)) {
> +            return 1;
> +        }
> +    }
> +
> +    vlbrrep(&vd, (uint8_t *)&ss64 + 4, 1);
> +    for (ix = 0; ix < 8; ix++) {
> +        if (0xAD0B != vd.h[ix]) {
> +            return 1;
> +        }
> +    }
> +
> +    return 0;
> +}
> diff --git a/tests/tcg/s390x/vxeh2_vs.c b/tests/tcg/s390x/vxeh2_vs.c
> new file mode 100644
> index 0000000000..04a3d4d7bb
> --- /dev/null
> +++ b/tests/tcg/s390x/vxeh2_vs.c

[...]

> +int main(int argc, char *argv[])
> +{
> +    const S390Vector vt_vsl  = { .d[0] = 0x7FEDBB32D5AA311Dull,
> +                                 .d[1] = 0xBB65AA10912220C0ull };
> +    const S390Vector vt_vsra = { .d[0] = 0xF1FE6E7399AA5466ull,
> +                                 .d[1] = 0x0E762A5188221044ull };
> +    const S390Vector vt_vsrl = { .d[0] = 0x11FE6E7399AA5466ull,
> +                                 .d[1] = 0x0E762A5188221044ull };
> +    const S390Vector vt_vsld = { .d[0] = 0x7F76EE65DD54CC43ull,
> +                                 .d[1] = 0xBB32AA2199108838ull };
> +    const S390Vector vt_vsrd = { .d[0] = 0x0E060802040E000Aull,
> +                                 .d[1] = 0x0C060802040E000Aull };
> +    S390Vector vs  = { .d[0] = 0x8FEEDDCCBBAA9988ull,
> +                       .d[1] = 0x7766554433221107ull };
> +    S390Vector  vd = { .d[0] = 0, .d[1] = 0 };
> +    S390Vector vsi = { .d[0] = 0, .d[1] = 0 };
> +
> +    for (int ix = 0; ix < 16; ix++) {
> +        vsi.b[ix] = (1 + (5 ^ ~ix)) & 7;
> +    }
> +
> +    vsl (&vd, &vs, &vsi);       vtst(vd, vt_vsl);
> +    vsra(&vd, &vs, &vsi);       vtst(vd, vt_vsra);
> +    vsrl(&vd, &vs, &vsi);       vtst(vd, vt_vsrl);
> +    vsld(&vd, &vs, &vsi, 3);  vtst(vd, vt_vsld);
> +    vsrd(&vd, &vs, &vsi, 15); vtst(vd, vt_vsrd);

Dito. Please put each statement on a new line.

> +
> +    return 0;
> +}


-- 
Thanks,

David / dhildenb