[v1] target/s390x: Fix R[NOX]SBG with T=1

[PATCH 2/2] tests/tcg/s390x: Add rxsbg.c

Posted by Ilya Leoshkevich 2 years, 11 months ago

Add a small test for RXSBG with T=1 to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/rxsbg.c         | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 tests/tcg/s390x/rxsbg.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index cf93b966862..b4d0d704534 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -29,6 +29,7 @@ TESTS+=clst
 TESTS+=long-double
 TESTS+=cdsg
 TESTS+=chrl
+TESTS+=rxsbg
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/rxsbg.c b/tests/tcg/s390x/rxsbg.c
new file mode 100644
index 00000000000..b7f35411899
--- /dev/null
+++ b/tests/tcg/s390x/rxsbg.c
@@ -0,0 +1,25 @@
+/*
+ * Smoke test RXSBG instruction with T=1.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+
+int main(void)
+{
+    unsigned long r1, r2, cc;
+
+    r1 = 0xc8dc86a225a77bb4;
+    r2 = 0xd6aff24fa3e7320;
+    cc = 0;
+    asm("rxsbg %[r1],%[r2],177,43,228\n"
+        "ipm %[cc]"
+        : [cc] "+r" (cc)
+        : [r1] "r" (r1)
+        , [r2] "r" (r2)
+        : "cc");
+    cc = (cc >> 28) & 1;
+    assert(cc == 1);
+
+    return EXIT_SUCCESS;
+}
-- 
2.39.2

Re: [PATCH 2/2] tests/tcg/s390x: Add rxsbg.c

Posted by Thomas Huth 2 years, 11 months ago

On 15/03/2023 00.34, Ilya Leoshkevich wrote:
> Add a small test for RXSBG with T=1 to prevent regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/Makefile.target |  1 +
>   tests/tcg/s390x/rxsbg.c         | 25 +++++++++++++++++++++++++
>   2 files changed, 26 insertions(+)
>   create mode 100644 tests/tcg/s390x/rxsbg.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index cf93b966862..b4d0d704534 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -29,6 +29,7 @@ TESTS+=clst
>   TESTS+=long-double
>   TESTS+=cdsg
>   TESTS+=chrl
> +TESTS+=rxsbg
>   
>   cdsg: CFLAGS+=-pthread
>   cdsg: LDFLAGS+=-pthread
> diff --git a/tests/tcg/s390x/rxsbg.c b/tests/tcg/s390x/rxsbg.c
> new file mode 100644
> index 00000000000..b7f35411899
> --- /dev/null
> +++ b/tests/tcg/s390x/rxsbg.c
> @@ -0,0 +1,25 @@
> +/*
> + * Smoke test RXSBG instruction with T=1.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> +    unsigned long r1, r2, cc;
> +
> +    r1 = 0xc8dc86a225a77bb4;
> +    r2 = 0xd6aff24fa3e7320;
> +    cc = 0;
> +    asm("rxsbg %[r1],%[r2],177,43,228\n"
> +        "ipm %[cc]"
> +        : [cc] "+r" (cc)
> +        : [r1] "r" (r1)
> +        , [r2] "r" (r2)
> +        : "cc");
> +    cc = (cc >> 28) & 1;
> +    assert(cc == 1);
> +
> +    return EXIT_SUCCESS;
> +}

This also fails with Clang 15:
tests/tcg/s390x/rxsbg.c:15:9: error: invalid operand for instruction
     asm("rxsbg %[r1],%[r2],177,43,228\n"
         ^
<inline asm>:1:23: note: instantiated into assembly here
         rxsbg %r1,%r2,177,43,228
                              ^

  Thomas

Re: [PATCH 2/2] tests/tcg/s390x: Add rxsbg.c

Posted by Ilya Leoshkevich 2 years, 11 months ago

On Wed, 2023-03-15 at 19:12 +0100, Thomas Huth wrote:
> On 15/03/2023 00.34, Ilya Leoshkevich wrote:
> > Add a small test for RXSBG with T=1 to prevent regressions.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   tests/tcg/s390x/Makefile.target |  1 +
> >   tests/tcg/s390x/rxsbg.c         | 25 +++++++++++++++++++++++++
> >   2 files changed, 26 insertions(+)
> >   create mode 100644 tests/tcg/s390x/rxsbg.c
> > 
> > diff --git a/tests/tcg/s390x/Makefile.target
> > b/tests/tcg/s390x/Makefile.target
> > index cf93b966862..b4d0d704534 100644
> > --- a/tests/tcg/s390x/Makefile.target
> > +++ b/tests/tcg/s390x/Makefile.target
> > @@ -29,6 +29,7 @@ TESTS+=clst
> >   TESTS+=long-double
> >   TESTS+=cdsg
> >   TESTS+=chrl
> > +TESTS+=rxsbg
> >   
> >   cdsg: CFLAGS+=-pthread
> >   cdsg: LDFLAGS+=-pthread
> > diff --git a/tests/tcg/s390x/rxsbg.c b/tests/tcg/s390x/rxsbg.c
> > new file mode 100644
> > index 00000000000..b7f35411899
> > --- /dev/null
> > +++ b/tests/tcg/s390x/rxsbg.c
> > @@ -0,0 +1,25 @@
> > +/*
> > + * Smoke test RXSBG instruction with T=1.
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#include <assert.h>
> > +#include <stdlib.h>
> > +
> > +int main(void)
> > +{
> > +    unsigned long r1, r2, cc;
> > +
> > +    r1 = 0xc8dc86a225a77bb4;
> > +    r2 = 0xd6aff24fa3e7320;
> > +    cc = 0;
> > +    asm("rxsbg %[r1],%[r2],177,43,228\n"
> > +        "ipm %[cc]"
> > +        : [cc] "+r" (cc)
> > +        : [r1] "r" (r1)
> > +        , [r2] "r" (r2)
> > +        : "cc");
> > +    cc = (cc >> 28) & 1;
> > +    assert(cc == 1);
> > +
> > +    return EXIT_SUCCESS;
> > +}
> 
> This also fails with Clang 15:
> tests/tcg/s390x/rxsbg.c:15:9: error: invalid operand for instruction
>      asm("rxsbg %[r1],%[r2],177,43,228\n"
>          ^
> <inline asm>:1:23: note: instantiated into assembly here
>          rxsbg %r1,%r2,177,43,228
>                               ^
> 
>   Thomas
> 

This seems to be a clang bug. PoP says:

    Bit 1 of the I3 field and bits 0-1 of the I4 field (bits 17
    and 24-25 of the instruction) are reserved and should
    contain zeros; otherwise, the program may not oper-
    ate compatibly in the future. Bits 0-1 of the I5 field
    (bits 32-33 of the instruction) are ignored.

But LLVM has:

    imm32zx8:$I4, imm32zx6:$I5

which looks like a mixup (should be imm32zx6 + imm32zx8 IMHO).

I guess there is not much we can do about this at the moment, so I will
choose another constant for the test and send a v2.