[PATCH 0/3] s390x: Fix for vistr instruction & addition to TCG tests

[PATCH 0/3] s390x: Fix for vistr instruction & addition to TCG tests

[Thomas Huth]

1) Improve tests/tcg/s390x/Makefile.target to look for -march flags only once
2) Fix a problem with the vistr instruction
3) Add a test for the vistr instruction

Thomas Huth (3):
  tests/tcg/s390x: Test compiler flags only once, not every time
  target/s390x: Fix emulation of the VISTR instruction
  tests/tcg/s390x: Add a test for the vistr instruction

 tests/tcg/s390x/vistr.c             | 45 +++++++++++++++++++++++++++++
 target/s390x/tcg/translate_vx.c.inc |  2 +-
 tests/tcg/s390x/Makefile.target     | 31 +++++++++++++-------
 3 files changed, 67 insertions(+), 11 deletions(-)
 create mode 100644 tests/tcg/s390x/vistr.c

-- 
2.31.1

[PATCH 1/3] tests/tcg/s390x: Test compiler flags only once, not every time

[Thomas Huth]

This is common practice, see the Makefile.target in the aarch64
folder for example.

Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/tcg/s390x/Makefile.target | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index c830313e67..29c8af8207 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -1,6 +1,13 @@
 S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
 VPATH+=$(S390X_SRC)
 CFLAGS+=-march=zEC12 -m64
+
+config-cc.mak: Makefile
+	$(quiet-@)( \
+	    $(call cc-option,-march=z14, CROSS_CC_HAS_Z14); \
+	    $(call cc-option,-march=z15, CROSS_CC_HAS_Z15)) 3> config-cc.mak
+-include config-cc.mak
+
 TESTS+=hello-s390x
 TESTS+=csst
 TESTS+=ipm
@@ -18,20 +25,20 @@ TESTS+=signals-s390x
 TESTS+=branch-relative-long
 TESTS+=noexec
 
+ifneq ($(CROSS_CC_HAS_Z14),)
 Z14_TESTS=vfminmax
 vfminmax: LDFLAGS+=-lm
 $(Z14_TESTS): CFLAGS+=-march=z14 -O2
+TESTS+=$(Z14_TESTS)
+endif
 
-TESTS+=$(if $(shell $(CC) -march=z14 -S -o /dev/null -xc /dev/null \
-			 >/dev/null 2>&1 && echo OK),$(Z14_TESTS))
-
-VECTOR_TESTS=vxeh2_vs
-VECTOR_TESTS+=vxeh2_vcvt
-VECTOR_TESTS+=vxeh2_vlstr
-$(VECTOR_TESTS): CFLAGS+=-march=z15 -O2
-
-TESTS+=$(if $(shell $(CC) -march=z15 -S -o /dev/null -xc /dev/null \
-			 >/dev/null 2>&1 && echo OK),$(VECTOR_TESTS))
+ifneq ($(CROSS_CC_HAS_Z15),)
+Z15_TESTS=vxeh2_vs
+Z15_TESTS+=vxeh2_vcvt
+Z15_TESTS+=vxeh2_vlstr
+$(Z15_TESTS): CFLAGS+=-march=z15 -O2
+TESTS+=$(Z15_TESTS)
+endif
 
 ifneq ($(HAVE_GDB_BIN),)
 GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
-- 
2.31.1

[PATCH 2/3] target/s390x: Fix emulation of the VISTR instruction

[Thomas Huth]

