[PATCH] libgcc/m68k: Fixes for soft float

[PATCH] libgcc/m68k: Fixes for soft float

[Keith Packard]

Check for non-zero denorm in __adddf3. Need to check both the upper and
lower 32-bit chunks of a 64-bit float for a non-zero value when
checking to see if the value is -0.

Fix __addsf3 when the sum exponent is exactly 0xff to ensure that
produces infinity and not nan.

Handle converting NaN/inf values between formats.

Handle underflow and overflow when truncating.

Write a replacement for __fixxfsi so that it does not raise extra
exceptions during an extra conversion from long double to double.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 libgcc/config/m68k/fpgnulib.c | 161 +++++++++++++++++++++++++++-------
 libgcc/config/m68k/lb1sf68.S  |   7 +-
 2 files changed, 134 insertions(+), 34 deletions(-)

diff --git a/libgcc/config/m68k/fpgnulib.c b/libgcc/config/m68k/fpgnulib.c
index fe41edf26aa..5b53778e986 100644
--- a/libgcc/config/m68k/fpgnulib.c
+++ b/libgcc/config/m68k/fpgnulib.c
@@ -54,6 +54,7 @@
 #define SIGNBIT		0x80000000L
 #define HIDDEN		(1L << 23L)
 #define SIGN(fp)	((fp) & SIGNBIT)
+#define EXPMASK		0xFFL
 #define EXP(fp)		(((fp) >> 23L) & 0xFF)
 #define MANT(fp)	(((fp) & 0x7FFFFFL) | HIDDEN)
 #define PACK(s,e,m)	((s) | ((e) << 23L) | (m))
@@ -262,6 +263,9 @@ __extendsfdf2 (float a1)
       mant &= ~HIDDEN;
     }
   exp = exp - EXCESS + EXCESSD;
+  /* Handle inf and NaN */
+  if (exp == EXPMASK - EXCESS + EXCESSD)
+    exp = EXPDMASK;
   dl.l.upper |= exp << 20;
   dl.l.upper |= mant >> 3;
   dl.l.lower = mant << 29;
@@ -295,40 +299,52 @@ __truncdfsf2 (double a1)
   /* shift double mantissa 6 bits so we can round */
   sticky |= mant & ((1 << 6) - 1);
   mant >>= 6;
-
-  /* Check for underflow and denormals.  */
-  if (exp <= 0)
+  if (exp == EXPDMASK - EXCESSD + EXCESS)
+    {
+      exp = EXPMASK;
+      mant = mant >> 1 | (mant & 1) | !!sticky;
+    }
+  else
     {
-      if (exp < -24)
+      /* Check for underflow and denormals.  */
+      if (exp <= 0)
 	{
-	  sticky |= mant;
-	  mant = 0;
+	  if (exp < -24)
+	    {
+	      sticky |= mant;
+	      mant = 0;
+	    }
+	  else
+	    {
+	      sticky |= mant & ((1 << (1 - exp)) - 1);
+	      mant >>= 1 - exp;
+	    }
+	  exp = 0;
 	}
-      else
+
+      /* now round */
+      shift = 1;
+      if ((mant & 1) && (sticky || (mant & 2)))
 	{
-	  sticky |= mant & ((1 << (1 - exp)) - 1);
-	  mant >>= 1 - exp;
-	}
-      exp = 0;
-    }
-  
-  /* now round */
-  shift = 1;
-  if ((mant & 1) && (sticky || (mant & 2)))
-    {
-      int rounding = exp ? 2 : 1;
+	  int rounding = exp ? 2 : 1;
 
-      mant += 1;
+	  mant += 1;
 
-      /* did the round overflow? */
-      if (mant >= (HIDDEN << rounding))
+	  /* did the round overflow? */
+	  if (mant >= (HIDDEN << rounding))
+	    {
+	      exp++;
+	      shift = rounding;
+	    }
+	}
+      /* shift down */
+      mant >>= shift;
+      if (exp >= EXPMASK)
 	{
-	  exp++;
-	  shift = rounding;
+	  exp = EXPMASK;
+	  mant = 0;
 	}
     }
-  /* shift down */
-  mant >>= shift;
 
   mant &= ~HIDDEN;
 
@@ -432,6 +448,30 @@ __extenddfxf2 (double d)
     }
 
   exp = EXPD (dl) - EXCESSD + EXCESSX;
