Re: [RFC PATCH 4/6] tests/tcg: add support for ppc64le softmmu tests

[Leandro Lupori]

On 3/25/22 06:50, Alex Bennée wrote:

> Leandro Lupori <leandro.lupori@eldorado.org.br> writes:
>
> > On 24/03/2022 17:34, Alex Bennée wrote:
> > > Leandro Lupori <leandro.lupori@eldorado.org.br> writes:
> > >
> > > > Adding a new, "virtual" TCG test target, ppc64le-softmmu, seems to
> > > > be the cleanest way to support both BE and LE tests for
> > > > ppc64-softmmu.
> > > >
> > > > Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> > > > ---
> > > >    tests/Makefile.include                    |  7 ++++---
> > > >    tests/tcg/configure.sh                    | 11 ++++++++++-
> > > >    tests/tcg/ppc64/Makefile.softmmu-target   |  2 ++
> > > >    tests/tcg/ppc64le/Makefile.softmmu-target |  7 +++++++
>
> Don't forget to add new files to MAINTAINERS by the way ;-)

Right.

> > > >    4 files changed, 23 insertions(+), 4 deletions(-)
> > > >    create mode 100644 tests/tcg/ppc64le/Makefile.softmmu-target
> > > >
> > > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > > index e7153c8e91..4001fedbc3 100644
> > > > --- a/tests/Makefile.include
> > > > +++ b/tests/Makefile.include
> > > > @@ -40,9 +40,10 @@ SPEED = quick
> > > >    TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
> > > >
> > > >    # Per guest TCG tests
> > > > -BUILD_TCG_TARGET_RULES=$(patsubst %,build-tcg-tests-%, $(TARGETS))
> > > > -CLEAN_TCG_TARGET_RULES=$(patsubst %,clean-tcg-tests-%, $(TARGETS))
> > > > -RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, $(TARGETS))
> > > > +TCG_TARGETS=$(patsubst tests/tcg/config-%.mak, %, $(wildcard 
> > > > tests/tcg/config-*.mak))
> > > > +BUILD_TCG_TARGET_RULES=$(patsubst %,build-tcg-tests-%, $(TCG_TARGETS))
> > > > +CLEAN_TCG_TARGET_RULES=$(patsubst %,clean-tcg-tests-%, $(TCG_TARGETS))
> > > > +RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, $(TCG_TARGETS))
> > > I'm not following what is going on here. Are we creating a new
> > > target
> > > type? Is this just to avoid duplication in tests/tcg subdirs?
> > Yes, together with the change in test/tcg/configure.sh, a new
> > ppc64le-softmmu target is created, in the context of TCG tests only.
> > But it isn't just to avoid duplication in tests/tcg subdirs.
> >
> > Without a ppc64le-softmmu target, the tcg tests' makefiles will only
> > include tests/tcg/ppc64/Makefile.softmmu-target file. They won't try
> > to include tests/tcg/ppc64le/Makefile.softmmu-target, because there is
> > no ppc64le-softmmu target.
>
> So according to IRC this is because the ppc64-softmmu target can support
> dynamically switching between BE/LE modes so there is only needs to be
> one 64 bit ppc system binary.
>
> > I've actually tried to do everything in
> > tests/tcg/ppc64/Makefile.softmmu-target. But when it is included,
> > everything is already setup to build for ppc64 (BE), such as CC,
> > EXTRA_CFLAGS and other variables. So it seems that, to be able to also
> > build and run the same tests for ppc64le, I would need to somehow
> > change CC, EXTRA_CFLAGS, etc, to setup them for a ppc64le build and
> > write another set of rules for the LE tests. Then I would also need to
> > handle output file conflicts, to be able have both BE and LE binaries
> > coexisting in the same ppc64-softmmu output directory.
>
> There is another approach you can take which is to generate alternative
> binaries from the same sources in the build. For example we build the
> sha512 test with a couple of different compiler options and run with
> slightly different QEMU_OPTS:
>
>    sha512-vector: CFLAGS +=-mcpu=power10 -O3
>    sha512-vector: sha512.c
>            $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
>
>    run-sha512-vector: QEMU_OPTS+=-cpu POWER10
>    run-plugin-sha512-vector-with-%: QEMU_OPTS+=-cpu POWER10
>
>    PPC64LE_TESTS += sha512-vector
>
> So you could do something similar for le versions of the tests.
> > I'm ambivalent to which makes the best approach. I only worry the
> "pseudo target" approach might break something else down the line.
> However as long as the ppc maintainers are happy with the tests you can
> have my:
>
> Acked-by: Alex Bennée <alex.bennee@linaro.org>
>
> for the check-tcg plumbing changes.

Ok, this approach worked too. It ended up being a bit more complex, 
mainly because LE versions of CRT and MINILIB objects must be used, but 
it should be ok, if it helps to avoid breaking something else.
I'll send a V2 with this new approach.

Thanks,
Leandro