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

cgd
2002-11-07T19:08:47Z

At Wed, 06 Nov 2002 20:40:45 -0500, Andrew Cagney wrote:
> > well:
> >
> > (1) w/ separate fn, you'd need to tag _all_ of the 5500 machine insns
> > w/ mips5500. (as opposed to ISA + a few mips5500 extensions.)
>
> Er, all the instructions that belonged to the MIPS 5500 should be taged
> with 5500 anyway. As I noted:
>
> > The real world order is: ISA YYY implements MIPS XXX, BUT with a few tiny exceptions .... You end up having to check that every single *&@^#$(*&@#$ instruction matches the generic ISA. Sigh.

So, i'd like to explore this a bit more. I'll state up front I don't
agree, so you know where i'm going before you're hip-deep in spew. 8-)

I hope this clears up where i'm trying to take things. I think you
knew when you signed me on as co-maintainer that I was likely to have
strong opinions. 8-)

My belief is the that following things, at least, should be considered
when trying to figure out how to support a new ISA, in no particular
order:

* clarity of code.

E.g., if it's MIPS32 with a few small deviations or
something, IMO it's _much_ clearer to code that using mips32
and then test for the machine to handle the diffferences,
than it is to scatter machine_name tags all over and make
people use compare_igen_models to tell the difference.

Obviously, there's a tradeoff: if there are enough if()
checks, then the code quality suffers.

* performance.

Being able to tune a simulator so that it performs well is
important to me, and to serious simulator users.

Like i've said before, we've simulated ... a whole heck of a
lot of instructions (i believe probably "trillions") using a
sim-based simulator, and we prefer that things run as fast
as possible. 8-)

Because there's a desire to support multi-machine
simulators, there's a tradeoff here, too.

To my mind, people who want multi-machine simulators are
willing to sacrifice some performance to do it (but do not
want to lose horribly, either). A few scattered run-time
checks will not significantly harm performance (and heck,
they've already bought a runtime check in sim_engine_run at
minimum).

On the other hand, the ability to provide multiple machine
support should minimally impact single machine support.
Once consequence of that is, when building a single machine
sim, as many run-time tests as possible should be able to be
removed at compile time (e.g. due to constant comparison
results).

* correctness.

By this i mean "matches the actual hardware," and not
"produces computationally-correct results given what the
author of the code was trying to simulatr." The latter is
not something which is negotiable. 8-)

Obviously, the simulator has to be "as close as necessary"
for the author's and the community's use.

There are differing degrees as to what's required there.
For instace, our internal simulator has a full and complete
cp0 implementation, good enough to run OSes and other "hard"
tests of the MIPS architecture.

Many people don't need that, or in fact don't even _want_
it.

For instance, our CP1 simulation actually does trap on the
same inputs that our real FPU does (denorms for certain ops,
or whatever -- i forget) and it's important to us that it
does (when running in "raw hardware" mode). That's
desirable in certain circumstances, but not in many
(e.g. running the GCC testsuite on simulated firmware),
since it'll cause some tests to fail that would normally
pass after OS or firmware fixups.

So, anyway, to get on to the specifics of how to add new machines,
with the choices being:

* leave 'shared' instructions tagged w/ the base ISA type
implemented by the machine (with additional tags for ASEs,
etc.) and use run-time tests to differentiate as necessary
in 'common' instructions

* tag each instruction as being specific to that machine,
differentiating by providing different functions or by
instructions.

* In the case of MIPS32/MIPS64 implementations, i strongly believe
that the former ("the new world order" as i see it) is the right
thing.

there are three basic types of deviations which require
conditionalization:

* cp0 stuff.

IMO, this pretty much _has_ to be done by checking the
machine type in C code.

If you try to do it in igen code, you need IGEN
functions per machine type, then they need to call out
to per-machine-type functions in .c files. It
unnecessarily blows up the size of the code.

This is especially true for MIPS32/MIPS64 CP0, which
_should_ be CP0 as specified, with possibly a tiny bit
of deviation per machine either for allowed customizations
(e.g. VA size, subsetting, etc.) or for defects.

Trying to do M32/M64 CP0 on a per-machine basis would
just be crazy.

* added instructions

There are easily enough handled as "ASE"-ish
additional machine type specifiers.

* bugs.

This should, IMO, be handled on a case-by-case basis.

If a machine has a few of them, they should probably
be handled with if()s on the machine type.

If it's got a _lot_, they should probably be handled
by not using the generic mips32/mips64 instructions,
and littering the new machine's name all over as need
be.

* Pre-MIPS32/MIPS64 is a bit trickier, because in addition to the
above cases where people are implementing on top of a standard ISA,
you run into stuff like:

* vastly diverent CP0 implementations.

* instructions _removed_ from a base ISA (e.g. no ll/sc), or
instructions with same encoding but different behaviour.

* divergent requirements re: hazards (i.e., load to use,
LO/HI, etc.) that it's desirable to have the simulator warn
about (so as to be sure the compiler is avoiding them
adequately).

I believe both should be evaluated on a case-by-case basis, the latter
obviously moreso than the former. (Odds are, if a MIPS CPU has _that_
many bugs, it's not gonna pass conformance testing or ship in any
significant volume. IMO, variations among pre-MIPS32/MIPS64 chips are
likely to be greater than post-, but thankfully going forward we
should see fewer of them being added to the simulator!)

anyway, that having been said, I think i should now state my opinion
re: evaluating this particular case, i.e. Richard's patch to add
vr5500 support (in particular the conditional in check_mf_cycles) as
modified by my subsequent patch:

* the check in check_mf_cycles will evaluate to a constant
true/false if not a multi-machine simulator,

* the vr5500 run-time conditionals are not many in number (uh,
1 8-), and therefore aren't intrusive in that manner (i.e.,
they don't make the code harder to read),

* based on the above 2 points, that runtime check will not
hurt code performance in the case where we care most about
performance (single-machine), and since it's just one check
shouldn't hurt much at all even if multi-machine.

You've suggested the alternative of changing check_mf_cycles to be a
machine-dependent function. I do believe (as you've stated) that
_that_ would require adding vr5500 tags scattered on every instruction
used by the vr5500, i.e., every mipsIV instruction, with the _only_
difference between vr5500 and mipsIV (over the set of insn/fns used by
mipsIV) being check_mf_cycles.

IMO, that would:

* have no significant performance benefit, but

* make the patch to add the vr5500 support harder to
understand (i.e. cluttered with Lots of lines of diffs to
just add vr5500 tags), and finally and most importantly:

* hurt the maintainability of the code.

Instead of vr5500 differences being clearly marked as with
Richard's code, one would _have_ to use
e.g. compare_igen_models to tell the difference, and even
that would say only that each had different check_mf_cycles
fns (i.e., not make the difference clear).

(I also think that doing it the way you suggest would set the wrong
precedent for future additions, which if MIPS32/MIPS64 based really
almost certainly should be done w/ conditional checks.)

In other words, IMO, doing it the way Richard has done it is basically
all gain, no pain, and that compared to that tagging every mipsIV insn
w/ vr5500 is much pain, no gain. 8-)

chris