Re: [PATCH] Support for MIPS R5900 (Sony Playstation 2)

["Jürgen Urban" <JuergenUrban at gmx dot de>]

>  FWIW, I don't like the way mips_opts.micromips has been special-cased for
> CPU_R5900 with this change, e.g.:
> 
> -	  if (mips_opts.isa != ISA_MIPS1 || mips_opts.micromips)
> +	  if ((mips_opts.isa != ISA_MIPS1 || mips_opts.micromips)
> +	      && (mips_opts.arch != CPU_R5900))
> 
> The original intent for -mmicromips was to override any specific CPU 
> selection -- for simplicity and because there are no vendor extensions to 
> the microMIPS instruction set (at least not yet).
> 
>  Of course one could say it makes no sense to enable microMIPS assembly 
> for processors that do not support it.  I guess this could be discussed, 
> but in any case a change to add R5900 support is not a place to change the
> preexisting general arrangements.
> 
>  I would therefore appreciate if pieces like the above were rewritten like
> below:
> 
> 	  if ((mips_opts.isa != ISA_MIPS1 && mips_opts.arch != CPU_R5900)
> 	      || mips_opts.micromips)
> 
> -- that is preserving the previous consistent behaviour (note that there 
> is no need for extra parentheses around an equality operation as it takes 
> precedence over the logical operation anyway).  I suppose the "ISA_MIPS1 
> || CPU_R5900" part could be macroised too -- it's used in several places 
> (e.g. some assertions as well).
> 
>  JÃrgen, as the author of the modification, would you please have a look 
> into it?

OK.

> 
>  As a side note -- I've noticed elsewhere in this change that the R5900 
> supports single-precision FP only.  This is similar to what the 
> R4640/R4650 does (up to supporting 32 single registers in the FR mode -- 
> is that any different to the R5900?), however up till now we did nothing 
> explicit for such a setup, except from respecting the -mfp32 option.  
> This change introduces extra bits however, like hardwiring -mfp32 -- 
> perhaps they could be generalised in some way to cover the R4640/R4650 as 
> well? -- again, for consistency.

The patch for R5900 currently sets -mfp32 with the line:

if (mips_opts.arch == CPU_R5900)
  {
    mips_opts.fp32 = 1;
  }
else
  ...

This avoids updating the FPU to 64 bit when setting higher mips ISA level.
The R4640/R4650 can be added here. Is this what you want?

Best regards
JÃrgen