FPU IEEE 754 for MIPS r5900

Hello Richard,

I used the SPU code in GCC as example for creating an r5900_single_format structure. The patch is attached to the e-mail. I want to submit this patch.

> >> * removing the ISA_HAS_LDC1_SDC1 setting.  I realise what you did
> >>   describes the reality of the processor, but the problem is that
> >>   the patch doesn't provide an alternative for 64-bit loads and
> >>   stores when -mfp64 is used.  That combination also isn't rejected,
> >>   so we're likely to get an internal compiler error instead.
> >>
> >>   This change shouldn't affect the soft-float case you describe.
> >>   It also shouldn't be important for the single-float code.

I needed to patch ISA_HAS_LDC1_SDC1, because the GCC uses the instructions also for single float to store FPRs on the stack. To get around the missing alternative, I reject in the code the following combinations, because it would never work:
-march=r5900 -mfp64 -mhard-float
-march=r5900 -mdouble-float -mhard-float

> >> FWIW, the Cygnus/Red Hat version of the port just stuck with the R5900
> >> behaviour and made GCC understand it (MODE_HAS_* & various other bits).
> >> This code was then updated and extended for the SPU.  I'd have expected
> >> the support to be in reasonably good shape because of the SPU.
> >
> > I assume that you mean the cell processor of the PS3 and not the Sound
> > Processing Unit of the PS2.
>
> :-)
>
> > The macros MODE_HAS_* in the GCC look promising.
>
> You've probably already seen it, but there's also spu_single_format.

To be able to use it, you need to use mipsr5900el and "--with-float=single". "--with-float=hard" results in double float because of MIPS ISA III.
I didn't changed the default in config.gcc. It is still soft float, because floating point doesn't behave as defined by IEEE 754. I don't see much improvement. This is the case on the PS2 and the PS3. For example inf minus inf should be NaN, but on both systems it is 0. I tested it on r5900 and the PS3 SPU. Both calculates the same result despite the MODE_HAS_* implementation. This means that there is a patch needed in the generic part (i.e. not  mips) of the GCC. Currently I only provide a simple patch, so that the GCC knows the missing stuff, even if it is not correctly handled in the generic GCC part. I thought that Linux is running on SPU, but this is not the case, because it just a simple coprocessor. It seems that SPU is not used for complex floating point calculations in homebrew software. I am pretty sure that nearly nobody knows that floating point in GCC behaves this way on SPU.

Best regards
Jürgen

"Jürgen Urban" <JuergenUrban@gmx.de> writes:
> I used the SPU code in GCC as example for creating an
> r5900_single_format structure. The patch is attached to the e-mail. I
> want to submit this patch.

Thanks.  Are there any real differences though?  E.g. in your version
you set has_sign_dependent_rounding, but that's not necessary when the
only rounding mode is towards zero.  has_sign_dependent_rounding says
whether rounding X vs. -X can give numbers of different magnitude.
(It was actually because of r5900 that this distinction was added.)

I'm also not sure it makes sense to choose a different NaN encoding
when NaNs aren't supported anyway.

It would be good if we could reuse spu_single_format directly.

> To be able to use it, you need to use mipsr5900el and
> "--with-float=single". "--with-float=hard" results in double float
> because of MIPS ISA III.

This isn't quite right as-is.  The code that applies the --with-float
setting is:

#define OPTION_DEFAULT_SPECS \
  ... \
  {"float", "%{!msoft-float:%{!mhard-float:-m%(VALUE)-float}}" }, \

in mips.h.  So -mdouble-float wouldn't override --with-float=single, etc.

single vs. double has traditionally been a separate choice from hard
vs. soft (which is a bit unfortunate given that single vs. double makes
no sense for soft float).  Maybe we should have --with-fpu=single and
--with-fpu=double instead.

> I didn't changed the default in config.gcc. It is still soft float,
> because floating point doesn't behave as defined by IEEE 754. I don't
> see much improvement. This is the case on the PS2 and the PS3. For
> example inf minus inf should be NaN, but on both systems it is 0.
> I tested it on r5900 and the PS3 SPU. Both calculates the same result
> despite the MODE_HAS_* implementation. This means that there is a
> patch needed in the generic part (i.e. not mips) of the GCC.

