Thomas Huth <th...@redhat.com> writes:
> The element size is encoded in the M3 field, not in the M4 > field. Let's also add a TCG test that shows the failing > behavior without this fix. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1248 > Signed-off-by: Thomas Huth <th...@redhat.com> > --- > tests/tcg/s390x/vf.c | 50 +++++++++++++++++++++++++++++ > target/s390x/tcg/translate_vx.c.inc | 2 +- > tests/tcg/s390x/Makefile.target | 6 ++++ > 3 files changed, 57 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/s390x/vf.c > > diff --git a/tests/tcg/s390x/vf.c b/tests/tcg/s390x/vf.c > new file mode 100644 > index 0000000000..fdc424ce7c > --- /dev/null > +++ b/tests/tcg/s390x/vf.c > @@ -0,0 +1,50 @@ > +/* > + * vf: vector facility tests > + */ > +#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"); > +} > + > +static int test_vistr(void) > +{ > + 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; > +} > + > +int main(int argc, char *argv[]) > +{ > + return test_vistr(); > +} > 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, > diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target > index c830313e67..f8e71a9439 100644 > --- a/tests/tcg/s390x/Makefile.target > +++ b/tests/tcg/s390x/Makefile.target > @@ -18,6 +18,12 @@ TESTS+=signals-s390x > TESTS+=branch-relative-long > TESTS+=noexec > > +Z13_TESTS=vf > +vf: LDFLAGS+=-lm > +$(Z13_TESTS): CFLAGS+=-march=z13 -O2 > +TESTS+=$(if $(shell $(CC) -march=z13 -S -o /dev/null -xc /dev/null \ > + >/dev/null 2>&1 && echo OK),$(Z13_TESTS)) > + I didn't realise there where a bunch of compile time tests in the s390x makefiles. The best practice (since Paolo's refactoring) is to emit a config-cc.mak to set some variables once, e.g.: config-cc.mak: Makefile $(quiet-@)( \ $(call cc-option,-march=armv8.1-a+sve, CROSS_CC_HAS_SVE); \ $(call cc-option,-march=armv8.1-a+sve2, CROSS_CC_HAS_SVE2); \ $(call cc-option,-march=armv8.3-a, CROSS_CC_HAS_ARMV8_3); \ $(call cc-option,-mbranch-protection=standard, CROSS_CC_HAS_ARMV8_BTI); \ $(call cc-option,-march=armv8.5-a+memtag, CROSS_CC_HAS_ARMV8_MTE)) 3> config-cc.mak -include config-cc.mak > Z14_TESTS=vfminmax > vfminmax: LDFLAGS+=-lm > $(Z14_TESTS): CFLAGS+=-march=z14 -O2 -- Alex Bennée
