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 juergenur...@gmx.de 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-25 Thread Richard Sandiford 
Jürgen Urban juergenur...@gmx.de writes:
 Jürgen Urban juergenur...@gmx.de 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-07 Thread Jürgen Urban 
Hello Richard,

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

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

2013-07-07 Thread Richard Sandiford 
Jürgen Urban juergenur...@gmx.de writes:
 Jürgen Urban juergenur...@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 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