[PATCH 0/2] target/s390x: Fix MXDB and MXDBR

[PATCH 0/2] target/s390x: Fix MXDB and MXDBR

[Ilya Leoshkevich]

Hi,

This is the last finding from [1].
With this fix, llvm-project testcases run without issues.

Best regards,
Ilya

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2211472

Ilya Leoshkevich (2):
  target/s390x: Fix MXDB and MXDBR
  tests/tcg/s390x: Test MXDB and MXDBR

 target/s390x/helper.h            |  2 +-
 target/s390x/tcg/fpu_helper.c    |  5 +++--
 target/s390x/tcg/insn-data.h.inc |  4 ++--
 target/s390x/tcg/translate.c     |  8 +-------
 tests/tcg/s390x/Makefile.target  |  1 +
 tests/tcg/s390x/mxdb.c           | 30 ++++++++++++++++++++++++++++++
 6 files changed, 38 insertions(+), 12 deletions(-)
 create mode 100644 tests/tcg/s390x/mxdb.c

-- 
2.40.1

[PATCH 1/2] target/s390x: Fix MXDB and MXDBR

[Ilya Leoshkevich]

These instructions multiply 64 bits by 64 bits, not 128 bits by 64 bits.

Reported-by: Tulio Magno Quites Machado Filho <tuliom@redhat.com>
Fixes: 2b91240f95fd ("target/s390x: Use Int128 for passing float128")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/helper.h            | 2 +-
 target/s390x/tcg/fpu_helper.c    | 5 +++--
 target/s390x/tcg/insn-data.h.inc | 4 ++--
 target/s390x/tcg/translate.c     | 8 +-------
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 7529e725f29..6bc01df73d7 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -50,7 +50,7 @@ DEF_HELPER_FLAGS_3(meeb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(mdeb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(mdb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(mxb, TCG_CALL_NO_WG, i128, env, i128, i128)
-DEF_HELPER_FLAGS_3(mxdb, TCG_CALL_NO_WG, i128, env, i128, i64)
+DEF_HELPER_FLAGS_3(mxdb, TCG_CALL_NO_WG, i128, env, i64, i64)
 DEF_HELPER_FLAGS_2(ldeb, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_3(ldxb, TCG_CALL_NO_WG, i64, env, i128, i32)
 DEF_HELPER_FLAGS_2(lxdb, TCG_CALL_NO_WG, i128, env, i64)
diff --git a/target/s390x/tcg/fpu_helper.c b/target/s390x/tcg/fpu_helper.c
index 0bdab5bcf71..57e58292833 100644
--- a/target/s390x/tcg/fpu_helper.c
+++ b/target/s390x/tcg/fpu_helper.c
@@ -321,10 +321,11 @@ Int128 HELPER(mxb)(CPUS390XState *env, Int128 a, Int128 b)
 }
 
 /* 128/64-bit FP multiplication */
-Int128 HELPER(mxdb)(CPUS390XState *env, Int128 a, uint64_t f2)
+Int128 HELPER(mxdb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
 {
+    float128 f1_128 = float64_to_float128(f1, &env->fpu_status);
     float128 ret = float64_to_float128(f2, &env->fpu_status);
-    ret = float128_mul(ARG128(a), ret, &env->fpu_status);
+    ret = float128_mul(f1_128, ret, &env->fpu_status);
     handle_exceptions(env, false, GETPC());
     return RET128(ret);
 }
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 937e18ea9d9..0a45dbbcda8 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -668,11 +668,11 @@
     F(0xb31c, MDBR,    RRE,   Z,   f1, f2, new, f1, mdb, 0, IF_BFP)
     F(0xb34c, MXBR,    RRE,   Z,   x1, x2, new_x, x1, mxb, 0, IF_BFP)
     F(0xb30c, MDEBR,   RRE,   Z,   f1, e2, new, f1, mdeb, 0, IF_BFP)
-    F(0xb307, MXDBR,   RRE,   Z,   0, f2, x1, x1, mxdb, 0, IF_BFP)
+    F(0xb307, MXDBR,   RRE,   Z,   f1, f2, new_x, x1, mxdb, 0, IF_BFP)
     F(0xed17, MEEB,    RXE,   Z,   e1, m2_32u, new, e1, meeb, 0, IF_BFP)
     F(0xed1c, MDB,     RXE,   Z,   f1, m2_64, new, f1, mdb, 0, IF_BFP)
     F(0xed0c, MDEB,    RXE,   Z,   f1, m2_32u, new, f1, mdeb, 0, IF_BFP)
-    F(0xed07, MXDB,    RXE,   Z,   0, m2_64, x1, x1, mxdb, 0, IF_BFP)
+    F(0xed07, MXDB,    RXE,   Z,   f1, m2_64, new_x, x1, mxdb, 0, IF_BFP)
 /* MULTIPLY HALFWORD */
     C(0x4c00, MH,      RX_a,  Z,   r1_o, m2_16s, new, r1_32, mul, 0)
     C(0xe37c, MHY,     RXY_a, GIE, r1_o, m2_16s, new, r1_32, mul, 0)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 3eb3708d551..3ac573dfce5 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -3421,7 +3421,7 @@ static DisasJumpType op_mxb(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_mxdb(DisasContext *s, DisasOps *o)
 {
-    gen_helper_mxdb(o->out_128, cpu_env, o->in1_128, o->in2);
+    gen_helper_mxdb(o->out_128, cpu_env, o->in1, o->in2);
     return DISAS_NEXT;
 }
 
@@ -5183,12 +5183,6 @@ static void prep_r1_P(DisasContext *s, DisasOps *o)
 }
 #define SPEC_prep_r1_P SPEC_r1_even
 
-static void prep_x1(DisasContext *s, DisasOps *o)
-{
-    o->out_128 = load_freg_128(get_field(s, r1));
-}
-#define SPEC_prep_x1 SPEC_r1_f128
-
 /* ====================================================================== */
 /* The "Write OUTput" generators.  These generally perform some non-trivial
    copy of data to TCG globals, or to main memory.  The trivial cases are
-- 
2.40.1

[PATCH 2/2] tests/tcg/s390x: Test MXDB and MXDBR

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/mxdb.c          | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 tests/tcg/s390x/mxdb.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 3c239fdd082..d33960caa0a 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -35,6 +35,7 @@ TESTS+=chrl
 TESTS+=rxsbg
 TESTS+=ex-relative-long
 TESTS+=ex-branch
+TESTS+=mxdb
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/mxdb.c b/tests/tcg/s390x/mxdb.c
new file mode 100644
index 00000000000..ae922559d3d
--- /dev/null
+++ b/tests/tcg/s390x/mxdb.c
@@ -0,0 +1,30 @@
+/*
+ * Test the MXDB and MXDBR instructions.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+
+int main(void)
+{
+    union {
+        double d[2];
+        long double ld;
+    } a;
+    double b;
+
+    a.d[0] = 1.2345;
+    a.d[1] = 999;
+    b = 6.789;
+    asm("mxdb %[a],%[b]" : [a] "+f" (a.ld) : [b] "R" (b));
+    assert(a.ld > 8.38 && a.ld < 8.39);
+
+    a.d[0] = 1.2345;
+    a.d[1] = 999;
+    b = 6.789;
+    asm("mxdbr %[a],%[b]" : [a] "+f" (a.ld) : [b] "f" (b));
+    assert(a.ld > 8.38 && a.ld < 8.39);
+
+    return EXIT_SUCCESS;
+}
-- 
2.40.1

Re: [PATCH 1/2] target/s390x: Fix MXDB and MXDBR

[David Hildenbrand]

On 02.06.23 00:30, Ilya Leoshkevich wrote:
> These instructions multiply 64 bits by 64 bits, not 128 bits by 64 bits.
> 
> Reported-by: Tulio Magno Quites Machado Filho <tuliom@redhat.com>
> Fixes: 2b91240f95fd ("target/s390x: Use Int128 for passing float128")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---

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

-- 
Thanks,

David / dhildenb

Re: [PATCH 2/2] tests/tcg/s390x: Test MXDB and MXDBR

[David Hildenbrand]

On 02.06.23 00:30, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/Makefile.target |  1 +
>   tests/tcg/s390x/mxdb.c          | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
>   create mode 100644 tests/tcg/s390x/mxdb.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 3c239fdd082..d33960caa0a 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -35,6 +35,7 @@ TESTS+=chrl
>   TESTS+=rxsbg
>   TESTS+=ex-relative-long
>   TESTS+=ex-branch
> +TESTS+=mxdb
>   
>   cdsg: CFLAGS+=-pthread
>   cdsg: LDFLAGS+=-pthread
> diff --git a/tests/tcg/s390x/mxdb.c b/tests/tcg/s390x/mxdb.c
> new file mode 100644
> index 00000000000..ae922559d3d
> --- /dev/null
> +++ b/tests/tcg/s390x/mxdb.c
> @@ -0,0 +1,30 @@
> +/*
> + * Test the MXDB and MXDBR instructions.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> +    union {
> +        double d[2];
> +        long double ld;
> +    } a;
> +    double b;
> +
> +    a.d[0] = 1.2345;
> +    a.d[1] = 999;
> +    b = 6.789;
> +    asm("mxdb %[a],%[b]" : [a] "+f" (a.ld) : [b] "R" (b));
> +    assert(a.ld > 8.38 && a.ld < 8.39);
> +
> +    a.d[0] = 1.2345;
> +    a.d[1] = 999;
> +    b = 6.789;
> +    asm("mxdbr %[a],%[b]" : [a] "+f" (a.ld) : [b] "f" (b));
> +    assert(a.ld > 8.38 && a.ld < 8.39);
> +
> +    return EXIT_SUCCESS;
> +}

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

-- 
Thanks,

David / dhildenb