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

Andrew Burgess
2021-02-08T12:12:56Z

* Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org> [2021-01-31 14:41:17 -0500]:

> 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.

Thanks for the explanation.

I don't really understand what your end vision is.

I don't agree that this is the right intermediate state to move to.
Sure, sometimes things have to get worse before they can get better,
but this feels like an even stronger argument for putting all of the
"worse" into a single central location, rather than replicating it all
over the place.

** BUT **

Given you clearly have more time for sim/ right now I think you should
just commit this and move on, I suspect we're never going to agree on
this one, and as its just an opinion thing I don't see the point in
arguing it any further.

If this ever causes problems in the future we just revisit this
conversation at that point :)

Thanks for taking to time to explain your position, I do appreciate
it.

Thanks,
Andrew