Re: [mips patch rfa] Add MIPS32 Release 2 support.

[cgd at broadcom dot com]

At 07 Jan 2003 14:35:46 +0000, Richard Sandiford wrote:
> cgd@broadcom.com writes:
> > +/* ISA includes the MIPS32r2 ALU extensions other than rotates
> > +   (seb, seh, ins, ext).  Rotates are handled by ISA_HAS_ROTR_SI.  */
> > +#define ISA_HAS_ALU32R2         (!TARGET_MIPS16                        \
> > +                                 && (ISA_MIPS32R2                      \
> > +                                     ))
> > +
> 
> Did you think about adding ISA_HAS_SEB_SEH?  I'm not saying it's
> a good idea, but it does seem more consistent with the other
> ISA_HAS_* macros (e.g. ISA_HAS_NMADD_NMSUB).

yes.  Actually, first I implemented it more like this, as ISA_HAS_SEx.
Eric, on the other hand, i more partial to ISA_HAS_SE.  ("Hmm."  8-)

That name was intended to be like ISA_HAS_FP4, but eric has indicated
he'd like it the other way, so i'm currently testing ISA_HAS_SE.

I need to restart it anyway, so i'll change it to SEB_SEH.  If it's
not going to be more general, that's probably the best name.


> > @@ -94,6 +94,8 @@ Boston, MA 02111-1307, USA.  */
> >  	builtin_define ("__mips=4");				\
> >        else if (ISA_MIPS32)					\
> >  	builtin_define ("__mips=32");				\
> > +      else if (ISA_MIPS32R2)					\
> > +	builtin_define ("__mips=33");				\
> >        else if (ISA_MIPS64)					\
> >  	builtin_define ("__mips=64");				\
> 
> Blech. ;)

indeed.


> How about keeping __mips=32 and introducing a separate
> define for the ISA revision?

I considered that.

Actually, my personal preference would be to make people use __mips
less in favor of some more verbosely-named define.  Such a thing
already exists, see mips.h...  It's just not used by NetBSD (you
quoted the netbsd.h diff).  ("Bummer for them."  8-)

Ultimately, I broke down and concluded: (1) it's not really 32 and
users should have some way to differentiate, and (2) one good way to
convince them not to use that define is to put strange values in it.
8-)


> (Wonder if there'll ever be
> a mips32r32...)

Heheheh.

(1) the architecture revision field is only 3 bits.  That doesn't say
    much, they could use 0x7 as "look at this other field."  But...

(2) time delta between mips32 rev. 0.92 (January 20, 2001) and 1.90
    (September 1, 2002) = approx 20 months.  be conservative, assume
    half that (10 mos) between major releases, that's still > 300
    months.  "BGAF."  8-)



> > @@ -7362,6 +7369,10 @@ Equivalent to @samp{-march=mips4}.
> >  @item -mips32
> >  @opindex mips32
> >  Equivalent to @samp{-march=mips32}.
> > +
> > +@item -mips32r2
> > +@opindex mips32r2
> > +Equivalent to @samp{-march=mips32r2}.
> >  
> >  @item -mips64
> >  @opindex mips64
> 
> Does this option actually work?  As far as I can tell, -mips32r2
> == -mips32, since -mips* is interpreted using atoi().  I guess
> -mips33 will select 32r2 though. ;)

to be honest, didn't try it.  Well that's really remarkably lame isn't
it.  8-)

OK, will fix, thanks very much.


> Having an integer mips_isa has worked well so far, but now you're
> adding a second mips32 ISA, maybe mips_isa should become an enum
> instead (like mips_arch and mips_tune).

Yes, I thought about this.  Really, though, IMO it should be done as a
separate patch since it's a separate change, and I wanted to get this
done first to cause myself minimal merge pain.  8-)


> E.g.
> 
> enum mips_isa {
>   ISA_MIPS1,
>   ...,
>   ISA_MIPS32,
>   ISA_MIPS32R2,
>   ...
> };

No, that won't work, those names are already quite well used.  (I'd
already looked at it, really.  8-)

They'd have to be other than that.  But yes, that idea.

(Or, they could be that, but then all of those existing defines would
have to change.)


> Then use
> mips_parse_cpu to interpret the -mips option.

That would seem sensible, but...  given the current implementation,
mips_isa_string has the value after -mips...  i.e., 32r2, 32, 64, 1,
2, 3, etc.  I.e., it's not a complete mipsNN string.

I suppose there are two ways to go: sprintf into a string then look
that up, or check for the explicitly allowed cases using strcmp.  I
think i'm going to go for the latter.

(atoi is, in general, never the right thing...  8-)


> That'd get
> rid of all this "ISA 33" nastiness.

heh.


> There'd still need to be a special case for -mips16, of course.  
> Actually, since we have target flags free, maybe -mips16
> should go into TARGET_SWITCHES...

Possibly.  Not being a real mips16-head I'm not sure what the right
thing is there.  9-).



OK, so:

* ISA_HAS_SEB_SEH.

* fix mips_isa_string handling.


chris