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: MIPS32r2 FP MADD instruction set support


On Thu, 21 Feb 2013, Richard Sandiford wrote:

> >  This issue was originally raised here:
> >
> > http://gcc.gnu.org/ml/gcc-patches/2012-12/msg00863.html
> >
> >  We have a shortcoming in GCC in that we only allow the use half of the FP 
> > MADD instruction subset (MADD.fmt and MSUB.fmt) in the 64-bit/32-register 
> > mode (CP0.Status.FR == 1) on MIPS32r2 processors.  Furthermore we never 
> > enable the other half (NMADD.fmt and NMSUB.fmt) on those processors.  
> > However this whole instruction subset is always available on MIPS32r2 FPUs 
> > regardless of the mode selected, just as it always has been on FPUs of the 
> > 64-bit ISA line from MIPS IV up.
> 
> Hmm, this was discussed here:
> 
>     http://gcc.gnu.org/ml/gcc-patches/2006-11/msg00488.html
>     http://gcc.gnu.org/ml/gcc-patches/2006-11/msg00492.html
> 
> The footnote for COP1X instructions on page 12 of volume 1 of the MIPS32 ISA
> (v2.50) says:

 Please note that v2.50 is obsolete, several typos were corrected in later 
revisions.  Sadly I don't even have a copy of that exact revision of 
volume I.

> 1. In Release 1 of the Architecture, these instructions are legal only
>    with a MIPS64 processor with 64-bit operations enabled (they are, in
>    effect, actually MIPS64 instructions). In Release 2 of the Architecture,
>    these instructions are legal with either a MIPS32 or MIPS64 processor
>    _which includes a 64-bit floating point unit_.
> 
> (my emphasis).  "which" rather than "that" makes this a bit ambiguous,
> but various comments in the rest of the manual imply that MIPS32r2 allows
> an implementation choice between 32-bit and 64-bit FPUs.  E.g. page 8 says:
> 
>   Support for 64-bit coprocessors with 32-bit CPUs: These changes allow
>   a 64-bit coprocessor (including an FPU) to be attached to a 32-bit
>   CPU. This enhancement is optional in a Release 2 implementation.
> 
> and page 45 says:
> 
>   In addition to an Instruction Set Architecture, the MIPS architecture
>   definition includes processing resources such as the set of
>   coprocessor general registers. In Release 1 of the Architecture, the
>   32-bit registers in MIPS32 were enlarged to 64-bits in MIPS64;
>   however, these 64-bit FPU registers are not backwards
>   compatible. Instead, processors implementing the MIPS64 Architecture
>   provide a mode bit to select either the 32-bit or 64-bit register
>   model. In Release 2 of the Architecture, a 32-bit CPU _may_ include a
>   full 64-bit coprocessor, including a floating point unit which
>   implements the same mode bit to select 32-bit or 64-bit FPU register
>   model.
> 
> On page 322 of volume 2, the footnote for "Table A-20 MIPS64 COP1X
> Encoding of Function Field" uses slightly different wording:
> 
>   COP1X instructions are legal only if 64-bit floating point operations
>   are enabled.
> 
> So was this all a big misunderstanding on my part?  The TARGET_FLOAT64
> condition came from MIPS themselves, and when challenged they seemed
> pretty adamant that it was correct.  If I was wrong to be convinced
> by the explanation, I hope you can at least see why I was convinced. :-)

 As good as my English comprehension might be I believe your understanding 
of documentation is right.  However several inconsistencies have already 
been spotted and it may well be that for example the restriction on COP1X 
instructions is simply a leftover that leaked from rev.1 of the MIPS64 
architecture and was not spotted on the update (please note that obviously 
all the architecture documents are produced from templates shared between 
the MIPS32 and MIPS64 architectures with some parts being output 
conditionally as appropriate).

> If it wasn't a misunderstanding, then the point is that we can't tell
> from ISA_MIPS32R2 alone whether the target has a 32-bit or 64-bit FPU,
> but we know that it must have a 64-bit FPU if using TARGET_FLOAT64.

 Well, the important point is as I understand all the MIPS32r2 FPU 
instructions (not all the FP formats of course though, L and PS are 
optional) are mandatory no matter if the FPU is 32-bit or 64-bit.  This 
should rule out any question as to what kind of FPU has been implemented 
or whether the 64-bit mode has been enabled.  I could be wrong however.

 However the good news is it's not the architecture documentation set that 
is normative for architecture implementers, it's the AVP (Architecture 
Verification Program) they have to run successfully on their processors to 
claim compliance.

 Steve, would you therefore please do us a favour and check what the AVP 
for MIPS32r2 requires for support of the floating-point MADD instruction 
subset, or preferably, the whole COP1X instruction set?  Are these 
instructions only mandatory for a 64-bit FPU (CP1.FIR.F64 == 1), or do 
they have to be included in all FPU implementations?

> This part is OK, thanks, and is probably the only bit that's suitable for
> 4.8 at this stage.  Would you mind applying it separately?

 Applied now, thanks.

> I was named in that commit but the VR54xx stuff wasn't mine.  I do remember
> that Mike put a lot of effort into tuning the VR54xx madd stuff though,
> because of the difficulty of having multiply-accumulate instructions
> that force the use of HI/LO on an architecture that also has efficient
> three-operand multiplies.  So I'm pretty sure that this worked correctly
> in the Cygnus devo tree, and your explanation of a misplaced hunk seems
> very convincing.

 I'll make appropriate adjustments then.  Also, regardless of the outcome 
of the instruction presence on 32-bit FPUs, do you agree it is a good idea 
to fold ISA_HAS_FP_MADD4_MSUB4 and ISA_HAS_NMADD4_NMSUB4 into one macro?

  Maciej


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