[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] tests/tcg/s390x: Add ex-relative-long.c
|
From: |
Nina Schoetterl-Glausch |
|
Subject: |
Re: [PATCH v2 2/2] tests/tcg/s390x: Add ex-relative-long.c |
|
Date: |
Thu, 16 Mar 2023 18:50:00 +0100 |
|
User-agent: |
Evolution 3.46.4 (3.46.4-1.fc37) |
On Wed, 2023-03-15 at 01:11 +0100, Ilya Leoshkevich wrote:
> > Test EXECUTE and EXECUTE RELATIVE LONG with relative long instructions
> > as targets.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Some comments below.
> > ---
> > tests/tcg/s390x/Makefile.target | 1 +
> > tests/tcg/s390x/ex-relative-long.c | 159 +++++++++++++++++++++++++++++
> > 2 files changed, 160 insertions(+)
> > create mode 100644 tests/tcg/s390x/ex-relative-long.c
> >
> > diff --git a/tests/tcg/s390x/Makefile.target
> > b/tests/tcg/s390x/Makefile.target
> > index cf93b966862..90bc48227db 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+=ex-relative-long
> >
> > cdsg: CFLAGS+=-pthread
> > cdsg: LDFLAGS+=-pthread
> > diff --git a/tests/tcg/s390x/ex-relative-long.c
> > b/tests/tcg/s390x/ex-relative-long.c
> > new file mode 100644
> > index 00000000000..4caa8c1b962
> > --- /dev/null
> > +++ b/tests/tcg/s390x/ex-relative-long.c
> > @@ -0,0 +1,159 @@
> > +/* Check EXECUTE with relative long instructions as targets. */
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +
> > +struct test {
> > + const char *name;
> > + long (*func)(long reg, long *cc);
> > + long exp_reg;
> > + long exp_mem;
> > + long exp_cc;
> > +};
> > +
> > +/*
> > + * Each test sets the MEM_IDXth element of the mem array to MEM and uses a
> > + * single relative long instruction on it. The other elements remain zero.
> > + * This is in order to prevent stumbling upon MEM in random memory in case
> > + * there is an off-by-a-small-value bug.
> > + *
> > + * Note that while gcc supports the ZL constraint for relative long
> > operands,
> > + * clang doesn't, so the assembly code accesses mem[MEM_IDX] using MEM_ASM.
> > + */
> > +long mem[0x1000];
This could be static, no?
> > +#define MEM_IDX 0x800
> > +#define MEM_ASM "mem+0x800*8"
> > +
> > +/* Initial %r2 value. */
> > +#define REG 0x1234567887654321
> > +
> > +/* Initial mem[MEM_IDX] value. */
> > +#define MEM 0xfedcba9889abcdef
> > +
> > +/* Initial cc value. */
> > +#define CC 0
> > +
> > +/* Relative long instructions and their expected effects. */
> > +#define FOR_EACH_INSN(F)
> > \
You could define some macros and then calculate a bunch of values in the table,
i.e.
#define SL(v) ((long)(v))
#define UL(v) ((unsigned long)(v))
#define SI(v, i) ((int)(v >> ((1 - i) * 32)))
#define UI(v, i) ((unsigned int)(v >> ((1 - i) * 32)))
#define SH(v, i) ((short)(v >> ((3 - i) * 16)))
#define UH(v, i) ((unsigned short)(v >> ((3 - i) * 16)))
#define CMP(f, s) ((f) == (s) ? 0 : ((f) < (s) ? 1 : 2 ))
F(cgfrl, REG, MEM, CMP(SL(REG), SI(MEM, 0))
But everything checks out, so no need.
> > + F(cgfrl, REG, MEM, 2)
> > \
> > + F(cghrl, REG, MEM, 2)
> > \
> > + F(cgrl, REG, MEM, 2)
> > \
> > + F(chrl, REG, MEM, 1)
> > \
> > + F(clgfrl, REG, MEM, 2)
> > \
> > + F(clghrl, REG, MEM, 2)
> > \
> > + F(clgrl, REG, MEM, 1)
> > \
> > + F(clhrl, REG, MEM, 2)
> > \
> > + F(clrl, REG, MEM, 1)
> > \
> > + F(crl, REG, MEM, 1)
> > \
> > + F(larl, (long)&mem[MEM_IDX], MEM, CC)
> > \
> > + F(lgfrl, 0xfffffffffedcba98, MEM, CC)
> > \
> > + F(lghrl, 0xfffffffffffffedc, MEM, CC)
> > \
> > + F(lgrl, MEM, MEM, CC)
> > \
> > + F(lhrl, 0x12345678fffffedc, MEM, CC)
> > \
> > + F(llghrl, 0x000000000000fedc, MEM, CC)
> > \
> > + F(llhrl, 0x123456780000fedc, MEM, CC)
> > \
> > + F(lrl, 0x12345678fedcba98, MEM, CC)
> > \
> > + F(stgrl, REG, REG, CC)
> > \
> > + F(sthrl, REG, 0x4321ba9889abcdef, CC)
> > \
> > + F(strl, REG, 0x8765432189abcdef, CC)
> > +
> > +/* Test functions. */
> > +#define DEFINE_EX_TEST(insn, exp_reg, exp_mem, exp_cc)
> > \
> > + static long test_ex_ ## insn(long reg, long *cc)
> > \
> > + {
> > \
> > + register long reg_val asm("r2");
> > \
> > + long cc_val, mask, target;
> > \
> > +
> > \
> > + reg_val = reg;
> > \
> > + asm("xgr %[cc_val],%[cc_val]\n" /* initial cc */
> > \
This confused me for a bit because it's not about cc_val at all.
Maybe do "cr %%r0,%%r0\n" instead?
Could also push it down before the ex, since that's where the change we care
about
takes place.
> > + "lghi %[mask],0x20\n" /* make target use %r2 */
> > \
> > + "larl %[target],0f\n"
> > \
> > + "ex %[mask],0(%[target])\n"
> > \
> > + "jg 1f\n"
> > \
> > + "0: " #insn " %%r0," MEM_ASM "\n"
> > \
> > + "1: ipm %[cc_val]\n"
> > \
> > + : [cc_val] "=&r" (cc_val)
> > \
> > + , [mask] "=&r" (mask)
> > \
> > + , [target] "=&r" (target)
> > \
> > + , [reg_val] "+&r" (reg_val)
> > \
> > + : : "cc", "memory");
> > \
> > + reg = reg_val;
> > \
What is the point of this assignment?
> > + *cc = (cc_val >> 28) & 3;
> > \
> > +
> > \
> > + return reg_val;
> > \
> > + }
> > +
> > +#define DEFINE_EXRL_TEST(insn, exp_reg, exp_mem, exp_cc)
> > \
> > + static long test_exrl_ ## insn(long reg, long *cc)
> > \
> > + {
> > \
> > + register long reg_val asm("r2");
> > \
> > + long cc_val, mask;
> > \
> > +
> > \
> > + reg_val = reg;
> > \
> > + asm("xgr %[cc_val],%[cc_val]\n" /* initial cc */
> > \
> > + "lghi %[mask],0x20\n" /* make target use %r2 */
> > \
> > + "exrl %[mask],0f\n"
> > \
> > + "jg 1f\n"
> > \
> > + "0: " #insn " %%r0," MEM_ASM "\n"
> > \
> > + "1: ipm %[cc_val]\n"
> > \
> > + : [cc_val] "=&r" (cc_val)
> > \
> > + , [mask] "=&r" (mask)
> > \
> > + , [reg_val] "+&r" (reg_val)
> > \
> > + : : "cc", "memory");
> > \
> > + reg = reg_val;
> > \
> > + *cc = (cc_val >> 28) & 3;
> > \
> > +
> > \
> > + return reg_val;
> > \
> > + }
> > +
> > +FOR_EACH_INSN(DEFINE_EX_TEST)
> > +FOR_EACH_INSN(DEFINE_EXRL_TEST)
> > +
> > +/* Test definitions. */
> > +#define REGISTER_EX_EXRL_TEST(ex_insn, insn, _exp_reg, _exp_mem, _exp_cc)
> > \
> > + {
> > \
> > + .name = #ex_insn " " #insn,
> > \
> > + .func = test_ ## ex_insn ## _ ## insn,
> > \
> > + .exp_reg = (long)(_exp_reg),
> > \
> > + .exp_mem = (long)(_exp_mem),
> > \
> > + .exp_cc = (long)(_exp_cc),
> > \
You don't need the casts, do you?
> > + },
> > +
> > +#define REGISTER_EX_TEST(insn, exp_reg, exp_mem, exp_cc)
> > \
> > + REGISTER_EX_EXRL_TEST(ex, insn, exp_reg, exp_mem, exp_cc)
> > +
> > +#define REGISTER_EXRL_TEST(insn, exp_reg, exp_mem, exp_cc)
> > \
> > + REGISTER_EX_EXRL_TEST(exrl, insn, exp_reg, exp_mem, exp_cc)
> > +
> > +static const struct test tests[] = {
> > + FOR_EACH_INSN(REGISTER_EX_TEST)
> > + FOR_EACH_INSN(REGISTER_EXRL_TEST)
> > +};
> > +
> > +/* Loop over all tests and run them. */
> > +int main(void)
> > +{
> > + const struct test *test;
> > + int ret = EXIT_SUCCESS;
> > + long reg, cc;
> > + size_t i;
> > +
> > + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) {
> > + test = &tests[i];
> > + mem[MEM_IDX] = MEM;
> > + cc = -1;
> > + reg = test->func(REG, &cc);
> > +#define ASSERT_EQ(expected, actual) do {
> > \
> > + if (expected != actual) {
> > \
> > + fprintf(stderr, "%s: " #expected " (0x%lx) != " #actual "
> > (0x%lx)\n", \
> > + test->name, expected, actual);
> > \
> > + ret = EXIT_FAILURE;
> > \
> > + }
> > \
> > +} while (0)
> > + ASSERT_EQ(test->exp_reg, reg);
> > + ASSERT_EQ(test->exp_mem, mem[MEM_IDX]);
> > + ASSERT_EQ(test->exp_cc, cc);
> > +#undef ASSERT_EQ
> > + }
> > +
> > + return ret;
> > +}