diff mbox

S/390: Throw FE_INVALID exception in the fp2int libgcc routines

Message ID 20140130145746.GA30339@bart
State New
Headers show

Commit Message

Andreas Krebbel Jan. 30, 2014, 2:57 p.m. UTC 
Hi,

our IEEE float to int conversion routines in libgcc do not throw the
FE_INVALID exceptions according to C99 Annex F.

Fixed with the attached patch.
This fixes the Glibc test-ldouble tests on s390:
< Failure: llrint (inf): Exception "Invalid operation" not set
< Failure: llrint (-inf): Exception "Invalid operation" not set
< Failure: llrint (qNaN): Exception "Invalid operation" not set
< Failure: llrint_tonearest (inf): Exception "Invalid operation" not set
< Failure: llrint_tonearest (-inf): Exception "Invalid operation" not set
< Failure: llrint_tonearest (qNaN): Exception "Invalid operation" not set
< Failure: llrint_towardzero (inf): Exception "Invalid operation" not set
< Failure: llrint_towardzero (-inf): Exception "Invalid operation" not set
< Failure: llrint_towardzero (qNaN): Exception "Invalid operation" not set
< Failure: llrint_downward (inf): Exception "Invalid operation" not set
< Failure: llrint_downward (-inf): Exception "Invalid operation" not set
< Failure: llrint_downward (qNaN): Exception "Invalid operation" not set
< Failure: llrint_upward (inf): Exception "Invalid operation" not set
< Failure: llrint_upward (-inf): Exception "Invalid operation" not set
< Failure: llrint_upward (qNaN): Exception "Invalid operation" not set
< Failure: llround (inf): Exception "Invalid operation" not set
< Failure: llround (-inf): Exception "Invalid operation" not set
< Failure: llround (qNaN): Exception "Invalid operation" not set

Glibc BZ: https://sourceware.org/bugzilla/show_bug.cgi?id=16449

I'll commit the patch after waiting a few days for comments.

Bye,

-Andreas-


2014-01-30  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>

	* config/s390/32/_fixdfdi.c: Throw invalid exception if number
	cannot be represented.
	* config/s390/32/_fixsfdi.c: Likewise.
	* config/s390/32/_fixtfdi.c: Likewise.
	* config/s390/32/_fixunsdfdi.c: Likewise.
	* config/s390/32/_fixunssfdi.c: Likewise.
	* config/s390/32/_fixunstfdi.c: Likewise.

commit da26827014daa447835f41bf07f3e3eb3ffe0c75
Author: Andreas Krebbel <krebbel@linux.vnet.ibm.com>
Date:   Wed Jan 29 10:13:51 2014 +0100

    S/390: libgcc: Throw INVALID exception for float->int conversions to
    implement C99 Annex F.4.

Comments

Joseph Myers Jan. 30, 2014, 5:47 p.m. UTC | #1 
On Thu, 30 Jan 2014, Andreas Krebbel wrote:

> @@ -81,6 +91,8 @@ __fixdfdi (double a1)
>  	l = (long long)1<<63;
>  	if (!SIGND(dl1))
>  	    l--;
> +	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
> +	fexceptdiv (0.0, 0.0);
>  	return l;

My understanding of this code is that the condition exp >= 11 means a 
number with magnitude at least 0x1p63.  If the number is exactly -0x1p63, 
then the result of the most negative "long long" value should not involve 
the "invalid" exception.

> @@ -78,6 +88,8 @@ __fixsfdi (float a1)
>  	l = (long long)1<<63;
>  	if (!SIGN(fl1))
>  	    l--;
> +	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
> +	fexceptdiv (0.0, 0.0);
>  	return l;

Likewise.