+  /* Check for underflow and denormals. */
+  if (exp < 0)
+    {
+      if (exp < -53)
+        {
+	  ldl.l.middle = 0;
+	  ldl.l.lower = 0;
+	}
+      else if (exp < -30)
+        {
+	  ldl.l.lower = (ldl.l.middle & MANTXMASK) >> ((1 - exp) - 32);
+	  ldl.l.middle &= ~MANTXMASK;
+	}
+      else
+        {
+	  ldl.l.lower >>= 1 - exp;
+	  ldl.l.lower |= (ldl.l.middle & MANTXMASK) << (32 - (1 - exp));
+	  ldl.l.middle = (ldl.l.middle & ~MANTXMASK) | (ldl.l.middle & MANTXMASK >> (1 - exp));
+	}
+      exp = 0;
+    }
+  /* Handle inf and NaN */
+  if (exp == EXPDMASK - EXCESSD + EXCESSX)
+    exp = EXPXMASK;
   ldl.l.upper |= exp << 16;
   ldl.l.middle = HIDDENX;
   /* 31-20: # mantissa bits in ldl.l.middle - # mantissa bits in dl.l.upper */
@@ -464,9 +504,38 @@ __truncxfdf2 (long double ld)
     }
 
   exp = EXPX (ldl) - EXCESSX + EXCESSD;
-  /* ??? quick and dirty: keep `exp' sane */
-  if (exp >= EXPDMASK)
-    exp = EXPDMASK - 1;
+  /* Check for underflow and denormals. */
+  if (exp <= 0)
+    {
+      if (exp < -53)
+        {
+	  ldl.l.middle = 0;
+	  ldl.l.lower = 0;
+	}
+      else if (exp < -30)
+        {
+	  ldl.l.lower = (ldl.l.middle & MANTXMASK) >> ((1 - exp) - 32);
+	  ldl.l.middle &= ~MANTXMASK;
+	}
+      else
+        {
+	  ldl.l.lower >>= 1 - exp;
+	  ldl.l.lower |= (ldl.l.middle & MANTXMASK) << (32 - (1 - exp));
+	  ldl.l.middle = (ldl.l.middle & ~MANTXMASK) | (ldl.l.middle & MANTXMASK >> (1 - exp));
+	}
+      exp = 0;
+    }
+  else if (exp == EXPXMASK - EXCESSX + EXCESSD)
+    {
+      exp = EXPDMASK;
+      ldl.l.middle |= ldl.l.lower;
+    }
+  else if (exp >= EXPDMASK)
+    {
+      exp = EXPDMASK;
+      ldl.l.middle = 0;
+      ldl.l.lower = 0;
+    }
   dl.l.upper |= exp << (32 - (EXPDBITS + 1));
   /* +1-1: add one for sign bit, but take one off for explicit-integer-bit */
   dl.l.upper |= (ldl.l.middle & MANTXMASK) >> (EXPDBITS + 1 - 1);
@@ -511,10 +580,40 @@ __floatunsixf (unsigned long l)
 
 /* convert a long double to an int */
 long
-__fixxfsi (long double ld)
+__fixxfsi (long double a)
 {
-  long foo = __fixdfsi ((double) ld);
-  return foo;
+  union long_double_long ldl;
+  long exp;
+  long l;
+
+  ldl.ld = a;
+
+  exp = EXPX(ldl);
+  if (exp == 0 && ldl.l.middle == 0 && ldl.l.lower == 0)
+    return 0;
+
+  exp = exp - EXCESSX - 63;
+
+  if (exp > 0) 
+    {
+      /* Return largest integer.  */
+      return SIGNX (ldl) ? 0x80000000L : 0x7fffffffL;
+    }
+
+  if (exp <= -64)
+    return 0;
+
+  if (exp <= -32)
+    {
+      ldl.l.lower = ldl.l.middle >> (-exp - 32);
+    }
+  else if (exp < 0)
+    {
+      ldl.l.lower = ldl.l.lower >> -exp;
+      ldl.l.lower |= ldl.l.middle << (32 + exp);
+    }
+
+  return SIGNX(ldl) ? -ldl.l.lower : ldl.l.lower;
 }
 
 /* The remaining provide crude math support by working in double precision.  */
