Re: sim/mips patch: add support for more NEC VR targets

cgd
2002-10-22T22:27:24Z

At 09 Oct 2002 15:14:43 +0100, Richard Sandiford wrote:
> Thanks for the review.

That's what I'm here for. 8-) (Sorry for taking so long to get to
this, but, as i mentioned in private mail before I left, I was out of
the office. 8-)

> Good catch. I checked that vr5000 implied mipsIV, but not the
> other way around (duh!).

(I dunno whether you were using the horrible compare_igen_models
script or not, but it does both ways for you and is ... much easier
than hand inspection. 8-)

> Yeah. I don't understand why, really, since the vr41xx processors don't
> have an FPU. Maybe the target environment was assumed to provide an
> emulator or something? But key instructions (like bc1f and bc1t ;-)
> are missing, so very little FP code is going to work.

I don't know, the code predates my time here. 8-)

FWIW, my personal preference is to have a simulator which is good
enough to act like real hardware, then, if necessary and appropriate
(i.e., trying to simulate a firmware or other SW environment which has
an emulator) do actual emulation.

(One implication of this is that a 'good' emulation for a particular
processor might well trap on certain FP cases unimplemented in HW.)

There are both plusses and minuses to this approach, of course.

> > There are a couple of specific things that would be nice if you could
> > arrange in follow-on patches, in addition to what you describe above:
> >
> > * it would be good if things like vr_run.c could be automatically
> > generated.
>
> OK. Guilt at my earlier lameness forced me to give this a go.
> New version attached. Not pretty (and I didn't use awk ;-) but
> it should reduce the amount of cut&paste if other targets use
> MULTI. I've also removed the need to define sim_multi_filter
> and sim_multi_machine, since igen can cope with the same machine
> & filter being given twice.

Heh. (no awk?! what will we do?!)

> > There is an annoying interaction between multi-arching a sim and using
> > STATE_ARCHITECTURE (SD)->mach: e.g. if you have the thing above where
> > you check bfd_mach_mips5500, if mips5500 is the 'default' for your sim
> > (i.e., catches unknown machine types), that check will fail.
> >
> > Really, we need to be sure to check and, if necessary, set 'mach' to a
> > default value when it is set.
>
> Where do you think this should happen? In sim_engine_run()?

To be honest, I'm not really sure. However, I do believe that that is
too late. 8-)

(The problem is that simulator machine state initialization has
happened by the time you invoke sim_engine_run(), and that may have to
differe depending on the type of machine you're simulating.)

Ideally what you want is a replacement for

STATE_ARCHITECTURE (sd) ? STATE_ARCHITECTURE (sd)->mach : 0

that does the correct default handling, but is very very fast. Alas,
I don't know that that's possible. 8-)

Probably the right thing is to just turn it into a function call if
doing MULTI, else have it #defined to the correct specific value if
not MULTI (i.e., a single-arch sim).

People running the GCC test suite in a MULTI sim don't care if they're
getting 200k insns/sec or 1m insns/sec... 8-)

On the other hand, people who want a really fast sim will build one
that targets a single CPU, which will get the #define which also
allows additional optimizations (code to be elided). (Having been in
the position of booting firmware under a sim, then booting OSes under
said firmware, I can say that I like the fastest sim possible
sometimes. 8-)

Thoughts?

> + # The MULTI generator can combine several simulation engines into
> + # one executable. A configuration which uses the MULTI should
> + # set ${sim_multi_arch_configs} to the list of engines to build.
> + # Each space-separated entry has the form NAME:MACHINE:FILTER@ARCHS,
> + # where:
> + #
> + # - NAME is a C-compatible prefix for the engine,
> + # - MACHINE is a -M argument,
> + # - FILTER is a -F argument, and
> + # - ARCHS is a comma-separated list of bfd machines that the
> + # simulator can run.
> + #
> + # Each entry will have a separate simulation engine whose prefix is
> + # m32<NAME>. If the machine list includes "mips16", there will also
> + # be a mips16 engine, prefix m16<NAME>. The mips16 engine will be
> + # generated using the same machine list as the 32-bit version,
> + # but the filter will be "16" instead of FILTER.
> + #
> + # The simulator compares the bfd mach against ARCHS to decide
> + # which engine to use. Entries in ARCHS can be a bfd_mach_mips*
> + # value with "bfd_mach_mips" removed, or the special string
> + # "default" to indicate that a particular engine should be
> + # the default.

