This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] MIPS: 64bit floating point support for MIPS32R2


Looks good, thanks.  Just a couple of nits:

David Ung <davidu@mips.com> writes:
> @@ -4804,8 +4824,9 @@
>  	 only one right answer here.  */
>        if (TARGET_64BIT && TARGET_DOUBLE_FLOAT && !TARGET_FLOAT64)
>  	error ("unsupported combination: %s", "-mgp64 -mfp32 -mdouble-float");
> -      else if (!TARGET_64BIT && TARGET_FLOAT64)
> -	error ("unsupported combination: %s", "-mgp32 -mfp64");
> +      else if (!TARGET_64BIT && TARGET_FLOAT64 
> +	       && !(ISA_HAS_MXHC1 && mips_abi == ABI_32))
> +	error ("unsupported combination: %s", "-mgp32 and -mfp64 can only be combined if the target supports the mfhc1 and mthc1 instructions");

This isn't quite right: the %s argument will not be exposed to
translation.  Just remove the first argument and it'll be fine.
Also wrap the line to 80 characters.  You can use string
concatenation for that:

	error ("-mgp32 and -mfp64 can only be combined if the target"
	       " supports the mfhc1 and mthc1 instructions");

(I used the "unsupported combination: %s" construct elsewhere so that
there weren't separate translation messages for each group of options.)

>    /* Loading a 32-bit value into a 64-bit floating-point register
>       will not sign-extend the value, despite what LOAD_EXTEND_OP says.
>       We can't allow 64-bit float registers to change from SImode to
>       to a wider mode.  */
> -  if (TARGET_FLOAT64
> +  if (TARGET_64BIT && TARGET_FLOAT64
>        && from == SImode
>        && GET_MODE_SIZE (to) >= UNITS_PER_WORD
>        && reg_classes_intersect_p (FP_REGS, class))
>      return true;

Minor nit, but please keep it to one condition per line.

> +Note that MIPS32R2 optionally supports 64-bit floating point units.
> +Use the @option{-mfp64} to tell the compiler to generate 64-bit
> +floating code as GCC defaults to 32-bit float when generating for any
> +MIPS32R2 cpu.  
> +Currently, only O32 ABI supports this combination.  The calling
> +conventions remains the same, except that the registers are in 64-bits
> +instead of paired of consecutive 32-bit for double types.
> +Eg. function returning value of full 64-bit in $f0 instead of two
> +32-bit values in $f0/$f1.

I would prefer something a bit more explicit.  How about:

   GCC supports a variant of the o32 ABI in which floating-point registers
   are 64 rather than 32 bits wide.  You can select this combination with
   @option{-mabi=32} @option{-mfp64}.  This ABI relies on the @samp{mthc1}
   and @samp{mfhc1} instructions and is therefore only supported for
   MIPS32R2 processors.

   The register assignments for arguments and return values remain the
   same, but each scalar value is passed in a single 64-bit register
   rather than a pair of 32-bit registers.  For example, scalar
   floating-point values are returned in @samp{$f0} only, not a
   @samp{$f0}/@samp{$f1} pair.  The set of call-saved registers also
   remains the same, but all 64 bits are saved.

These changes don't need a full retest.  Just make sure that cc1
builds and the new error message is sensible.  Also run "make dvi"
and "make info" and check that the modified documentation works OK.

OK with those changes.  (Could you post the final patch for reference?)

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]