But the format doesn't have an infinity representation, does it?
The IEEE infinity encoding is instead treated as a large number.
So it sounds like a bug if GCC is treating any (r5900|spu)_single_format
value as infinity to be begin with.

> @@ -989,7 +991,7 @@
>  /* True if trunc.w.s and trunc.w.d are real (not synthetic)
>     instructions.  Both require TARGET_HARD_FLOAT, and trunc.w.d
>     also requires TARGET_DOUBLE_FLOAT.  */
> -#define ISA_HAS_TRUNC_W		(!ISA_MIPS1)
> +#define ISA_HAS_TRUNC_W		(!ISA_MIPS1 || TARGET_MIPS5900)
>  
>  /* ISA includes the MIPS32r2 seb and seh instructions.  */
>  #define ISA_HAS_SEB_SEH		((ISA_MIPS32R2		\

This part shouldn't be necessary.  The ISA_* and TARGET_MIPS* macros are
kept in sync, so it can never be the case that ISA_MIPS1 && TARGET_MIPS5900.
(E.g. -mips1 -march=r5900 is rejected.)

Thanks,
Richard

Hello Richard,

> Gesendet: Sonntag, 07. Juli 2013 um 10:15 Uhr
> "Jürgen Urban" <JuergenUrban@gmx.de> writes:
> > I used the SPU code in GCC as example for creating an
> > r5900_single_format structure. The patch is attached to the e-mail. I
> > want to submit this patch.
>
> Thanks.  Are there any real differences though?  E.g. in your version
> you set has_sign_dependent_rounding, but that's not necessary when the
> only rounding mode is towards zero.  has_sign_dependent_rounding says
> whether rounding X vs. -X can give numbers of different magnitude.
> (It was actually because of r5900 that this distinction was added.)
>
> I'm also not sure it makes sense to choose a different NaN encoding
> when NaNs aren't supported anyway.
>
> It would be good if we could reuse spu_single_format directly.

I don't know what the effect of has_sign_dependent_rounding is. I also can't test it, because the GCC is already not correctly working on SPU.
The EE Core User's Manual also says that the Guard, Round and Sticky bits are ignored. So the rounding can differ from IEEE 754 in the least significant bit.
Exceptions are not supported and must be emulated by trap instructions.

> > To be able to use it, you need to use mipsr5900el and
> > "--with-float=single". "--with-float=hard" results in double float
> > because of MIPS ISA III.
>
> This isn't quite right as-is.  The code that applies the --with-float
> setting is:
>
> #define OPTION_DEFAULT_SPECS \
>   ... \
>   {"float", "%{!msoft-float:%{!mhard-float:-m%(VALUE)-float}}" }, \
>
> in mips.h.  So -mdouble-float wouldn't override --with-float=single, etc.
>
> single vs. double has traditionally been a separate choice from hard
> vs. soft (which is a bit unfortunate given that single vs. double makes
> no sense for soft float).  Maybe we should have --with-fpu=single and
> --with-fpu=double instead.

In my tests the parameter "--with-float=single" automatically selected hard float as default.
I don't see a way to change the configure script to use "--with-fpu" without changing the parameters of GCC also. This would make it incompatible with old GCC versions.

> > I didn't changed the default in config.gcc. It is still soft float,
> > because floating point doesn't behave as defined by IEEE 754. I don't
> > see much improvement. This is the case on the PS2 and the PS3. For
> > example inf minus inf should be NaN, but on both systems it is 0.
> > I tested it on r5900 and the PS3 SPU. Both calculates the same result
> > despite the MODE_HAS_* implementation. This means that there is a
> > patch needed in the generic part (i.e. not mips) of the GCC.
>
> But the format doesn't have an infinity representation, does it?

It doesn't have a representation for infinity, the calculation returns +Fmax or -Fmax according to the manual. In my test I can see that +Fmax is 0x7fffffff.

