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

2013-07-25 Thread Richard Sandiford
"Jürgen Urban"  writes:
>> "Jürgen Urban"  writes:
>> > Index: gcc/config.gcc
>> > ===
>> > --- gcc/config.gcc (Revision 200583)
>> > +++ gcc/config.gcc (Arbeitskopie)
>> > @@ -3080,7 +3080,7 @@
>> >esac
>> >  fi
>> >
>> > -# Infer a default setting for --with-float.
>> > +# Infer a default setting for --with-float and --with-fpu.
>> >  if test x$with_float = x; then
>> >case ${target} in
>> >  mips64r5900-*-* | mips64r5900el-*-* | mipsr5900-*-* | mipsr5900el-*-*)
>> > @@ -3089,6 +3089,17 @@
>> >with_float=soft
>> >;;
>> >esac
>> > +else
>> > +  case ${target} in
>> > +mips64r5900-*-* | mips64r5900el-*-* | mipsr5900-*-* | mipsr5900el-*-*)
>> > +  if test $with_float = hard; then
>> > +if test x$with_fpu = x; then
>> > +# The FPU of the R5900 is 32 bit.
>> > +with_fpu=single
>> > +fi
>> > +  fi
>> > +  ;;
>> > +  esac
>> >  fi
>>
>> I think the --with-fpu default should be independent of the --with-float
>> default.  Passing -mhard-float to the default soft-float configuration
>> should produce the same code as configuring with --with-float=hard.
>
> OK. I hoped to get more software working on the PS2 when the default is
> the same as on other mips64 systems with soft float.

The patch doesn't change that.  -msingle-float and -mdouble-float don't
affect the -msoft-float behaviour, just the -mhard-float behaviour.

Thanks,
Richard


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

2013-07-25 Thread Jürgen Urban
Hello Richard,

Sorry in the last days, I was not at home. So I couldn't test it until now.

> "Jürgen Urban"  writes:
> > Index: gcc/config.gcc
> > ===
> > --- gcc/config.gcc  (Revision 200583)
> > +++ gcc/config.gcc  (Arbeitskopie)
> > @@ -3080,7 +3080,7 @@
> >esac
> >  fi
> >
> > -# Infer a default setting for --with-float.
> > +# Infer a default setting for --with-float and --with-fpu.
> >  if test x$with_float = x; then
> >case ${target} in
> >  mips64r5900-*-* | mips64r5900el-*-* | mipsr5900-*-* | mipsr5900el-*-*)
> > @@ -3089,6 +3089,17 @@
> >with_float=soft
> >;;
> >esac
> > +else
> > +  case ${target} in
> > +mips64r5900-*-* | mips64r5900el-*-* | mipsr5900-*-* | mipsr5900el-*-*)
> > +  if test $with_float = hard; then
> > +if test x$with_fpu = x; then
> > + # The FPU of the R5900 is 32 bit.
> > + with_fpu=single
> > +fi
> > +  fi
> > +  ;;
> > +  esac
> >  fi
>
> I think the --with-fpu default should be independent of the --with-float
> default.  Passing -mhard-float to the default soft-float configuration
> should produce the same code as configuring with --with-float=hard.

OK. I hoped to get more software working on the PS2 when the default is the 
same as on other mips64 systems with soft float.

> > Index: gcc/config/mips/mips.c
> > ===
> > --- gcc/config/mips/mips.c  (Revision 200583)
> > +++ gcc/config/mips/mips.c  (Arbeitskopie)
> > @@ -16830,6 +16830,19 @@
> > target_flags &= ~MASK_FLOAT64;
> >  }
> >
> > +  if (TARGET_HARD_FLOAT_ABI && TARGET_FLOAT64 && TARGET_MIPS5900)
> > +{
> > +  /* FPU of r5900 only supports 32 bit. */
> > +  error ("unsupported combination: %s", "-march=r5900 -mfp64 
> > -mhard-float");
> > +}
> > +
> > +  if (TARGET_HARD_FLOAT_ABI && TARGET_DOUBLE_FLOAT && TARGET_MIPS5900)
> > +{
> > +  /* FPU of r5900 only supports 32 bit. */
> > +  error ("unsupported combination: %s",
> > + "-march=r5900 -mdouble-float -mhard-float");
> > +}
>
> Only one of these should be needed, since we complain about -mfp64
> -msingle-float earlier in the function.  Also, the coding conventions
> say that there should be no braces for single-statement if blocks.
>
> Here's what I installed.  Please let me know if I managed to mangle
> things somehow.
>

The patch is OK and tested.

Best regards
Jürgen


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

2013-07-07 Thread Richard Sandiford
"Jürgen Urban"  writes:
>> "Jürgen Urban"  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 0x7fff.

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


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

2013-07-07 Thread Jürgen Urban
Hello Richard,

> Gesendet: Sonntag, 07. Juli 2013 um 10:15 Uhr
> "Jürgen Urban"  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 0x7fff.

> 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