libgcc2 __fixsfdi

libgcc2 __fixsfdi

[Alan Modra]

gcc's __fixsfdi function returns extremely surprising results for
floats that overflow the range of DWtype.  The integer value
wraps rather than limiting to min/max values.  I know that return
values for the overflow case are unspecified, even in current
draft ieee-754, but "each of the operations shall be performed as if
it first produced an intermediate result correct to infinite precision
and with unbounded range, and then coerced this intermediate result to
fit in the destination's format" could be taken as support for this
patch.  After all, the nearest values that the intermediate result can
be coerced to in case of overflow, are the min and max integers.  Also
see http://754r.ucbtest.org/proposals/integer/integer.html where this
behaviour is specified.

If the following is reasonable, I'll do the same for __fixtfdi,
__fixxfdi and __fixdfdi, and submit a proper patch with changelog.

Index: gcc/libgcc2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/libgcc2.c,v
retrieving revision 1.149
diff -u -p -r1.149 libgcc2.c
--- gcc/libgcc2.c	8 Sep 2002 12:47:26 -0000	1.149
+++ gcc/libgcc2.c	4 Oct 2002 14:25:34 -0000
@@ -994,12 +994,27 @@ __fixunssfDI (SFtype original_a)
 #endif
 
 #ifdef L_fixsfdi
+#define DWORD_SIZE (sizeof (DWtype) * BITS_PER_UNIT)
+#define MAXDWtype ((((DWtype) 1) << (DWORD_SIZE-1))-1)
+
 DWtype
 __fixsfdi (SFtype a)
 {
+  DWtype tmp;
+
   if (a < 0)
-    return - __fixunssfDI (-a);
-  return __fixunssfDI (a);
+    {
+      tmp = __fixunssfDI (-a);
+      tmp = -tmp;
+      if (tmp > 0)
+	tmp = -MAXDWtype - 1;
+      return tmp;
+    }
+
+  tmp = __fixunssfDI (a);
+  if (tmp < 0)
+    tmp = MAXDWtype;
+  return tmp;
 }
 #endif
 

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: libgcc2 __fixsfdi

[Richard Henderson]

On Sat, Oct 05, 2002 at 12:22:44AM +0930, Alan Modra wrote:
> gcc's __fixsfdi function returns extremely surprising results for
> floats that overflow the range of DWtype.  The integer value
> wraps rather than limiting to min/max values.

Well, rather it crops to the unsigned max and then wraps.

Surprisingly, while implementing the new real.c, I found
that something in gcc relied on this.  I forget what,
exactly, but if you change real_to_integer (which returns
a signed result) to crop to the maximum signed integer
(rather than wrapping), you'll get some test suite failures.

I hadn't been interested in understanding why at the time.
But we should if we go making this sort of change, we should
be consistent between runtime and compiletime, which means
addressing this half-remembered failure.


r~

Re: libgcc2 __fixsfdi

[Alan Modra]

On Fri, Oct 04, 2002 at 12:00:42PM -0700, Richard Henderson wrote:
> On Sat, Oct 05, 2002 at 12:22:44AM +0930, Alan Modra wrote:
> > gcc's __fixsfdi function returns extremely surprising results for
> > floats that overflow the range of DWtype.  The integer value
> > wraps rather than limiting to min/max values.
> 
> Well, rather it crops to the unsigned max and then wraps.

Yes, and that's probably the strongest argument I have for float
-> int conversions to peg at signed max/min.  If float -> unsigned
peg at unsigned max, then it why not something similar for signed
conversions?

The following test gives some rather odd results for compile time
conversions on i686-linux.

int main (void)
{
#define BIGF 1.0e+30F

  printf ("%#llx, %#llx\n", (long long) -BIGF, (long long) BIGF);
  printf ("%#x, %#x\n", (int) -BIGF, (int) BIGF);
  return 0;
}

gcc-2.95.3
0x100000000, 0xffffffff00000000
0, 0

mainline without my patch
0x8000000000000000, 0x7fffffffffffffff
0, 0xffffffff

mainline with patch
0x8000000000000000, 0x7fffffffffffffff
0x80000000, 0x7fffffff

> Surprisingly, while implementing the new real.c, I found
> that something in gcc relied on this.  I forget what,
> exactly, but if you change real_to_integer (which returns
> a signed result) to crop to the maximum signed integer
> (rather than wrapping), you'll get some test suite failures.

