Re: [PATCH] sim: testsuite: push $arch out to targets

[Mike Frysinger <vapier@gentoo.org>]

On 31 Jan 2021 10:54, Andrew Burgess wrote:
> * Mike Frysinger <vapier@gentoo.org> [2021-01-22 01:36:05 -0500]:
> > On 21 Jan 2021 09:22, Andrew Burgess wrote:
> > > * Mike Frysinger <vapier@gentoo.org> [2021-01-18 13:01:53 -0500]:
> > > > On 18 Jan 2021 09:52, Andrew Burgess wrote:
> > > > > * Mike Frysinger [2021-01-17 11:09:45 -0500]:
> > > > > > This is needed to move to automake & its dejagnu-provided logic,
> > > > > > and eventually by the unified sim logic.
> > > > > 
> > > > > I looked through this patch and I didn't understand what's going on
> > > > > here.
> > > > > 
> > > > > If this needs doing at all, could it not be done in some global location?
> > > > 
> > > > the sim ports use a unique subdir for their `run` program.  the tests
> > > > need to find that path.  this $arch value is what binds the specific
> > > > subdir to the test.
> > > 
> > > I don't understand that paragraph I'm afraid.
> > > 
> > > I guess my question is each *.exp file gets invoked by dejagnu, but
> > > there are other hooks that dejagnu calls, like the ${tool}_init proc.
> > > Could we not use that to set these things instead?  It seems like
> > > there's a 1:1 mapping between [istarget ????] patterns and the values
> > > pushed into both arch and all_machs.
> > 
> > i'm not seeing how any hooks would help.  the targets need to know where
> > their sim program lives.  let me brain dump on you :p.
> > 
> > lets look at a bfin & frv build tree.
> > build-bfin-elf/
> > `-- sim/
> >     `-- bfin/
> >         `-- run
> > build-frv-elf/
> > `-- sim/
> >     `-- frv/
> >         `-- run
> > 
> > sim/testsuite/frv/*.exp files need to know to look for $(builddir)/frv/run
> > while sim/testsuite/bfin/*.exp files need $(builddir)/bfin/run.
> > 
> > i know the variable is called $arch and that can be confusing.  i could
> > rename it if it helps.  just keep in mind that this is used for exactly
> > one thing: where to find `run` under $(builddir)/.
> > 
> > when we invoke dejagnu, it was with --tool=sim.  if we had combined trees
> > with other tools (gas/gdb/etc...), this would make sense, but it doesn't,
> > so we changed it to --tool="".  now when you run `runtest`, it will find
> > all *.exp files under sim/testsuite/ and attempt to execute them.
> 
> OK.  I don't understand why changing tool from "sim" to "" is either
> necessary, or a good thing, but, whatever....

i covered that here:
https://sourceware.org/pipermail/gdb/2021-January/049098.html
https://sourceware.org/pipermail/gdb-patches/2021-January/175115.html

> For gdb we have a 'gdb_init' proc, which is called based on
> '${tool}_init' from within GDB.  Now that we have no tool set does
> this prevent us having an 'init' proc?

it does not mean that.  we have `sim_init` in lib/sim-defs.exp and it is
still called via config/default.exp.

(i'll delete the part of your email below that talks about not having it)

> Lets assume for one moment that we _did_ have an init proc.  Could
> that proc not just have one huge [istarget ???] { set arch ... } tree
> that handled all known istarget patterns?

we could do that.  i don't think that's better than what i'm proposing
with setting the arch in the <arch>/ subdirs.  it seems slightly worse
because the [istarget ???] patterns that we use in the <arch>/ subdirs
need to be replicated -- some have a 1-to-many map of arch-to-targets.
although we can prob have the centralized logic use target mappings that
capture more than necessary as long as it's still unique between each
other.  e.g. we can glob cris* even though the cris port itself uses
cris-*-* & crisv32-*-*.

imo, this is the wrong direction: anytime we have a centralized place
where we have arch-specific logic, we're doing it wrong.  we already
have a centralized list today that i want to move away from.  the
sim/configure.tgt is setting up $arch based on the target.  but it
doesn't work when doing multi-target (--enable-targets=all) because
there isn't a single port testsuite to bind to, hence it needs a new
home.

> I think you say that right now we have pretty much one test script for
> each arch, but there's no reason which this has to be the case
> forever, right?

correct.  but i don't see how that's necessarily relevant to this thread.
even if we were to fragment the arch .exp files (somewhat akin to what
gdb does with some of its testsuites), we would still be able to have
each port share a common file (e.g. testsuite/bfin/common.exp) where we
could put the arch-specific settings.

on a related note, imo, dejagnu is a dead end.  Ian wrote a good post:
https://www.airs.com/blog/archives/499

but dejagnu is even more unnecessary for the sim as we have no need for
any of the target hooks.  e.g. the ability for people to say "in order
to execute this program, copy the file to this remote system, then ssh
over to it, and run it like so".  those are useful to gas, gdb, etc...
but would never be used by the sim.  so really we should be looking at
throwing out dejagnu entirely from the sim and switching to something
like automake's builtin test runners.  we use this in some projects in
the tree already when the test does not require any target execution.

> It just seems weird to me to say that each test is required to start
> with a highly predictable bit of boiler plate...

i don't disagree.  but to be clear, i don't see any of this as the end
state long term.  this is all intermediate steps for a better future.
sometimes that process means taking on a little tech debt to unblock
the next step, and then slowly collapsing that back as the common code
improves.  so here we're taking a configure-time constant that locks
the testsuite to a single specific target and moving it to the tests.
then as we unify the sim ports into a single build, we'd delete the
setting from their testsuites in the process.
-mike