diff mbox

powerpc: Fix inline feraiseexcept, feclearexcept macros

Message ID 54EB2D9B.20404@linux.vnet.ibm.com
State New
Headers show

Commit Message

Adhemerval Zanella Feb. 23, 2015, 1:39 p.m. UTC
On 11-02-2015 19:57, Joseph Myers wrote:
> On Mon, 9 Feb 2015, Adhemerval Zanella wrote:
>
>> Ping (with NEWS adjusted to 2.22).
>>
>>> Indeed, I have dropped the conditionals (I used test-fenv.c as base, but I think
>>> in this context they are not really required).
>>>
>>> Tested on powerpc64 with fix for BZ#17885 applied.
>>>
>>> --
>>>
>>> 	[BZ #17776]
>>> 	* sysdeps/powerpc/bits/fenvinline.h (feraiseexcept): Convert input to
>>> 	integer before bitwise and assembly operations.
>>> 	(feclearexcept): Likewise.
>>> 	* math/test-fenvinline.c: New file.
>>> 	* math/Makefile: Add test-fenvinline test.
> OK, though I think we could do for test coverage for the other functions 
> taking an integer argument (fetestexcept fesetround feenableexcept 
> fedisableexcept), to protect against this type of bug appearing for any 
> of them in future.
>
Right, I have added fetestexcept, fesetround, feenableexcept, and fedisableexcept
tests with integer and double arguments.  Did I miss something?

--

	[BZ #17776]
	* sysdeps/powerpc/bits/fenvinline.h (feraiseexcept): Convert input to
	integer before bitwise and assembly operations.
	(feclearexcept): Likewise.
	* math/test-fenvinline.c: New file.
	* math/Makefile: Add test-fenvinline test.

--

Comments

Joseph Myers Feb. 24, 2015, 5:37 p.m. UTC | #1
On Mon, 23 Feb 2015, Adhemerval Zanella wrote:

> +static void
> +test_fesetround (void)
> +{
> +#if defined FE_TONEAREST && defined FE_TOWARDZERO
> +  int res;
> +
> +  printf ("Tests for fesetround\n");
> +
> +  res = fesetround ((int) FE_TOWARDZERO);
> +  if (res != 0)
> +    {
> +      printf ("fesetround (FE_TOWARDZERO) failed: %d\n", res);
> +      ++count_errors;
> +    }

That fesetround fails should not itself cause the test to fail.  But it 
should either succeed for both int and double versions of the same 
argument value, or fail for both.
Adhemerval Zanella March 3, 2015, 3:02 p.m. UTC | #2
On 24-02-2015 14:37, Joseph Myers wrote:
> On Mon, 23 Feb 2015, Adhemerval Zanella wrote:
>
>> +static void
>> +test_fesetround (void)
>> +{
>> +#if defined FE_TONEAREST && defined FE_TOWARDZERO
>> +  int res;
>> +
>> +  printf ("Tests for fesetround\n");
>> +
>> +  res = fesetround ((int) FE_TOWARDZERO);
>> +  if (res != 0)
>> +    {
>> +      printf ("fesetround (FE_TOWARDZERO) failed: %d\n", res);
>> +      ++count_errors;
>> +    }
> That fesetround fails should not itself cause the test to fail.  But it 
> should either succeed for both int and double versions of the same 
> argument value, or fail for both.
>
Right, I have pushed the patch with this change. Thanks for the review!
diff mbox

Patch

diff --git a/NEWS b/NEWS
index 28ef45d..7d94b43 100644
--- a/NEWS
+++ b/NEWS
@@ -10,8 +10,8 @@  Version 2.22
 * The following bugs are resolved with this release:
 
   4719, 13064, 14094, 15319, 15467, 15790, 16560, 17269, 17569, 17588,
-  17792, 17912, 17932, 17944, 17949, 17964, 17965, 17967, 17969, 17978,
-  17987, 17991, 17996, 17998, 17999.
+  17776, 17792, 17912, 17932, 17944, 17949, 17964, 17965, 17967, 17969,
+  17978, 17987, 17991, 17996, 17998, 17999.
 
 * Character encoding and ctype tables were updated to Unicode 7.0.0, using
   new generator scripts contributed by Pravin Satpute and Mike FABIAN (Red
diff --git a/math/Makefile b/math/Makefile
index fec7627..3904e41 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -90,7 +90,8 @@  tests = test-matherr test-fenv atest-exp atest-sincos atest-exp2 basic-test \
 	test-misc test-fpucw test-fpucw-ieee tst-definitions test-tgmath \
 	test-tgmath-ret bug-nextafter bug-nexttoward bug-tgmath1 \
 	test-tgmath-int test-tgmath2 test-powl tst-CMPLX tst-CMPLX2 test-snan \
-	test-fenv-tls test-fenv-preserve test-fenv-return $(tests-static)
+	test-fenv-tls test-fenv-preserve test-fenv-return test-fenvinline \
+	$(tests-static)
 tests-static = test-fpucw-static test-fpucw-ieee-static
 # We do the `long double' tests only if this data type is available and
 # distinct from `double'.
diff --git a/math/test-fenvinline.c b/math/test-fenvinline.c
new file mode 100644
index 0000000..1d643f8
--- /dev/null
+++ b/math/test-fenvinline.c
@@ -0,0 +1,357 @@ 
+/* Test for fenv inline implementations.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _GNU_SOURCE
+# define _GNU_SOURCE
+#endif
+
+/* To make sure the fenv inline function are used.  */
+#undef __NO_MATH_INLINES
+
+#include <fenv.h>
+#include <stdio.h>
+#include <math-tests.h>
+
+/*
+  Since not all architectures might define all exceptions, we define
+  a private set and map accordingly.
+*/
+#define NO_EXC 0
+#define INEXACT_EXC 0x1
+#define DIVBYZERO_EXC 0x2
+#define UNDERFLOW_EXC 0x04
+#define OVERFLOW_EXC 0x08
+#define INVALID_EXC 0x10
+#define ALL_EXC \
+        (INEXACT_EXC | DIVBYZERO_EXC | UNDERFLOW_EXC | OVERFLOW_EXC | \
+         INVALID_EXC)
+static int count_errors;
+
+#if FE_ALL_EXCEPT
+static void
+test_single_exception_fp_int (int exception,
+			      int exc_flag,
+			      int fe_flag,
+			      const char *flag_name)
+{
+  if (exception & exc_flag)
+    {
+      if (fetestexcept (fe_flag))
+        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
+      else
+        {
+          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
+          ++count_errors;
+        }
+    }
+  else
+    {
+      if (fetestexcept (fe_flag))
+        {
+          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
+          ++count_errors;
+        }
+      else
+        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
+    }
+}
+/* Test whether a given exception was raised.  */
+static void
+test_single_exception_fp_double (int exception,
+				 int exc_flag,
+				 double fe_flag,
+				 const char *flag_name)
+{
+  if (exception & exc_flag)
+    {
+      if (fetestexcept (fe_flag))
+        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
+      else
+        {
+          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
+          ++count_errors;
+        }
+    }
+  else
+    {
+      if (fetestexcept (fe_flag))
+        {
+          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
+          ++count_errors;
+        }
+      else
+        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
+    }
+}
+#endif
+
+static void
+test_exceptions (const char *test_name, int exception)
+{
+  printf ("Test: %s\n", test_name);
+#ifdef FE_DIVBYZERO
+  test_single_exception_fp_double (exception, DIVBYZERO_EXC, FE_DIVBYZERO,
+				   "DIVBYZERO");
+#endif
+#ifdef FE_INVALID
+  test_single_exception_fp_double (exception, INVALID_EXC, FE_INVALID,
+				   "INVALID");
+#endif
+#ifdef FE_INEXACT
+  test_single_exception_fp_double (exception, INEXACT_EXC, FE_INEXACT,
+				   "INEXACT");
+#endif
+#ifdef FE_UNDERFLOW
+  test_single_exception_fp_double (exception, UNDERFLOW_EXC, FE_UNDERFLOW,
+				   "UNDERFLOW");
+#endif
+#ifdef FE_OVERFLOW
+  test_single_exception_fp_double (exception, OVERFLOW_EXC, FE_OVERFLOW,
+				   "OVERFLOW");
+#endif
+}
+
+static void
+test_exceptionflag (void)
+{
+  printf ("Test: fegetexceptionflag (FE_ALL_EXCEPT)\n");
+#if FE_ALL_EXCEPT
+  fexcept_t excepts;
+
+  feclearexcept (FE_ALL_EXCEPT);
+
+  feraiseexcept (FE_INVALID);
+  fegetexceptflag (&excepts, FE_ALL_EXCEPT);
+
+  feclearexcept (FE_ALL_EXCEPT);
+  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
+
+  fesetexceptflag (&excepts, FE_ALL_EXCEPT);
+
+  test_single_exception_fp_int (INVALID_EXC, INVALID_EXC, FE_INVALID,
+				"INVALID (int)");
+  test_single_exception_fp_int (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
+				"OVERFLOW (int)");
+  test_single_exception_fp_int (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
+				"INEXACT (int)");
+
+  /* Same test, but using double as argument  */
+  feclearexcept (FE_ALL_EXCEPT);
+
+  feraiseexcept (FE_INVALID);
+  fegetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
+
+  feclearexcept (FE_ALL_EXCEPT);
+  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
+
+  fesetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
+
+  test_single_exception_fp_double (INVALID_EXC, INVALID_EXC, FE_INVALID,
+				   "INVALID (double)");
+  test_single_exception_fp_double (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
+				   "OVERFLOW (double)");
+  test_single_exception_fp_double (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
+				   "INEXACT (double)");
+#endif
+}
+
+static void
+test_fesetround (void)
+{
+#if defined FE_TONEAREST && defined FE_TOWARDZERO
+  int res;
+
+  printf ("Tests for fesetround\n");
+
+  res = fesetround ((int) FE_TOWARDZERO);
+  if (res != 0)
+    {
+      printf ("fesetround (FE_TOWARDZERO) failed: %d\n", res);
+      ++count_errors;
+    }
+  res = fesetround ((int) FE_TONEAREST);
+  if (res != 0)
+    {
+      printf ("fesetround (FE_TONEAREST) failed: %d\n", res);
+      ++count_errors;
+    }
+
+  res = fesetround ((double) FE_TOWARDZERO);
+  if (res != 0)
+    {
+      printf ("fesetround (FE_TOWARDZERO) failed: %d\n", res);
+      ++count_errors;
+    }
+  res = fesetround ((double) FE_TONEAREST);
+  if (res != 0)
+    {
+      printf ("fesetround (FE_TONEAREST) failed: %d\n", res);
+      ++count_errors;
+    }
+#endif
+}
+
+/* Tests for feenableexcept/fedisableexcept.  */
+static void
+feenable_test (const char *flag_name, fexcept_t fe_exc)
+{
+#if FE_ALL_EXCEPT
+  int fe_exci = fe_exc;
+  double fe_excd = fe_exc;
+  int excepts;
+
+  /* First disable all exceptions.  */
+  if (fedisableexcept (FE_ALL_EXCEPT) == -1)
+    {
+      printf ("Test: fedisableexcept (FE_ALL_EXCEPT) failed\n");
+      ++count_errors;
+      /* If this fails, the other tests don't make sense.  */
+      return;
+    }
+
+  /* Test for inline macros using integer argument.  */
+  excepts = feenableexcept (fe_exci);
+  if (!EXCEPTION_ENABLE_SUPPORTED (fe_exci) && excepts == -1)
+    {
+      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
+      return;
+    }
+  if (excepts == -1)
+    {
+      printf ("Test: feenableexcept (%s) failed\n", flag_name);
+      ++count_errors;
+      return;
+    }
+  if (excepts != 0)
+    {
+      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
+              flag_name, excepts);
+      ++count_errors;
+    }
+
+  /* And now disable the exception again.  */
+  excepts = fedisableexcept (fe_exc);
+  if (excepts == -1)
+    {
+      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
+      ++count_errors;
+      return;
+    }
+  if (excepts != fe_exc)
+    {
+      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
+              flag_name, fe_exc, excepts);
+      ++count_errors;
+    }
+
+  /* Test for inline macros using double argument.  */
+  excepts = feenableexcept (fe_excd);
+  if (!EXCEPTION_ENABLE_SUPPORTED (fe_excd) && excepts == -1)
+    {
+      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
+      return;
+    }
+  if (excepts == -1)
+    {
+      printf ("Test: feenableexcept (%s) failed\n", flag_name);
+      ++count_errors;
+      return;
+    }
+  if (excepts != 0)
+    {
+      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
+              flag_name, excepts);
+      ++count_errors;
+    }
+
+  /* And now disable the exception again.  */
+  excepts = fedisableexcept (fe_exc);
+  if (excepts == -1)
+    {
+      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
+      ++count_errors;
+      return;
+    }
+  if (excepts != fe_exc)
+    {
+      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
+              flag_name, fe_exc, excepts);
+      ++count_errors;
+    }
+#endif
+}
+
+static void
+test_feenabledisable (void)
+{
+  printf ("Tests for feenableexcepts/fedisableexcept\n");
+
+  /* We might have some exceptions still set.  */
+  feclearexcept (FE_ALL_EXCEPT);
+
+#ifdef FE_DIVBYZERO
+  feenable_test ("FE_DIVBYZERO", FE_DIVBYZERO);
+#endif
+#ifdef FE_INVALID
+  feenable_test ("FE_INVALID", FE_INVALID);
+#endif
+#ifdef FE_INEXACT
+  feenable_test ("FE_INEXACT", FE_INEXACT);
+#endif
+#ifdef FE_UNDERFLOW
+  feenable_test ("FE_UNDERFLOW", FE_UNDERFLOW);
+#endif
+#ifdef FE_OVERFLOW
+  feenable_test ("FE_OVERFLOW", FE_OVERFLOW);
+#endif
+  fesetenv (FE_DFL_ENV);
+}
+
+static int
+do_test (void)
+{
+  /* clear all exceptions and test if all are cleared  */
+  feclearexcept (FE_ALL_EXCEPT);
+  test_exceptions ("feclearexcept (FE_ALL_EXCEPT) clears all exceptions",
+                   NO_EXC);
+
+  /* raise all exceptions and test if all are raised  */
+  feraiseexcept (FE_ALL_EXCEPT);
+  test_exceptions ("feraiseexcept (FE_ALL_EXCEPT) raises all exceptions",
+                   ALL_EXC);
+
+  /* Same test, but using double as argument  */
+  feclearexcept ((double)FE_ALL_EXCEPT);
+  test_exceptions ("feclearexcept ((double)FE_ALL_EXCEPT) clears all exceptions",
+                   NO_EXC);
+
+  feraiseexcept ((double)FE_ALL_EXCEPT);
+  test_exceptions ("feraiseexcept ((double)FE_ALL_EXCEPT) raises all exceptions",
+                   ALL_EXC);
+
+  test_exceptionflag ();
+
+  test_fesetround ();
+
+  test_feenabledisable ();
+
+  return count_errors;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
index 35c2114..894789e 100644
--- a/sysdeps/powerpc/bits/fenvinline.h
+++ b/sysdeps/powerpc/bits/fenvinline.h
@@ -34,29 +34,41 @@ 
 
 /* Inline definition for feraiseexcept.  */
 #  define feraiseexcept(__excepts) \
-  ((__builtin_constant_p (__excepts)					      \
-    && ((__excepts) & ((__excepts)-1)) == 0				      \
-    && (__excepts) != FE_INVALID)					      \
-   ? ((__excepts) != 0							      \
-      ? (__extension__ ({ __asm__ __volatile__				      \
-			  ("mtfsb1 %s0"					      \
-			   : : "i#*X"(__builtin_ffs (__excepts)));	      \
-			  0; }))					      \
-      : 0)								      \
-   : (feraiseexcept) (__excepts))
+  (__extension__  ({ 							      \
+    int __e = __excepts;						      \
+    int __ret;								      \
+    if (__builtin_constant_p (__e)					      \
+        && (__e & (__e - 1)) == 0					      \
+        && __e != FE_INVALID)						      \
+      {									      \
+	if (__e != 0)							      \
+	  __asm__ __volatile__ ("mtfsb1 %s0"				      \
+				: : "i#*X" (__builtin_ffs (__e)));	      \
+        __ret = 0;							      \
+      }									      \
+    else								      \
+      __ret = feraiseexcept (__e);					      \
+    __ret;								      \
+  }))
 
 /* Inline definition for feclearexcept.  */
 #  define feclearexcept(__excepts) \
-  ((__builtin_constant_p (__excepts)					      \
-    && ((__excepts) & ((__excepts)-1)) == 0				      \
-    && (__excepts) != FE_INVALID)					      \
-   ? ((__excepts) != 0							      \
-      ? (__extension__ ({ __asm__ __volatile__				      \
-			  ("mtfsb0 %s0"					      \
-			   : : "i#*X"(__builtin_ffs (__excepts)));	      \
-			  0; }))					      \
-      : 0)								      \
-   : (feclearexcept) (__excepts))
+  (__extension__  ({ 							      \
+    int __e = __excepts;						      \
+    int __ret;								      \
+    if (__builtin_constant_p (__e)					      \
+        && (__e & (__e - 1)) == 0					      \
+        && __e != FE_INVALID)						      \
+      {									      \
+	if (__e != 0)							      \
+	  __asm__ __volatile__ ("mtfsb0 %s0"				      \
+				: : "i#*X" (__builtin_ffs (__e)));	      \
+        __ret = 0;							      \
+      }									      \
+    else								      \
+      __ret = feclearexcept (__e);					      \
+    __ret;								      \
+  }))
 
 # endif /* !__NO_MATH_INLINES.  */