Possible long double bug fix (Problem Report sparc64/55773)

[Stephen Paskaluk]

On Thu, 23 Dec 2004 10:12:10 -0700, Stephen Paskaluk
<stephen.paskaluk at gmail.com> wrote:
> I think the sign check is probably sufficient, I'll try to put
> together and test a patch shortly but I'm busier today than I was
> yesterday.

Here's a patch for fpu_explode.c, as well I'm attaching the patch to fpu_qp.c

also over http:
http://dero.tamu.edu/~tyler/code/fpu_explode.diff
http://dero.tamu.edu/~tyler/code/fpu_qp.diff

These patches seem to correct three issues with int/long/uint/ulong
values to long double floating point representation.  The first issue
is the checking of bit 31 for sign on a 64 bit value instead of bit
63, which is solved by using sizeof to determine the proper bit to
check.  The second issue was always treating long and int values as
signed, even if they were unsigned, which was solved by adding a sign
argument to the _QP_TTOQ macro that generates the _Qp_uxtof() and
_Qp_uitof() functions.  This argument is checked before setting the
fp_sign member of fp structure.

The third problem (which is only a problem when the sign is actually
handled properly) is that the long double magnitude for longs or ints
was set with the assumption they were signed, whether they should have
been unsigned or not.  This was fixed by checking the fp_sign member
before negating the value.  This means that fp_sign *must* be set
before a call to __fpu_itof() or __fpu_xtof() (which is true in the
only uses of the functions I could find) and a comment has been place
in the functions noting the check.

These probably aren't the only problems in this code, I'm not sure
where the function __fpu_explode() is used but it doesn't do a proper
sign check either, but rudimentary testing on Tyler's Ultra2 running
-current shows good results.  All testing is welcome (in fact I
request it).

Cheers,

-- 
Stephen Paskaluk
-------------- next part --------------
--- /usr/src/lib/libc/sparc64/fpu/fpu_explode.c.orig	Sat May 11 16:20:04 2002
+++ /usr/src/lib/libc/sparc64/fpu/fpu_explode.c	Fri Dec 24 09:09:27 2004
@@ -101,7 +101,16 @@
 	 * fpu_norm()'s handling of `supernormals'; see fpu_subr.c.
 	 */
 	fp->fp_exp = FP_LG;
-	fp->fp_mant[0] = (int)i < 0 ? -i : i;
+	/*
+	 * The sign should be handled outside of this function and
+	 * the sign bit should be set appropriately.  The sign bit
+	 * can then be used to determine if a two's complement
+	 * should be performed when determining the magnitude.
+	 */
+	if (fp->fp_sign == 1)
+		fp->fp_mant[0] = (int) -i;
+	else
+		fp->fp_mant[0] = i;
 	fp->fp_mant[1] = 0;
 	fp->fp_mant[2] = 0;
 	fp->fp_mant[3] = 0;
@@ -127,7 +136,16 @@
 	 * fpu_norm()'s handling of `supernormals'; see fpu_subr.c.
 	 */
 	fp->fp_exp = FP_LG2;
-	*((int64_t*)fp->fp_mant) = (int64_t)i < 0 ? -i : i;
+	/*
+	 * The sign should be handled outside of this function and
+	 * the sign bit should be set appropriately.  The sign bit
+	 * can then be used to determine if a two's complement
+	 * should be performed when determining the magnitude.
+	 */
+	if (fp->fp_sign == 1)
+		*((int64_t*)fp->fp_mant) = (int64_t) -i;
+	else
+		*((u_int64_t*)fp->fp_mant) = i;
 	fp->fp_mant[2] = 0;
 	fp->fp_mant[3] = 0;
 	__fpu_norm(fp);
-------------- next part --------------
--- /usr/src/lib/libc/sparc64/fpu/fpu_qp.c.orig	Tue Mar 16 14:42:02 2004
+++ /usr/src/lib/libc/sparc64/fpu/fpu_qp.c	Fri Dec 24 00:24:10 2004
@@ -51,7 +51,7 @@
 	c[0] = __fpu_ftoq(&fe, r, c); \
 }
 
-#define	_QP_TTOQ(qname, fname, ntype, atype, ...) \
+#define	_QP_TTOQ(qname, fname, ntype, atype, signed, ...) \
 void _Qp_ ## qname ## toq(u_int *c, ntype n); \
 void \
 _Qp_ ## qname ## toq(u_int *c, ntype n) \
@@ -59,7 +59,12 @@
 	struct fpemu fe; \
 	union { atype a[2]; ntype n; } u = { .n = n }; \
 	__asm __volatile("stx %%fsr, %0" : "=m" (fe.fe_fsr) :); \
-	fe.fe_f1.fp_sign = u.a[0] >> 31; \
+	if (signed == 1){ \
+		fe.fe_f1.fp_sign = u.a[0] >> (sizeof(u.a[0]) * 8 - 1); \
+	} \
+        else{ \
+                fe.fe_f1.fp_sign = 0; \
+        } \
 	fe.fe_f1.fp_sticky = 0; \
 	fe.fe_f1.fp_class = __fpu_ ## fname ## tof(&fe.fe_f1, __VA_ARGS__); \
 	c[0] = __fpu_ftoq(&fe, &fe.fe_f1, c); \
@@ -123,12 +128,12 @@
 _QP_OP(mul)
 _QP_OP(sub)
 
-_QP_TTOQ(d,	d,	double,	u_int,	u.a[0], u.a[1])
-_QP_TTOQ(i,	i,	int,	u_int,	u.a[0])
-_QP_TTOQ(s,	s,	float,	u_int,	u.a[0])
-_QP_TTOQ(x,	x,	long,	u_long,	u.a[0])
-_QP_TTOQ(ui,	i,	u_int,	u_int,	u.a[0])
-_QP_TTOQ(ux,	x,	u_long,	u_long,	u.a[0])
+_QP_TTOQ(d,	d,	double,	u_int,	1, u.a[0], u.a[1])
+_QP_TTOQ(i,	i,	int,	u_int,	1, u.a[0])
+_QP_TTOQ(s,	s,	float,	u_int,  1, u.a[0])
+_QP_TTOQ(x,	x,	long,	u_long,	1, u.a[0])
+_QP_TTOQ(ui,	i,	u_int,	u_int,	0, u.a[0])
+_QP_TTOQ(ux,	x,	u_long,	u_long,	0, u.a[0])
 
 _QP_QTOT(d,	d,	double,	&u.a)
 _QP_QTOT(i,	i,	int)