> @@ -90,6 +100,8 @@ __fixtfdi (long double a1)
>         or more.  */
>      if (exp >= 0)
>        {
> +	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
> +	fexceptdiv (0.0, 0.0);
>  	l = 1ULL << 63; /* long long min */
>  	return SIGND (dl1) ? l : l - 1;

Likewise, except in this case there are also values whose integer part is 
LLONG_MIN but whose fractional part is nonzero, for which "invalid" should 
also not be raised.

(This complication only arises for conversion to signed integers, not for 
conversion to unsigned.  But for the conversions to unsigned, you need to 
raise "invalid" for arguments <= -1, which your patch doesn't seem to do.)
diff mbox

Patch

diff --git a/libgcc/config/s390/32/_fixdfdi.c b/libgcc/config/s390/32/_fixdfdi.c
index 1c6954c..7178c01 100644
--- a/libgcc/config/s390/32/_fixdfdi.c
+++ b/libgcc/config/s390/32/_fixdfdi.c
@@ -46,6 +46,12 @@  union double_long {
     UDItype_x ll;
 };
 
+static __inline__ void
+fexceptdiv (float d, float e)
+{
+  __asm__ __volatile__ ("debr %0,%1" : : "f" (d), "f" (e) );
+}
+
 DItype_x __fixdfdi (double a1);
 
 /* convert double to int */
@@ -73,7 +79,11 @@  __fixdfdi (double a1)
     /* NaN */
 
     if ((EXPD(dl1) == 0x7ff) && (FRACD_LL(dl1) != 0)) /* NaN */
-      return 0x8000000000000000ULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0x8000000000000000ULL;
+      }
 
     /* Number big number & +/- inf */
 
@@ -81,6 +91,8 @@  __fixdfdi (double a1)
 	l = (long long)1<<63;
 	if (!SIGND(dl1))
 	    l--;
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
 	return l;
     }
 
diff --git a/libgcc/config/s390/32/_fixsfdi.c b/libgcc/config/s390/32/_fixsfdi.c
index eaabd18..cea3b84 100644
--- a/libgcc/config/s390/32/_fixsfdi.c
+++ b/libgcc/config/s390/32/_fixsfdi.c
@@ -43,6 +43,12 @@  union float_long
     USItype_x l;
   };
 
+static __inline__ void
+fexceptdiv (float d, float e)
+{
+  __asm__ __volatile__ ("debr %0,%1" : : "f" (d), "f" (e) );
+}
+
 DItype_x __fixsfdi (float a1);
 
 /* convert double to int */
@@ -70,7 +76,11 @@  __fixsfdi (float a1)
     /* NaN */
 
     if ((EXP(fl1) == 0xff) && (FRAC(fl1) != 0)) /* NaN */
-      return 0x8000000000000000ULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0x8000000000000000ULL;
+      }
 
     /* Number big number & +/- inf */
 
@@ -78,6 +88,8 @@  __fixsfdi (float a1)
 	l = (long long)1<<63;
 	if (!SIGN(fl1))
 	    l--;
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
 	return l;
     }
 
diff --git a/libgcc/config/s390/32/_fixtfdi.c b/libgcc/config/s390/32/_fixtfdi.c
index 51807d1..d17c516 100644
--- a/libgcc/config/s390/32/_fixtfdi.c
+++ b/libgcc/config/s390/32/_fixtfdi.c
@@ -50,6 +50,12 @@  union double_long {
   UDItype_x ll[2];   /* 64 bit parts: 0 upper, 1 lower */
 };
 
+static __inline__ void
+fexceptdiv (float d, float e)
+{
+  __asm__ __volatile__ ("debr %0,%1" : : "f" (d), "f" (e) );
+}
+
 DItype_x __fixtfdi (long double a1);
 
 /* convert double to unsigned int */
@@ -77,7 +83,11 @@  __fixtfdi (long double a1)
 
     /* NaN: All exponent bits set and a nonzero fraction.  */
     if ((EXPD(dl1) == 0x7fff) && !FRACD_ZERO_P (dl1))
-      return 0x8000000000000000ULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0x8000000000000000ULL;
+      }
 
     /* One extra bit is needed for the unit bit which is appended by
        MANTD_HIGH_LL on the left of the matissa.  */
@@ -90,6 +100,8 @@  __fixtfdi (long double a1)
        or more.  */
     if (exp >= 0)
       {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
 	l = 1ULL << 63; /* long long min */
 	return SIGND (dl1) ? l : l - 1;
       }
diff --git a/libgcc/config/s390/32/_fixunsdfdi.c b/libgcc/config/s390/32/_fixunsdfdi.c
index 7786bdc..4179b3f 100644
--- a/libgcc/config/s390/32/_fixunsdfdi.c
+++ b/libgcc/config/s390/32/_fixunsdfdi.c
@@ -46,6 +46,12 @@  union double_long {
     UDItype_x ll;
 };
 