> The IEEE infinity encoding is instead treated as a large number.
> So it sounds like a bug if GCC is treating any (r5900|spu)_single_format
> value as infinity to be begin with.
>
> > @@ -989,7 +991,7 @@
> >  /* True if trunc.w.s and trunc.w.d are real (not synthetic)
> >     instructions.  Both require TARGET_HARD_FLOAT, and trunc.w.d
> >     also requires TARGET_DOUBLE_FLOAT.  */
> > -#define ISA_HAS_TRUNC_W		(!ISA_MIPS1)
> > +#define ISA_HAS_TRUNC_W		(!ISA_MIPS1 || TARGET_MIPS5900)
> >
> >  /* ISA includes the MIPS32r2 seb and seh instructions.  */
> >  #define ISA_HAS_SEB_SEH		((ISA_MIPS32R2		\
>
> This part shouldn't be necessary.  The ISA_* and TARGET_MIPS* macros are
> kept in sync, so it can never be the case that ISA_MIPS1 && TARGET_MIPS5900.
> (E.g. -mips1 -march=r5900 is rejected.)

OK, I tested it again. You are right, it is working without this part of the patch.

Best regards
Jürgen

"Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> "Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> > I used the SPU code in GCC as example for creating an
>> > r5900_single_format structure. The patch is attached to the e-mail. I
>> > want to submit this patch.
>>
>> Thanks.  Are there any real differences though?  E.g. in your version
>> you set has_sign_dependent_rounding, but that's not necessary when the
>> only rounding mode is towards zero.  has_sign_dependent_rounding says
>> whether rounding X vs. -X can give numbers of different magnitude.
>> (It was actually because of r5900 that this distinction was added.)
>>
>> I'm also not sure it makes sense to choose a different NaN encoding
>> when NaNs aren't supported anyway.
>> 
>> It would be good if we could reuse spu_single_format directly.
>
> I don't know what the effect of has_sign_dependent_rounding is.

Like I say, it tells GCC whether -X can round to something other than -Y
in cases where X would round to Y.  This is true for IEEE when rounding
towards +infinity or -infinity, but those modes aren't supported on the
R5900.

Some transformations are invalid when has_sign_dependent is true.
E.g. -(X - Y) is not always equal to Y - X.  We want it to be false
when possible, so it looked like the spu_single_format version was right.

> I also can't test it, because the GCC is already not correctly working
> on SPU.

Can you give an example?

> The EE Core User's Manual also says that the Guard, Round and Sticky
> bits are ignored. So the rounding can differ from IEEE 754 in the least
> significant bit.
> Exceptions are not supported and must be emulated by trap instructions.

But defining r5900_single_format doesn't change the way GCC handles that,
does it?

I suppose my point is that we should only introduce another format if
there is a testcase where r5900_single_format produces the right results
and spu_single_format doesn't.

>> > To be able to use it, you need to use mipsr5900el and
>> > "--with-float=single". "--with-float=hard" results in double float
>> > because of MIPS ISA III.
>>
>> This isn't quite right as-is.  The code that applies the --with-float
>> setting is:
>>
>> #define OPTION_DEFAULT_SPECS \
>>   ... \
>>   {"float", "%{!msoft-float:%{!mhard-float:-m%(VALUE)-float}}" }, \
>>
>> in mips.h.  So -mdouble-float wouldn't override --with-float=single, etc.
>>
>> single vs. double has traditionally been a separate choice from hard
>> vs. soft (which is a bit unfortunate given that single vs. double makes
>> no sense for soft float).  Maybe we should have --with-fpu=single and
>> --with-fpu=double instead.
>
> In my tests the parameter "--with-float=single" automatically selected
> hard float as default.
> I don't see a way to change the configure script to use "--with-fpu"
> without changing the parameters of GCC also. This would make it
> incompatible with old GCC versions.

It should just be a case of adding:

  {"fpu", "%{!msingle-float:%{!mdouble-float:-m%(VALUE)-float}}" }, \

to the macro above.

>> > I didn't changed the default in config.gcc. It is still soft float,
>> > because floating point doesn't behave as defined by IEEE 754. I don't
>> > see much improvement. This is the case on the PS2 and the PS3. For
>> > example inf minus inf should be NaN, but on both systems it is 0.
>> > I tested it on r5900 and the PS3 SPU. Both calculates the same result
>> > despite the MODE_HAS_* implementation. This means that there is a
>> > patch needed in the generic part (i.e. not mips) of the GCC.
>>
>> But the format doesn't have an infinity representation, does it?
>
> It doesn't have a representation for infinity, the calculation returns
> +Fmax or -Fmax according to the manual. In my test I can see that +Fmax
> is 0x7fffffff.