The element size is encoded in the M3 field, not in the M4
field.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1248
Fixes: be6324c6b734 ("s390x/tcg: Implement VECTOR ISOLATE STRING")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/tcg/translate_vx.c.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/tcg/translate_vx.c.inc b/target/s390x/tcg/translate_vx.c.inc
index 3526ba3e3b..b69c1a111c 100644
--- a/target/s390x/tcg/translate_vx.c.inc
+++ b/target/s390x/tcg/translate_vx.c.inc
@@ -2723,7 +2723,7 @@ static DisasJumpType op_vfene(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_vistr(DisasContext *s, DisasOps *o)
 {
-    const uint8_t es = get_field(s, m4);
+    const uint8_t es = get_field(s, m3);
     const uint8_t m5 = get_field(s, m5);
     static gen_helper_gvec_2 * const g[3] = {
         gen_helper_gvec_vistr8,
-- 
2.31.1

[PATCH 3/3] tests/tcg/s390x: Add a test for the vistr instruction

[Thomas Huth]

This test can be used to verify that the change in the previous
commit is indeed fixing the problem with the M3 vs. M4 field
mixup.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/tcg/s390x/vistr.c         | 45 +++++++++++++++++++++++++++++++++
 tests/tcg/s390x/Makefile.target |  4 +++
 2 files changed, 49 insertions(+)
 create mode 100644 tests/tcg/s390x/vistr.c

diff --git a/tests/tcg/s390x/vistr.c b/tests/tcg/s390x/vistr.c
new file mode 100644
index 0000000000..8e3e987d71
--- /dev/null
+++ b/tests/tcg/s390x/vistr.c
@@ -0,0 +1,45 @@
+/*
+ * Test the VECTOR ISOLATE STRING (vistr) instruction
+ */
+#include <stdint.h>
+#include <stdio.h>
+#include "vx.h"
+
+static inline void vistr(S390Vector *v1, S390Vector *v2,
+                         const uint8_t m3, const uint8_t m5)
+{
+    asm volatile("vistr %[v1], %[v2], %[m3], %[m5]\n"
+                 : [v1] "=v" (v1->v)
+                 : [v2]  "v" (v2->v)
+                 , [m3]  "i" (m3)
+                 , [m5]  "i" (m5)
+                 : "cc");
+}
+
+int main(int argc, char *argv[])
+{
+    S390Vector vd = {};
+    S390Vector vs16 = {
+        .h[0] = 0x1234, .h[1] = 0x0056, .h[2] = 0x7800, .h[3] = 0x0000,
+        .h[4] = 0x0078, .h[5] = 0x0000, .h[6] = 0x6543, .h[7] = 0x2100
+    };
+    S390Vector vs32 = {
+        .w[0] = 0x12340000, .w[1] = 0x78654300,
+        .w[2] = 0x0, .w[3] = 0x12,
+    };
+
+    vistr(&vd, &vs16, 1, 0);
+    if (vd.h[0] != 0x1234 || vd.h[1] != 0x0056 || vd.h[2] != 0x7800 ||
+        vd.h[3] || vd.h[4] || vd.h[5] || vd.h[6] || vd.h[7]) {
+        puts("ERROR: vitrh failed!");
+        return 1;
+    }
+
+    vistr(&vd, &vs32, 2, 0);
+    if (vd.w[0] != 0x12340000 || vd.w[1] != 0x78654300 || vd.w[2] || vd.w[3]) {
+        puts("ERROR: vitrf failed!");
+        return 1;
+    }
+
+    return 0;
+}
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 29c8af8207..07fcc6d0ce 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -25,6 +25,10 @@ TESTS+=signals-s390x
 TESTS+=branch-relative-long
 TESTS+=noexec
 
+Z13_TESTS=vistr
+$(Z13_TESTS): CFLAGS+=-march=z13 -O2
+TESTS+=$(Z13_TESTS)
+
 ifneq ($(CROSS_CC_HAS_Z14),)
 Z14_TESTS=vfminmax
 vfminmax: LDFLAGS+=-lm
-- 
2.31.1

Re: [PATCH 1/3] tests/tcg/s390x: Test compiler flags only once, not every time

[David Hildenbrand]

On 12.10.22 20:27, Thomas Huth wrote:
> This is common practice, see the Makefile.target in the aarch64
> folder for example.
> 
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/tcg/s390x/Makefile.target | 27 +++++++++++++++++----------
>   1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index c830313e67..29c8af8207 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -1,6 +1,13 @@
>   S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
>   VPATH+=$(S390X_SRC)
>   CFLAGS+=-march=zEC12 -m64
> +
> +config-cc.mak: Makefile
> +	$(quiet-@)( \
> +	    $(call cc-option,-march=z14, CROSS_CC_HAS_Z14); \
> +	    $(call cc-option,-march=z15, CROSS_CC_HAS_Z15)) 3> config-cc.mak
> +-include config-cc.mak
> +
>   TESTS+=hello-s390x
>   TESTS+=csst
>   TESTS+=ipm
> @@ -18,20 +25,20 @@ TESTS+=signals-s390x
>   TESTS+=branch-relative-long
>   TESTS+=noexec
>   
> +ifneq ($(CROSS_CC_HAS_Z14),)
>   Z14_TESTS=vfminmax
>   vfminmax: LDFLAGS+=-lm
>   $(Z14_TESTS): CFLAGS+=-march=z14 -O2
> +TESTS+=$(Z14_TESTS)
> +endif
>   
> -TESTS+=$(if $(shell $(CC) -march=z14 -S -o /dev/null -xc /dev/null \
> -			 >/dev/null 2>&1 && echo OK),$(Z14_TESTS))
> -
> -VECTOR_TESTS=vxeh2_vs
> -VECTOR_TESTS+=vxeh2_vcvt
> -VECTOR_TESTS+=vxeh2_vlstr
> -$(VECTOR_TESTS): CFLAGS+=-march=z15 -O2
> -
> -TESTS+=$(if $(shell $(CC) -march=z15 -S -o /dev/null -xc /dev/null \
> -			 >/dev/null 2>&1 && echo OK),$(VECTOR_TESTS))
> +ifneq ($(CROSS_CC_HAS_Z15),)
> +Z15_TESTS=vxeh2_vs
> +Z15_TESTS+=vxeh2_vcvt
> +Z15_TESTS+=vxeh2_vlstr
> +$(Z15_TESTS): CFLAGS+=-march=z15 -O2
> +TESTS+=$(Z15_TESTS)
> +endif
>   
>   ifneq ($(HAVE_GDB_BIN),)
>   GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 2/3] target/s390x: Fix emulation of the VISTR instruction

[David Hildenbrand]

On 12.10.22 20:27, Thomas Huth wrote:
> The element size is encoded in the M3 field, not in the M4
> field.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1248
> Fixes: be6324c6b734 ("s390x/tcg: Implement VECTOR ISOLATE STRING")
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 3/3] tests/tcg/s390x: Add a test for the vistr instruction

[David Hildenbrand]

On 12.10.22 20:27, Thomas Huth wrote:
> This test can be used to verify that the change in the previous
> commit is indeed fixing the problem with the M3 vs. M4 field
> mixup.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 1/3] tests/tcg/s390x: Test compiler flags only once, not every time

[Richard Henderson]

On 10/13/22 11:27, Thomas Huth wrote:
> This is common practice, see the Makefile.target in the aarch64
> folder for example.
> 
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/tcg/s390x/Makefile.target | 27 +++++++++++++++++----------
>   1 file changed, 17 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/3] tests/tcg/s390x: Add a test for the vistr instruction

[Richard Henderson]

On 10/13/22 11:27, Thomas Huth wrote:
> This test can be used to verify that the change in the previous
> commit is indeed fixing the problem with the M3 vs. M4 field
> mixup.
> 
> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   tests/tcg/s390x/vistr.c         | 45 +++++++++++++++++++++++++++++++++
>   tests/tcg/s390x/Makefile.target |  4 +++
>   2 files changed, 49 insertions(+)
>   create mode 100644 tests/tcg/s390x/vistr.c

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~