RE: [PATCHv2,MIPS 1/2] MIPS64r6 support

[Matthew Fortune <Matthew dot Fortune at imgtec dot com>]

Moore, Catherine <Catherine_Moore@mentor.com> writes:
> Hi Matthew,
> 
> > -----Original Message-----
> > From: Matthew Fortune [mailto:Matthew.Fortune@imgtec.com]
> > Sent: Friday, November 14, 2014 6:07 PM
> 
> Overall, this patch looks really good.  It took me a while to get
> through it, but I only have a couple of minor comments.
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> 02268f3..7797b31 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> 
> @@ -11896,6 +12052,15 @@ mips_hard_regno_mode_ok_p (unsigned int regno,
> machine_mode mode)
>        if (TARGET_O32_FP64A_ABI && size <= 4 && (regno & 1) != 0)
>         return false;
> 
> +      /* Prevent the use of odd-numbered registers for CCFmode with the
> +        o32 FPXX ABI, otherwise allow them.
> +        The FPXX ABI does not permit double-precision data to be placed
> +        in odd-numbered registers and double-precision compares write
> +        them as 64-bit values.  Without this restriction the R6 FPXX
> +        ABI would not be able to execute in FR=1 FRE=1 mode.  */
> +      if (mode == CCFmode && ISA_HAS_CCF)
> +       return !(TARGET_FLOATXX && (regno & 1) != 0);
> +
>        /* Allow 64-bit vector modes for Loongson-2E/2F.  */
>        if (TARGET_LOONGSON_VECTORS
>           && (mode == V2SImode
> 
> I don't think we ever have CCFmode when ISA_HAS_CCF is false.  Maybe
> just check for CCFmode?

Good point. I'll remove the ISA_HAS_CCF part.

> If there really is a need for both conditions, then the order of the
> checks needs to be reversed.  Also, the comment is hard to follow.
> 
> How about:
> 
> /* The FPXX ABI requires double-precision values to be placed in even-
> numbered registers.  Disallow
>      odd-numbered registers with CCFmode because CCF mode double-
> precision compares will write a 64-bit value
>      to a register.  */
> 
> Did I get that right?

I think that captures it, thanks.

> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h index
> 8a38829..c110b5e 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> 
> 
> -/* ISA supports instructions MULT and MULTU.
> -   This is always true, but the macro is needed for ISA_HAS_<D>MULT
> -   in mips.md.  */
> -#define ISA_HAS_MULT           (1)
> +/* ISA supports instructions MULT and MULTU.  */
> +#define ISA_HAS_MULT           ISA_HAS_HILO
> +
> 
> I preferred the definition in your original patch :
> #define ISA_HAS_MULT	mips_isa_rev <= 5
> 
> Would you mind switching it back?

Fine with me.

I'll sync the work with trunk and do some testsuite runs to check for
regressions in the interim and commit early next week probably.

Thanks for the review,
Matthew