Re: [patch] change to MIPS 4650 instruction support

[cgd at sibyte dot com (Chris G. Demetriou)]

Daniel Jacobowitz <dmj+@andrew.cmu.edu> writes:
> The goal of this change is mostly to support the Nevada line - the
> Linux kernel makefiles traditionally pretend that they are r8000's,
> because that's the closest match to their scheduling that GCC has
> available, and add -mmad to gcc to enable these extra instructions. 
> That used to work (with gcc converting -mmad to -m4650), but doesn't
> any more.

Some part of me wants to say "so, the gross hack that used to work has
been 'fixed' into not working, the obvious solution is to do the fix
right and treat them as indpendent CPUs..."  8-)


> I agree completely... I just didn't want to touch all that code right
> away.  Both ISA and CPU are currently tested with equality in a bunch
> of places, so changing them to masks is not trivial.

Actually, the ISA value is _already_ mask-based.  (This was part of
the changes that had me poking at OPCODE_IS_MEMBER().)

Another possible solution -- in my opinion slightly less satisfying --
is to add an additional argument which is the mask of extensions which
are enabled.  The issue i have with that is, what's the use of
checking the CPU type in OPCODE_IS_MEMBER(), if what you really mean
by "cpu" is "the set of extensions implied by that CPU"?


> I was going to do it that way, but Ralf Baechle suggested otherwise:
>     Don't think of the r8000; the kernel only uses the -mcpu=r8000 option
>     because the Nevada CPUs have _somewhat_ similar scheduling properties
>     to the R8000.  This of it as an independant ISA expension which can
>     be used with an arbitrary MIPS processor - even a R3000 processor.

But, in fact, it can't _really_ be used on an arbitrary processor --
only on a processor that supports those instructions.  8-)

If you start adding arbitrary sets of extensions, you also have to be
careful of conflicts with other instructions defined by other CPUs
and/or by "standard" ISAs...


> > Note also that the rest of the stuff in mips_set_options can be used
> > as described above, i.e. they're changeable state.  Even if this patch
> > was otherwise OK (and I don't particularly like it, but i'm not in a
> > position to reject it 8-), then mad4650 should be .set'able.
>
> I could certainly do that.  I could even do something cleaner overall,
> if anyone expresses a preference for how it should be done.

Personally, I would suggest:

(1) add proper support for the CPUs you're trying to support.  This
has several benefits: it makes clearer what is and what isn't
supported, it means that people have to understand less black magic to
make the tools work for a given CPU type, and it means that internally
the dependencies used by particular CPU types are nicely delineated.

(2) implement the .set cpu=... directive to take a CPU name.  Probably
not strictly necessary for what you want to do (if you implement
proper support for your CPU type), but quite useful and it'd make a
lot of code better...


> I envision the CPU argument setting defaults for a collection of
> extension flags for all sorts of groups of instructions (like single
> float, double float, mad, etc.) as well as a scheduling or otherwise
> quirky policy, and then allowing the instruction flags to be
> overridden.

I'd suggest that, as a start, you just stick to the meaning implied by
'-mcpu=', and add support for the particular CPUs you're worried
about.


chris