+static __inline__ void
+fexceptdiv (float d, float e)
+{
+  __asm__ __volatile__ ("debr %0,%1" : : "f" (d), "f" (e) );
+}
+
 UDItype_x __fixunsdfdi (double a1);
 
 /* convert double to unsigned int */
@@ -73,13 +79,20 @@  __fixunsdfdi (double a1)
     /* NaN */
 
     if ((EXPD(dl1) == 0x7ff) && (FRACD_LL(dl1) != 0)) /* NaN */
-      return 0x0ULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0x0ULL;
+      }
 
     /* Number big number & + inf */
 
-    if (exp >= 12) {
-      return 0xFFFFFFFFFFFFFFFFULL;
-    }
+    if (exp >= 12)
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0xFFFFFFFFFFFFFFFFULL;
+      }
 
     l = MANTD_LL(dl1);
 
diff --git a/libgcc/config/s390/32/_fixunssfdi.c b/libgcc/config/s390/32/_fixunssfdi.c
index b007cf6..9dd3154 100644
--- a/libgcc/config/s390/32/_fixunssfdi.c
+++ b/libgcc/config/s390/32/_fixunssfdi.c
@@ -43,6 +43,12 @@  union float_long
     USItype_x l;
   };
 
+static __inline__ void
+fexceptdiv (float d, float e)
+{
+  __asm__ __volatile__ ("debr %0,%1" : : "f" (d), "f" (e) );
+}
+
 UDItype_x __fixunssfdi (float a1);
 
 /* convert float to unsigned int */
@@ -70,13 +76,20 @@  __fixunssfdi (float a1)
     /* NaN */
 
     if ((EXP(fl1) == 0xff) && (FRAC(fl1) != 0)) /* NaN */
-      return 0x0ULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);	
+	return 0x0ULL;
+      }
 
     /* Number big number & + inf */
 
-    if (exp >= 41) {
-      return 0xFFFFFFFFFFFFFFFFULL;
-    }
+    if (exp >= 41)
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0xFFFFFFFFFFFFFFFFULL;
+      }
 
     l = MANT(fl1);
 
diff --git a/libgcc/config/s390/32/_fixunstfdi.c b/libgcc/config/s390/32/_fixunstfdi.c
index cf4fbdb..00c22bf 100644
--- a/libgcc/config/s390/32/_fixunstfdi.c
+++ b/libgcc/config/s390/32/_fixunstfdi.c
@@ -50,6 +50,12 @@  union double_long {
   UDItype_x ll[2];   /* 64 bit parts: 0 upper, 1 lower */
 };
 
+static __inline__ void
+fexceptdiv (float d, float e)
+{
+  __asm__ __volatile__ ("debr %0,%1" : : "f" (d), "f" (e) );
+}
+
 UDItype_x __fixunstfdi (long double a1);
 
 /* convert double to unsigned int */
@@ -78,16 +84,25 @@  __fixunstfdi (long double a1)
 
     /* NaN: All exponent bits set and a nonzero fraction.  */
     if ((EXPD(dl1) == 0x7fff) && !FRACD_ZERO_P (dl1))
-      return 0x0ULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0x0ULL;
+      }
 
     /* One extra bit is needed for the unit bit which is appended by
        MANTD_HIGH_LL on the left of the matissa.  */
     exp += HIGH_LL_FRAC_BITS + 1;
 
-    /* If the result would still need a left shift it will be too large
-       to be represented.  */
+    /* If the result would still need a left shift it will be too
+       large to be represented.  Infinities have all exponent bits set
+       and will end up here as well.  */
     if (exp > 0)
-      return 0xFFFFFFFFFFFFFFFFULL;
+      {
+	/* C99 Annex F.4 requires an "invalid" exception to be thrown.  */
+	fexceptdiv (0.0, 0.0);
+	return 0xFFFFFFFFFFFFFFFFULL;
+      }
 
     l = MANTD_LOW_LL (dl1) >> (HIGH_LL_FRAC_BITS + 1)
         | MANTD_HIGH_LL (dl1) << (64 - (HIGH_LL_FRAC_BITS + 1));