diff --git a/libgcc/config/m68k/lb1sf68.S b/libgcc/config/m68k/lb1sf68.S
index 8ba85c53656..736a9a7872f 100644
--- a/libgcc/config/m68k/lb1sf68.S
+++ b/libgcc/config/m68k/lb1sf68.S
@@ -1383,6 +1383,8 @@ Ladddf$a:
 	bge	2f			|
 	movel	d0,d0           	| check for zero, since we don't  '
 	bne	Ladddf$ret		| want to return -0 by mistake
+	movel	d1,d1			|
+	bne	Ladddf$ret		|
 	bclr	IMM (31),d7		|
 	bra	Ladddf$ret		|
 2:
@@ -2090,8 +2092,7 @@ Ldivdf$a$nf:
 | If a is INFINITY we have to check b
 	cmpl	d7,d2		| compare b with INFINITY 
 	bge	Ld$inop		| if b is NaN or INFINITY return NaN
-	tstl	d3		|
-	bne	Ld$inop		| 
+	movl	a0,d7		| restore sign bit to d7
 	bra	Ld$overflow	| else return overflow
 
 | If a number is denormalized we put an exponent of 1 but do not put the 
@@ -2936,7 +2937,7 @@ Laddsf$4:
 #else
 	cmpl	IMM (0xff),d2
 #endif
-	bhi	1f
+	bge	1f
 	bclr	IMM (FLT_MANT_DIG-1),d0
 #ifndef __mcoldfire__
 	lslw	IMM (7),d2
-- 
2.40.1

Re: [PATCH] libgcc/m68k: Fixes for soft float

[Jeff Law]

On 8/22/23 20:15, Keith Packard via Gcc-patches wrote:
> Check for non-zero denorm in __adddf3. Need to check both the upper and
> lower 32-bit chunks of a 64-bit float for a non-zero value when
> checking to see if the value is -0.
> 
> Fix __addsf3 when the sum exponent is exactly 0xff to ensure that
> produces infinity and not nan.
> 
> Handle converting NaN/inf values between formats.
> 
> Handle underflow and overflow when truncating.
> 
> Write a replacement for __fixxfsi so that it does not raise extra
> exceptions during an extra conversion from long double to double.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
I pushed this to the trunk after fixing a few minor whitespace nits. 
You didn't mention the divdf change, but I'll assume that was just an 
oversight.

I'm largely trusting your reputation on the fpgnulib changes.  I won't 
claim to know that code at all.  The assembly bits were simple enough 
that I could make out what you were doing relatively easily.

Thanks again,
Jeff

Re: [PATCH] libgcc/m68k: Fixes for soft float

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]


> I pushed this to the trunk after fixing a few minor whitespace nits. 
> You didn't mention the divdf change, but I'll assume that was just an 
> oversight.

Yeah, a couple of minor fixes there that I forgot to mention in the log.

> I'm largely trusting your reputation on the fpgnulib changes.  I won't 
> claim to know that code at all.  The assembly bits were simple enough 
> that I could make out what you were doing relatively easily.

Thanks for that review -- m68k assembly isn't my strongest language. The
kludge to return pointers in both d1 and a1 was a bit ugly, but seemed
like a much more robust plan than attempting to use different registers
depending on the target ABI...

The real check for these fixes was to run a fairly comprehensive C
library test suite (part of picolibc) and just iterate until I stopped
getting failures. Those tests have found so many corner cases in both
the C library, FPU emulation and compilers ...

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] libgcc/m68k: Fixes for soft float

[Jeff Law]

On 11/10/23 18:02, Keith Packard wrote:

> 
>> I'm largely trusting your reputation on the fpgnulib changes.  I won't
>> claim to know that code at all.  The assembly bits were simple enough
>> that I could make out what you were doing relatively easily.
> 
> Thanks for that review -- m68k assembly isn't my strongest language. The
> kludge to return pointers in both d1 and a1 was a bit ugly, but seemed
> like a much more robust plan than attempting to use different registers
> depending on the target ABI...
I get by.  Things like m68k, hppa, etc have stuck harder than I would 
have ever imagined.  But ppc, alpha, and some of the others I've worked 
on through the years, not so much.

And yes there are ABI implications on the return value.  As you know 
back in the day things weren't typed so well, so varying the location of 
the return value based on type wasn't terribly safe.  Dual output worked 
around a class of problems.  These days varying args/return value 
location based on types is common.


> 
> The real check for these fixes was to run a fairly comprehensive C
> library test suite (part of picolibc) and just iterate until I stopped
> getting failures. Those tests have found so many corner cases in both
> the C library, FPU emulation and compilers ...
Not surprised.

Thanks again,
Jeff