* [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
* 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
* [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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).