Right, that was my point:

>> The IEEE infinity encoding is instead treated as a large number.
>> So it sounds like a bug if GCC is treating any (r5900|spu)_single_format
>> value as infinity to be begin with.

You were saying that GCC produces the wrong result for "inf minus inf".
But you can't even do that calculation on r5900 floats, because there's
no infinity representation to begin with.  Maybe it's just semantics,
but it sounded like the bug was that we assumed r5900 had inf in the
first place, not that "inf - inf" produced the wrong result.

Thanks,
Richard

Hello,
> "Jürgen Urban" <JuergenUrban@gmx.de> writes:
> >> "Jürgen Urban" <JuergenUrban@gmx.de> writes:
> >> > I used the SPU code in GCC as example for creating an
> >> > r5900_single_format structure. The patch is attached to the e-mail. I
> >> > want to submit this patch.
> >>
> >> Thanks.  Are there any real differences though?  E.g. in your version
> >> you set has_sign_dependent_rounding, but that's not necessary when the
> >> only rounding mode is towards zero.  has_sign_dependent_rounding says
> >> whether rounding X vs. -X can give numbers of different magnitude.
> >> (It was actually because of r5900 that this distinction was added.)
> >>
> >> I'm also not sure it makes sense to choose a different NaN encoding
> >> when NaNs aren't supported anyway.
> >>
> >> It would be good if we could reuse spu_single_format directly.
> >
> > I don't know what the effect of has_sign_dependent_rounding is.
>
> Like I say, it tells GCC whether -X can round to something other than -Y
> in cases where X would round to Y.  This is true for IEEE when rounding
> towards +infinity or -infinity, but those modes aren't supported on the
> R5900.
>
> Some transformations are invalid when has_sign_dependent is true.
> E.g. -(X - Y) is not always equal to Y - X.  We want it to be false
> when possible, so it looked like the spu_single_format version was right.

The manual says that the rounding differs in the least significant bit, so we need to assume that this can happen any time and it also leads to a different result when the sign is different. Currently I have problems testing it, because the latest GCC which I use (svn r200583) is not able to build the Linux kernel, because it has at least 2 bugs. The first is the bug 57698 introduced between 17.06. and 26.06. It seems that no GCC developer is able to fix it. The second bug affects the dvb_demux.c from Linux 2.6.34 when -Os and -mlong is used. This triggers an sanity check in do_SUBST in file gcc/combine.c. The first bugs happens on all architectures. The second also appears with normal mipsel. There is also a bug which annoys me since years, __attribute__((aligned(16))) is not working with local variables and doesn't print a warning. This is particulary a problem when used in typedefs, because the problem is not visible.
My native ps2sdk doesn't have an implementation of rounding. I don't have an environment with a combination of the correct versions of the different components for Linux. So I can't test it at the moment.

> > I also can't test it, because the GCC is already not correctly working
> > on SPU.
>
> Can you give an example?

I wanted to say that I don't know what is correct or not, because the GCC is already not handling the other stuff correctly, e.g.:
inf - inf => 0
nan - nan => 0
nan == nan => true (this must be false according to IEEE 754)
but __builtin_nanf() == __builtin_nanf() => false (this is correct according to IEEE 754, because this is already evaluted by GCC before creating the assembler code)
nan == inf => true
__builtin_inff() is 0x7fffffff
__builtin_nanf() is 0x7fffffff

> > The EE Core User's Manual also says that the Guard, Round and Sticky
> > bits are ignored. So the rounding can differ from IEEE 754 in the least
> > significant bit.
> > Exceptions are not supported and must be emulated by trap instructions.
>
> But defining r5900_single_format doesn't change the way GCC handles that,
> does it?

At least I expected that exceptions are generated.
Rounding is something new which also needs to be added, but not now.

> I suppose my point is that we should only introduce another format if
> there is a testcase where r5900_single_format produces the right results
> and spu_single_format doesn't.

Currently I don't have a testsuite for this. So I also don't have any tests which have these results.

