Aw: Re: Re: [PATCH] FPU IEEE 754 for MIPS r5900

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

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