* [PATCH v1 0/2] target/s390x: Fix emulation of C(G)HRL
@ 2023-03-06 20:55 Nina Schoetterl-Glausch
2023-03-06 20:55 ` [PATCH v1 1/2] " Nina Schoetterl-Glausch
2023-03-06 20:55 ` [PATCH v1 2/2] tests/tcg/s390x: Add C(G)HRL test Nina Schoetterl-Glausch
0 siblings, 2 replies; 5+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-03-06 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: Nina Schoetterl-Glausch, Richard Henderson, David Hildenbrand,
Ilya Leoshkevich, Thomas Huth, qemu-s390x
The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
halfword, it does not have the same size as the first operand.
Fix this and add a tcg test for c(g)hrl.
Nina Schoetterl-Glausch (2):
target/s390x: Fix emulation of C(G)HRL
tests/tcg/s390x: Add C(G)HRL test
target/s390x/tcg/insn-data.h.inc | 4 +-
target/s390x/tcg/translate.c | 7 +++
tests/tcg/s390x/chrl.c | 76 ++++++++++++++++++++++++++++++++
tests/tcg/s390x/Makefile.target | 1 +
4 files changed, 86 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/s390x/chrl.c
base-commit: a2b5f8b8ab7b2c947823088103a40f0ff11fe06b
--
2.37.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL
2023-03-06 20:55 [PATCH v1 0/2] target/s390x: Fix emulation of C(G)HRL Nina Schoetterl-Glausch
@ 2023-03-06 20:55 ` Nina Schoetterl-Glausch
2023-03-06 20:59 ` Richard Henderson
2023-03-06 20:55 ` [PATCH v1 2/2] tests/tcg/s390x: Add C(G)HRL test Nina Schoetterl-Glausch
1 sibling, 1 reply; 5+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-03-06 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: Nina Schoetterl-Glausch, Richard Henderson, David Hildenbrand,
Ilya Leoshkevich, Thomas Huth, qemu-s390x
The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
halfword, it does not have the same size as the first operand.
Fixes: a7e836d5eb ("target-s390: Convert COMPARE, COMPARE LOGICAL")
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
target/s390x/tcg/insn-data.h.inc | 4 ++--
target/s390x/tcg/translate.c | 7 +++++++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 9d2d35f084..6fe8ca5143 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -199,8 +199,8 @@
C(0xe55c, CHSI, SIL, GIE, m1_32s, i2, 0, 0, 0, cmps64)
C(0xe558, CGHSI, SIL, GIE, m1_64, i2, 0, 0, 0, cmps64)
/* COMPARE HALFWORD RELATIVE LONG */
- C(0xc605, CHRL, RIL_b, GIE, r1_o, mri2_32s, 0, 0, 0, cmps32)
- C(0xc604, CGHRL, RIL_b, GIE, r1_o, mri2_64, 0, 0, 0, cmps64)
+ C(0xc605, CHRL, RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps32)
+ C(0xc604, CGHRL, RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps64)
/* COMPARE HIGH */
C(0xb9cd, CHHR, RRE, HW, r1_sr32, r2_sr32, 0, 0, 0, cmps32)
C(0xb9dd, CHLR, RRE, HW, r1_sr32, r2_o, 0, 0, 0, cmps32)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 811049ea28..60b1254bb2 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5974,6 +5974,13 @@ static void in2_m2_64a(DisasContext *s, DisasOps *o)
#define SPEC_in2_m2_64a 0
#endif
+static void in2_mri2_16s(DisasContext *s, DisasOps *o)
+{
+ in2_ri2(s, o);
+ tcg_gen_qemu_ld16s(o->in2, o->in2, get_mem_index(s));
+}
+#define SPEC_in2_mri2_16s 0
+
static void in2_mri2_16u(DisasContext *s, DisasOps *o)
{
in2_ri2(s, o);
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 2/2] tests/tcg/s390x: Add C(G)HRL test
2023-03-06 20:55 [PATCH v1 0/2] target/s390x: Fix emulation of C(G)HRL Nina Schoetterl-Glausch
2023-03-06 20:55 ` [PATCH v1 1/2] " Nina Schoetterl-Glausch
@ 2023-03-06 20:55 ` Nina Schoetterl-Glausch
1 sibling, 0 replies; 5+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-03-06 20:55 UTC (permalink / raw)
To: qemu-devel
Cc: Nina Schoetterl-Glausch, Richard Henderson, David Hildenbrand,
Ilya Leoshkevich, Thomas Huth, qemu-s390x
Test COMPARE HALFWORD RELATIVE LONG instructions.
Test that the bytes following the second operand do not affect the
instruction.
Test the sign extension performed on the second operand.
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
I don't know what the coding style is for inline asm.
checkpatch.sh complains about the tabs inside the asm, which I find a
bit surprising given they're inside a string.
IMO emitting tabs makes sense in order to be consistent with gcc output.
I left the tabs in for now, but can remove them.
tests/tcg/s390x/chrl.c | 76 +++++++++++++++++++++++++++++++++
tests/tcg/s390x/Makefile.target | 1 +
2 files changed, 77 insertions(+)
create mode 100644 tests/tcg/s390x/chrl.c
diff --git a/tests/tcg/s390x/chrl.c b/tests/tcg/s390x/chrl.c
new file mode 100644
index 0000000000..fa33d43c59
--- /dev/null
+++ b/tests/tcg/s390x/chrl.c
@@ -0,0 +1,76 @@
+#include <stdlib.h>
+#include <assert.h>
+#include <stdint.h>
+
+static void test_chrl(void)
+{
+ uint32_t program_mask, cc;
+
+ asm volatile (
+ ".pushsection .rodata\n"
+ "0: .short 1,0x8000\n"
+ " .popsection\n"
+
+ " chrl %[r],0b\n"
+ " ipm %[program_mask]\n"
+ : [program_mask] "=r" (program_mask)
+ : [r] "r" (1)
+ );
+
+ cc = program_mask >> 28;
+ assert(!cc);
+
+ asm volatile (
+ ".pushsection .rodata\n"
+ "0: .short -1,0x8000\n"
+ " .popsection\n"
+
+ " chrl %[r],0b\n"
+ " ipm %[program_mask]\n"
+ : [program_mask] "=r" (program_mask)
+ : [r] "r" (-1)
+ );
+
+ cc = program_mask >> 28;
+ assert(!cc);
+}
+
+static void test_cghrl(void)
+{
+ uint32_t program_mask, cc;
+
+ asm volatile (
+ ".pushsection .rodata\n"
+ "0: .short 1,0x8000,0,0\n"
+ " .popsection\n"
+
+ " cghrl %[r],0b\n"
+ " ipm %[program_mask]\n"
+ : [program_mask] "=r" (program_mask)
+ : [r] "r" (1L)
+ );
+
+ cc = program_mask >> 28;
+ assert(!cc);
+
+ asm volatile (
+ ".pushsection .rodata\n"
+ "0: .short -1,0x8000,0,0\n"
+ " .popsection\n"
+
+ " cghrl %[r],0b\n"
+ " ipm %[program_mask]\n"
+ : [program_mask] "=r" (program_mask)
+ : [r] "r" (-1L)
+ );
+
+ cc = program_mask >> 28;
+ assert(!cc);
+}
+
+int main(void)
+{
+ test_chrl();
+ test_cghrl();
+ return EXIT_SUCCESS;
+}
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 72ad309b27..a3d3beeffe 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -28,6 +28,7 @@ TESTS+=div
TESTS+=clst
TESTS+=long-double
TESTS+=cdsg
+TESTS+=chrl
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL
2023-03-06 20:55 ` [PATCH v1 1/2] " Nina Schoetterl-Glausch
@ 2023-03-06 20:59 ` Richard Henderson
2023-03-06 21:33 ` Nina Schoetterl-Glausch
0 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2023-03-06 20:59 UTC (permalink / raw)
To: Nina Schoetterl-Glausch, qemu-devel
Cc: David Hildenbrand, Ilya Leoshkevich, Thomas Huth, qemu-s390x
On 3/6/23 12:55, Nina Schoetterl-Glausch wrote:
> The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
> halfword, it does not have the same size as the first operand.
>
> Fixes: a7e836d5eb ("target-s390: Convert COMPARE, COMPARE LOGICAL")
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
> target/s390x/tcg/insn-data.h.inc | 4 ++--
> target/s390x/tcg/translate.c | 7 +++++++
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
> index 9d2d35f084..6fe8ca5143 100644
> --- a/target/s390x/tcg/insn-data.h.inc
> +++ b/target/s390x/tcg/insn-data.h.inc
> @@ -199,8 +199,8 @@
> C(0xe55c, CHSI, SIL, GIE, m1_32s, i2, 0, 0, 0, cmps64)
> C(0xe558, CGHSI, SIL, GIE, m1_64, i2, 0, 0, 0, cmps64)
> /* COMPARE HALFWORD RELATIVE LONG */
> - C(0xc605, CHRL, RIL_b, GIE, r1_o, mri2_32s, 0, 0, 0, cmps32)
> - C(0xc604, CGHRL, RIL_b, GIE, r1_o, mri2_64, 0, 0, 0, cmps64)
> + C(0xc605, CHRL, RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps32)
> + C(0xc604, CGHRL, RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps64)
> /* COMPARE HIGH */
> C(0xb9cd, CHHR, RRE, HW, r1_sr32, r2_sr32, 0, 0, 0, cmps32)
> C(0xb9dd, CHLR, RRE, HW, r1_sr32, r2_o, 0, 0, 0, cmps32)
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 811049ea28..60b1254bb2 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -5974,6 +5974,13 @@ static void in2_m2_64a(DisasContext *s, DisasOps *o)
> #define SPEC_in2_m2_64a 0
> #endif
>
> +static void in2_mri2_16s(DisasContext *s, DisasOps *o)
> +{
> + in2_ri2(s, o);
> + tcg_gen_qemu_ld16s(o->in2, o->in2, get_mem_index(s));
> +}
> +#define SPEC_in2_mri2_16s 0
This is incorrect. See
https://patchew.org/QEMU/20230227054233.390271-1-richard.henderson@linaro.org/20230227054233.390271-51-richard.henderson@linaro.org/
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1 1/2] target/s390x: Fix emulation of C(G)HRL
2023-03-06 20:59 ` Richard Henderson
@ 2023-03-06 21:33 ` Nina Schoetterl-Glausch
0 siblings, 0 replies; 5+ messages in thread
From: Nina Schoetterl-Glausch @ 2023-03-06 21:33 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: David Hildenbrand, Ilya Leoshkevich, Thomas Huth, qemu-s390x
On Mon, 2023-03-06 at 12:59 -0800, Richard Henderson wrote:
> On 3/6/23 12:55, Nina Schoetterl-Glausch wrote:
> > The second operand of COMPARE HALFWORD RELATIVE LONG is a signed
> > halfword, it does not have the same size as the first operand.
> >
> > Fixes: a7e836d5eb ("target-s390: Convert COMPARE, COMPARE LOGICAL")
> > Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> > ---
> > target/s390x/tcg/insn-data.h.inc | 4 ++--
> > target/s390x/tcg/translate.c | 7 +++++++
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
> > index 9d2d35f084..6fe8ca5143 100644
> > --- a/target/s390x/tcg/insn-data.h.inc
> > +++ b/target/s390x/tcg/insn-data.h.inc
> > @@ -199,8 +199,8 @@
> > C(0xe55c, CHSI, SIL, GIE, m1_32s, i2, 0, 0, 0, cmps64)
> > C(0xe558, CGHSI, SIL, GIE, m1_64, i2, 0, 0, 0, cmps64)
> > /* COMPARE HALFWORD RELATIVE LONG */
> > - C(0xc605, CHRL, RIL_b, GIE, r1_o, mri2_32s, 0, 0, 0, cmps32)
> > - C(0xc604, CGHRL, RIL_b, GIE, r1_o, mri2_64, 0, 0, 0, cmps64)
> > + C(0xc605, CHRL, RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps32)
> > + C(0xc604, CGHRL, RIL_b, GIE, r1_o, mri2_16s, 0, 0, 0, cmps64)
> > /* COMPARE HIGH */
> > C(0xb9cd, CHHR, RRE, HW, r1_sr32, r2_sr32, 0, 0, 0, cmps32)
> > C(0xb9dd, CHLR, RRE, HW, r1_sr32, r2_o, 0, 0, 0, cmps32)
> > diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> > index 811049ea28..60b1254bb2 100644
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -5974,6 +5974,13 @@ static void in2_m2_64a(DisasContext *s, DisasOps *o)
> > #define SPEC_in2_m2_64a 0
> > #endif
> >
> > +static void in2_mri2_16s(DisasContext *s, DisasOps *o)
> > +{
> > + in2_ri2(s, o);
> > + tcg_gen_qemu_ld16s(o->in2, o->in2, get_mem_index(s));
> > +}
> > +#define SPEC_in2_mri2_16s 0
>
> This is incorrect. See
>
> https://patchew.org/QEMU/20230227054233.390271-1-richard.henderson@linaro.org/20230227054233.390271-51-richard.henderson@linaro.org/
>
Ah, that got merged yesterday, didn't it?
base-commit: a2b5f8b8ab7b2c947823088103a40f0ff11fe06b
which is a bit older, I'll rebase.
>
> r~
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-06 21:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-06 20:55 [PATCH v1 0/2] target/s390x: Fix emulation of C(G)HRL Nina Schoetterl-Glausch
2023-03-06 20:55 ` [PATCH v1 1/2] " Nina Schoetterl-Glausch
2023-03-06 20:59 ` Richard Henderson
2023-03-06 21:33 ` Nina Schoetterl-Glausch
2023-03-06 20:55 ` [PATCH v1 2/2] tests/tcg/s390x: Add C(G)HRL test Nina Schoetterl-Glausch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.