Hmm, real_to_integer crops to max/min HOST_WIDE_INT.

> I hadn't been interested in understanding why at the time.
> But we should if we go making this sort of change, we should
> be consistent between runtime and compiletime, which means
> addressing this half-remembered failure.

OK, I've addressed the consistency issue by the patch to fold-const.c.
At least, this makes libgcc runtime consistent with compile time.  We
can't be consistent with everybody's fp hardware.  :-(  Bootstrapped
and regression tested i686-linux and powerpc-linux.

Also, a reminder that neither your or my fix to __floatdisf has
been applied yet..

	* fold-const.c (fold_convert): Overflowing float->int conversions
	clip to min/max int values.
	* libgcc2.c (__fixtfdi, __fixxfdi, __fixdfdi, __fixsfdi): Similarly.
	(__floatdisf): Properly cure double rounding.

Index: gcc/fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.222
diff -u -p -r1.222 fold-const.c
--- gcc/fold-const.c	22 Sep 2002 14:09:32 -0000	1.222
+++ gcc/fold-const.c	8 Oct 2002 04:57:27 -0000
@@ -1556,6 +1556,7 @@ fold_convert (t, arg1)
 	  REAL_VALUE_TYPE u;
 	  tree type1 = TREE_TYPE (arg1);
 	  int no_upper_bound;
+	  HOST_WIDE_INT low, high;
 
 	  x = TREE_REAL_CST (arg1);
 	  l = real_value_from_int_cst (type1, TYPE_MIN_VALUE (type));
@@ -1573,17 +1574,27 @@ fold_convert (t, arg1)
 	  /* If X is a NaN, use zero instead and show we have an overflow.
 	     Otherwise, range check.  */
 	  if (REAL_VALUE_ISNAN (x))
-	    overflow = 1, x = dconst0;
-	  else if (! (REAL_VALUES_LESS (l, x)
-		      && !no_upper_bound
-		      && REAL_VALUES_LESS (x, u)))
-	    overflow = 1;
-
-	  {
-	    HOST_WIDE_INT low, high;
+	    {
+	      overflow = 1;
+	      low = 0;
+	      high = 0;
+	    }
+	  else if (!REAL_VALUES_LESS (l, x))
+	    {
+	      overflow = 1;
+	      low = TREE_INT_CST_LOW (TYPE_MIN_VALUE (type));
+	      high = TREE_INT_CST_HIGH (TYPE_MIN_VALUE (type));
+	    }
+	  else if (!no_upper_bound && !REAL_VALUES_LESS (x, u))
+	    {
+	      overflow = 1;
+	      low = TREE_INT_CST_LOW (TYPE_MAX_VALUE (type));
+	      high = TREE_INT_CST_HIGH (TYPE_MAX_VALUE (type));
+	    }
+	  else
 	    REAL_VALUE_TO_INT (&low, &high, x);
-	    t = build_int_2 (low, high);
-	  }
+
+	  t = build_int_2 (low, high);
 	  TREE_TYPE (t) = type;
 	  TREE_OVERFLOW (t)
 	    = TREE_OVERFLOW (arg1) | force_fit_type (t, overflow);
Index: gcc/libgcc2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/libgcc2.c,v
retrieving revision 1.149
diff -u -p -r1.149 libgcc2.c
--- gcc/libgcc2.c	8 Sep 2002 12:47:26 -0000	1.149
+++ gcc/libgcc2.c	8 Oct 2002 04:57:29 -0000
@@ -864,12 +864,27 @@ __fixunstfDI (TFtype a)
 #endif
 
 #if defined(L_fixtfdi) && (LIBGCC2_LONG_DOUBLE_TYPE_SIZE == 128)
+#define DWORD_SIZE (sizeof (DWtype) * BITS_PER_UNIT)
+#define MAXDWtype ((DWtype) (((UDWtype) 1 << (DWORD_SIZE - 1)) - 1))
+
 DWtype
 __fixtfdi (TFtype a)
 {
+  DWtype tmp;
+
   if (a < 0)
-    return - __fixunstfDI (-a);
-  return __fixunstfDI (a);
+    {
+      tmp = __fixunstfDI (-a);
+      tmp = -tmp;
+      if (tmp > 0)
+	tmp = -MAXDWtype - 1;
+      return tmp;
+    }
+
+  tmp = __fixunstfDI (a);
+  if (tmp < 0)
+    tmp = MAXDWtype;
+  return tmp;
 }
 #endif
 
@@ -906,12 +921,27 @@ __fixunsxfDI (XFtype a)
 #endif
 
 #if defined(L_fixxfdi) && (LIBGCC2_LONG_DOUBLE_TYPE_SIZE == 96)
+#define DWORD_SIZE (sizeof (DWtype) * BITS_PER_UNIT)
+#define MAXDWtype ((DWtype) (((UDWtype) 1 << (DWORD_SIZE - 1)) - 1))
+
 DWtype
 __fixxfdi (XFtype a)
 {
+  DWtype tmp;
+
   if (a < 0)
-    return - __fixunsxfDI (-a);
-  return __fixunsxfDI (a);
+    {
+      tmp = __fixunsxfDI (-a);
+      tmp = -tmp;
+      if (tmp > 0)
+	tmp = -MAXDWtype - 1;
+      return tmp;
+    }
+
+  tmp = __fixunsxfDI (a);
+  if (tmp < 0)
+    tmp = MAXDWtype;
+  return tmp;
 }
 #endif
 
@@ -948,12 +978,27 @@ __fixunsdfDI (DFtype a)
 #endif
 
 #ifdef L_fixdfdi
+#define DWORD_SIZE (sizeof (DWtype) * BITS_PER_UNIT)
+#define MAXDWtype ((DWtype) (((UDWtype) 1 << (DWORD_SIZE - 1)) - 1))
+
 DWtype
 __fixdfdi (DFtype a)
 {
+  DWtype tmp;
+
   if (a < 0)
-    return - __fixunsdfDI (-a);
-  return __fixunsdfDI (a);
+    {
+      tmp = __fixunsdfDI (-a);
+      tmp = -tmp;
+      if (tmp > 0)
+	tmp = -MAXDWtype - 1;
+      return tmp;
+    }
+
+  tmp = __fixunsdfDI (a);
+  if (tmp < 0)
+    tmp = MAXDWtype;
+  return tmp;
 }
 #endif
 
@@ -994,12 +1039,27 @@ __fixunssfDI (SFtype original_a)
 #endif
 
 #ifdef L_fixsfdi
+#define DWORD_SIZE (sizeof (DWtype) * BITS_PER_UNIT)
+#define MAXDWtype ((DWtype) (((UDWtype) 1 << (DWORD_SIZE - 1)) - 1))
+
 DWtype
 __fixsfdi (SFtype a)
 {
+  DWtype tmp;
+
   if (a < 0)
-    return - __fixunssfDI (-a);
-  return __fixunssfDI (a);
+    {
+      tmp = __fixunssfDI (-a);
+      tmp = -tmp;
+      if (tmp > 0)
+	tmp = -MAXDWtype - 1;
+      return tmp;
+    }
+
+  tmp = __fixunssfDI (a);
+  if (tmp < 0)
+    tmp = MAXDWtype;
+  return tmp;
 }
 #endif
 
@@ -1091,7 +1151,10 @@ __floatdisf (DWtype u)
 	     && u < ((DWtype) 1 << DF_SIZE)))
 	{
 	  if ((UDWtype) u & (REP_BIT - 1))
-	    u |= REP_BIT;
+	    {
+	      u &= ~ (REP_BIT - 1);
+	      u |= REP_BIT;
+	    }
 	}
     }
   f = (Wtype) (u >> WORD_SIZE);

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: libgcc2 __fixsfdi

[Richard Henderson]

On Tue, Oct 08, 2002 at 05:12:13PM +0930, Alan Modra wrote:
> Yes, and that's probably the strongest argument I have for float
> -> int conversions to peg at signed max/min.  If float -> unsigned
> peg at unsigned max, then it why not something similar for signed
> conversions?

I dunno.  I'm of two minds here.  On the one hand, pegging at
particular values makes mathematic sense.  On the other, 
overflow is undefined (at least in C, C++, and Fortran), so
there's little point spending any energy on it.  You're 
increasing the code size in libgcc for no apparent gain.

Anyone else out there have an opinion?

> Also, a reminder that neither your or my fix to __floatdisf has
> been applied yet..

Oh, right.  Let's go with yours for now, since it uses fewer
operations.



r~