> >> > To be able to use it, you need to use mipsr5900el and
> >> > "--with-float=single". "--with-float=hard" results in double float
> >> > because of MIPS ISA III.
> >>
> >> This isn't quite right as-is.  The code that applies the --with-float
> >> setting is:
> >>
> >> #define OPTION_DEFAULT_SPECS \
> >>   ... \
> >>   {"float", "%{!msoft-float:%{!mhard-float:-m%(VALUE)-float}}" }, \
> >>
> >> in mips.h.  So -mdouble-float wouldn't override --with-float=single, etc.
> >>
> >> single vs. double has traditionally been a separate choice from hard
> >> vs. soft (which is a bit unfortunate given that single vs. double makes
> >> no sense for soft float).  Maybe we should have --with-fpu=single and
> >> --with-fpu=double instead.
> >
> > In my tests the parameter "--with-float=single" automatically selected
> > hard float as default.
> > I don't see a way to change the configure script to use "--with-fpu"
> > without changing the parameters of GCC also. This would make it
> > incompatible with old GCC versions.
>
> It should just be a case of adding:
>
>   {"fpu", "%{!msingle-float:%{!mdouble-float:-m%(VALUE)-float}}" }, \
>
> to the macro above.

OK, I didn't know that.

> >> > I didn't changed the default in config.gcc. It is still soft float,
> >> > because floating point doesn't behave as defined by IEEE 754. I don't
> >> > see much improvement. This is the case on the PS2 and the PS3. For
> >> > example inf minus inf should be NaN, but on both systems it is 0.
> >> > I tested it on r5900 and the PS3 SPU. Both calculates the same result
> >> > despite the MODE_HAS_* implementation. This means that there is a
> >> > patch needed in the generic part (i.e. not mips) of the GCC.
> >>
> >> But the format doesn't have an infinity representation, does it?
> >
> > It doesn't have a representation for infinity, the calculation returns
> > +Fmax or -Fmax according to the manual. In my test I can see that +Fmax
> > is 0x7fffffff.
>
> Right, that was my point:
>
> >> The IEEE infinity encoding is instead treated as a large number.
> >> So it sounds like a bug if GCC is treating any (r5900|spu)_single_format
> >> value as infinity to be begin with.
>
> You were saying that GCC produces the wrong result for "inf minus inf".
> But you can't even do that calculation on r5900 floats, because there's
> no infinity representation to begin with.  Maybe it's just semantics,
> but it sounded like the bug was that we assumed r5900 had inf in the
> first place, not that "inf - inf" produced the wrong result.

According to the documentation http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html: __builting_inf() should produce a warning if infinities are not supported. This doesn't happen, so I need to assume that GCC thinks that it is supported and emulates it, so it should work.

Best regards
Jürgen

"Jürgen Urban" <JuergenUrban@gmx.de> writes:
> Hello,
>> "Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> >> "Jürgen Urban" <JuergenUrban@gmx.de> writes:
>> >> > I used the SPU code in GCC as example for creating an
>> >> > r5900_single_format structure. The patch is attached to the e-mail. I
>> >> > want to submit this patch.
>> >>
>> >> Thanks.  Are there any real differences though?  E.g. in your version
>> >> you set has_sign_dependent_rounding, but that's not necessary when the
>> >> only rounding mode is towards zero.  has_sign_dependent_rounding says
>> >> whether rounding X vs. -X can give numbers of different magnitude.
>> >> (It was actually because of r5900 that this distinction was added.)
>> >>
>> >> I'm also not sure it makes sense to choose a different NaN encoding
>> >> when NaNs aren't supported anyway.
>> >>
>> >> It would be good if we could reuse spu_single_format directly.
>> >
>> > I don't know what the effect of has_sign_dependent_rounding is.
>>
>> Like I say, it tells GCC whether -X can round to something other than -Y
>> in cases where X would round to Y.  This is true for IEEE when rounding
>> towards +infinity or -infinity, but those modes aren't supported on the
>> R5900.
>>
>> Some transformations are invalid when has_sign_dependent is true.
>> E.g. -(X - Y) is not always equal to Y - X.  We want it to be false
>> when possible, so it looked like the spu_single_format version was right.
>
> The manual says that the rounding differs in the least significant bit,
> so we need to assume that this can happen any time and it also leads to
> a different result when the sign is different.

