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

[cgd at broadcom dot com]

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