I believe there _must_ be one default entry, and this should be
checked for in the code too. What do you think?

Why use "@" rather than another :... makes the seds slightly easier,
that it?

Suggest either:

* archs have only bfd_mach_ prepended i.e., mips4111, mips_sb1, etc.
The current method is ... not so pretty for archs whose bfd define
isn't a number. This makes the specifictions a bit longer, but IMO
that's not biggie.

* more special casing, e.g.:

[0-9]* add bfd_mach_mips
[a-z]* add bfd_mach_mips_

8-) This could get out of hand, though.

(personally, my feeling is that adding even more defines of the form
bfd_mach_mipsNNNN is ... not the right thing. It should be
bfd_mach_mips_vendnum
^^^^^^^ vendor letter/number combo, e.g. sb1, vrNNNN
^^^^ ARCHITECTURE!!!
or whatever... but that's water under the bridge.)

> + if test ${sim_gen} = MULTI; then
> + rm -f multi-include.h multi-switch.c
> + sim_multi_flags=
> + sim_multi_obj=multi_run.o
> + for fc in ${sim_multi_arch_configs}; do
> + c=`echo ${fc} | sed 's/@.*//'`
> + archs=`echo ${fc} | sed 's/.*@//'`
> + name=`echo ${c} | sed 's/:.*//'`
> + m=`echo ${c} | sed 's/.*:\(.*\):.*/\1/'`
> + f=`echo ${c} | sed 's/.*://'`
> + sim_multi_flags="${sim_multi_flags} -F ${f} -M ${m}"
> + case $c in
> + *:*mips16*:*)
> + ws="m32 m16"
> + sim_multi_src="${sim_multi_src} m16${name}_run.c"
> + sim_multi_obj="${sim_multi_obj} m16${name}_run.o"
> + sim_multi_flags="${sim_multi_flags} -F 16"
> + ;;
> + *)
> + ws=m32
> + ;;
> + esac
> + for w in ${ws}; do
> + for base in engine icache idecode model semantics support; do
> + sim_multi_src="${sim_multi_src} ${w}${name}_${base}.c"
> + sim_multi_src="${sim_multi_src} ${w}${name}_${base}.h"
> + sim_multi_obj="${sim_multi_obj} ${w}${name}_${base}.o"
> + done
> + sim_multi_configs="${sim_multi_configs} ${w}${c}"
> + done
> + echo "#include \"${w}${name}_engine.h\"" >> multi-include.h
> + for a in `echo $archs | sed 's/,/ /g'`; do
> + case $a in
> + default) echo "default:" >> multi-switch.c ;;
> + *) echo "case bfd_mach_mips$a:" >> multi-switch.c ;;
> + esac
> + done
> + echo " ${w}${name}_engine_run (sd, next_cpu_nr, nr_cpus, signal);" \
> + >> multi-switch.c
> + echo " break;" >> multi-switch.c
> + done
> + else
> + # For clean-extra
> + sim_multi_src=doesnt-exist.c
> + fi
> sim_igen_flags="-F ${sim_igen_filter} ${sim_igen_machine} ${sim_igen_smp}"
> sim_m16_flags=" -F ${sim_m16_filter} ${sim_m16_machine} ${sim_igen_smp}"
> AC_SUBST(sim_igen_flags)
> AC_SUBST(sim_m16_flags)
> AC_SUBST(sim_gen)
> + AC_SUBST(sim_multi_flags)
> + AC_SUBST(sim_multi_configs)
> + AC_SUBST(sim_multi_src)
> + AC_SUBST(sim_multi_obj)

err, shouldn't multi-switch.c be cleaned somehow (during distclean, i
guess, since it's config-generated).

Also, shouldn't multi-run.o depend on multi-switch.c so that the right
thing happens after re-config?

(obviously, if there's a fn to pick the right machine type, the case
statement in the engine generated above doesn't need to include the
default, that fn does.)

chris