Just to be clear, do you mean different from IEEE?  That doesn't matter
for defining has_sign_dependent_rounding, which is comparing the R5900
rounding of intermediate result -X vs. the R5900 rounding of intermediate
result X.

> Currently I have problems
> testing it, because the latest GCC which I use (svn r200583) is not able
> to build the Linux kernel, because it has at least 2 bugs. The first is
> the bug 57698 introduced between 17.06. and 26.06. It seems that no GCC
> developer is able to fix it. The second bug affects the dvb_demux.c from
> Linux 2.6.34 when -Os and -mlong is used. This triggers an sanity check
> in do_SUBST in file gcc/combine.c. The first bugs happens on all
> architectures. The second also appears with normal mipsel. There is also
> a bug which annoys me since years, __attribute__((aligned(16))) is not
> working with local variables and doesn't print a warning. This is
> particulary a problem when used in typedefs, because the problem is not
> visible.
> My native ps2sdk doesn't have an implementation of rounding. I don't
> have an environment with a combination of the correct versions of the
> different components for Linux. So I can't test it at the moment.

OK, but the patch does need to be tested before it goes in.
As far as 57698 goes, it would be fine to test with a 17.06 version.

But why do you need to be able to build linux with gcc trunk to test this?
The kernel doesn't use FP anyway.  Can't you just use testcases running
under the kernel you already have?

>> > I also can't test it, because the GCC is already not correctly working
>> > on SPU.
>>
>> Can you give an example?
>
> I wanted to say that I don't know what is correct or not, because the
> GCC is already not handling the other stuff correctly, e.g.:
> inf - inf => 0
> nan - nan => 0
> nan == nan => true (this must be false according to IEEE 754)

Sorry, I was meaning in terms of source code.  I still think these
expressions have no meaning for R5900 floats, where there isn't an
infinity or a nan to begin with.  If the format doesn't have infinity
to begin with, there's no right or wrong answer for "inf - inf"
(__builtin_inff() - __builtin_inff()).  0x7f800000 is not inf,
so what the real FPU does for 0x7f800000 doesn't affect things.

When you construct 0x7f800000 as a finite value it seems to be
handled correctly.  On spu-elf I tried:

float f = 0x1.0p128f - 0x1.0p128f;

and got zero as expected.  Note that the equivalent on x86_64 gives
a warning:

warning: floating constant exceeds range of ‘float’ [-Woverflow]

and produces a NaN.  So it looks like this is working.

> According to the documentation
> http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html: __builting_inf()
> should produce a warning if infinities are not supported. This doesn't
> happen, so I need to assume that GCC thinks that it is supported and
> emulates it, so it should work.

I tried:

  float f = __builtin_inff ();

for spu-elf and got:

  warning: target format does not support infinity [enabled by default]

This doesn't give an error:

  float f = __builtin_inf ();

because doubles do have infinity.  Is that the problem you're seeing?
It's not obvious what should happen there though -- others would know
better than me.  But it shouldn't occur in practice anyway.  The
standard INFINITY macro always uses __builtin_inff, and the code was
written for that assumption:

  /* __builtin_inff is intended to be usable to define INFINITY on all
     targets.  If an infinity is not available, INFINITY expands "to a
     positive constant of type float that overflows at translation
     time", footnote "In this case, using INFINITY will violate the
     constraint in 6.4.4 and thus require a diagnostic." (C99 7.12#4).
     Thus we pedwarn to ensure this constraint violation is
     diagnosed.  */
  if (!MODE_HAS_INFINITIES (TYPE_MODE (type)) && warn)
    pedwarn (loc, 0, "target format does not support infinity");

So using INFINITY should always give the warning.

Directly using the double __builtin_inf instead of the float __builtin_inff
to construct a float inf on a target that doesn't have float infs is a bit
"doctor, it hurts if I do this".  I think we can ignore it for the
initial patch.  Any existing code that converts __builtin_inf to float
isn't going to expect it to be treated as a finite value anyway.

I'm trying to make things simpler here, believe me :-)

Thanks,
Richard