This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: soft-fp: Remove FP_CLEAR_EXCEPTIONS
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: "Joseph S. Myers" <joseph at codesourcery dot com>, libc-alpha at sourceware dot org
- Cc: "David S. Miller" <davem at davemloft dot net>
- Date: Wed, 08 Oct 2014 19:41:38 -0400
- Subject: Re: soft-fp: Remove FP_CLEAR_EXCEPTIONS
- Authentication-results: sourceware.org; auth=none
- References: <Pine dot LNX dot 4 dot 64 dot 1409162328380 dot 4182 at digraph dot polyomino dot org dot uk>
On 09/16/2014 07:31 PM, Joseph S. Myers wrote:
> As noted in
> <https://sourceware.org/ml/libc-alpha/2013-10/msg00516.html>, the
> soft-fp macro FP_CLEAR_EXCEPTIONS should not be necessary, as soft-fp
> code should never set an exception and later clear it.
>
> In fact, all four uses in glibc (for SPARC) are indeed unnecessary:
> they appear in files that convert 32-bit or 64-bit integers to IEEE
> binary128, an operation that can never raise any exceptions. If this
> was intended to enable the compiler to optimize away any FP_FROM_INT
> code testing for exceptional cases, we now have a better way of doing
> this: defining FP_NO_EXCEPTIONS before including soft-fp.h causes all
> code handling exceptions to be stubbed out, and the rounding mode to
> be hardwired for round-to-zero, to allow such optimizations for source
> files where (a) the operation in question, for the particular types in
> question, can never raise exceptions, but (b) some instances of the
> operation for other types can, so the macros used in the file do
> contain references to rounding or exceptions, albeit dead in that
> particular file.
>
> The uses in the Linux kernel are also unnecessary (clearing exceptions
> at a point where they are already cleared).
>
> This patch duly removes FP_CLEAR_EXCEPTIONS, making the SPARC code in
> question use FP_NO_EXCEPTIONS and stop using exception-related macros.
>
> Untested (testing for 32-bit SPARC is needed).
>
> 2014-09-16 Joseph Myers <joseph@codesourcery.com>
>
> * soft-fp/soft-fp.h (FP_CLEAR_EXCEPTIONS): Remove macro.
> * sysdeps/sparc/sparc32/soft-fp/q_itoq.c: Define FP_NO_EXCEPTIONS.
> (_Q_itoq): Do not use FP_DECL_EX, FP_CLEAR_EXCEPTIONS or
> FP_HANDLE_EXCEPTIONS.
> * sysdeps/sparc/sparc32/soft-fp/q_lltoq.c: Define FP_NO_EXCEPTIONS.
> (_Q_lltoq): Do not use FP_DECL_EX, FP_CLEAR_EXCEPTIONS or
> FP_HANDLE_EXCEPTIONS.
> * sysdeps/sparc/sparc32/soft-fp/q_ulltoq.c: Define FP_NO_EXCEPTIONS.
> (_Q_ulltoq): Do not use FP_DECL_EX, FP_CLEAR_EXCEPTIONS or
> FP_HANDLE_EXCEPTIONS.
> * sysdeps/sparc/sparc32/soft-fp/q_utoq.c: Define FP_NO_EXCEPTIONS.
> (_Q_utoq): Do not use FP_DECL_EX, FP_CLEAR_EXCEPTIONS or
> FP_HANDLE_EXCEPTIONS.
Looks good to me. I did not test this and conducted only a source-level review.
I think this should go forward as a cleanup though since it makes sense.
> diff --git a/soft-fp/soft-fp.h b/soft-fp/soft-fp.h
> index 8d0efa5..31f91ff 100644
> --- a/soft-fp/soft-fp.h
> +++ b/soft-fp/soft-fp.h
> @@ -129,9 +129,6 @@
> #define FP_SET_EXCEPTION(ex) \
> _fex |= (ex)
>
> -#define FP_CLEAR_EXCEPTIONS \
> - _fex = 0
> -
OK.
> #define FP_CUR_EXCEPTIONS \
> (_fex)
>
> diff --git a/sysdeps/sparc/sparc32/soft-fp/q_itoq.c b/sysdeps/sparc/sparc32/soft-fp/q_itoq.c
> index 3640506..1ebb2de 100644
> --- a/sysdeps/sparc/sparc32/soft-fp/q_itoq.c
> +++ b/sysdeps/sparc/sparc32/soft-fp/q_itoq.c
> @@ -19,19 +19,17 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> +#define FP_NO_EXCEPTIONS
> #include "soft-fp.h"
> #include "quad.h"
>
> long double _Q_itoq(const int a)
> {
> - FP_DECL_EX;
> FP_DECL_Q(C);
> int b = a;
> long double c;
>
> FP_FROM_INT_Q(C, b, 32, unsigned int);
> FP_PACK_RAW_Q(c, C);
> - FP_CLEAR_EXCEPTIONS;
> - FP_HANDLE_EXCEPTIONS;
OK.
> return c;
> }
> diff --git a/sysdeps/sparc/sparc32/soft-fp/q_lltoq.c b/sysdeps/sparc/sparc32/soft-fp/q_lltoq.c
> index 52b2712..b60d26d 100644
> --- a/sysdeps/sparc/sparc32/soft-fp/q_lltoq.c
> +++ b/sysdeps/sparc/sparc32/soft-fp/q_lltoq.c
> @@ -19,19 +19,17 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> +#define FP_NO_EXCEPTIONS
OK.
> #include "soft-fp.h"
> #include "quad.h"
>
> long double _Q_lltoq(const long long a)
> {
> - FP_DECL_EX;
> FP_DECL_Q(C);
> long double c;
> long long b = a;
>
> FP_FROM_INT_Q(C, b, 64, unsigned long long);
> FP_PACK_RAW_Q(c, C);
> - FP_CLEAR_EXCEPTIONS;
> - FP_HANDLE_EXCEPTIONS;
OK.
> return c;
> }
> diff --git a/sysdeps/sparc/sparc32/soft-fp/q_ulltoq.c b/sysdeps/sparc/sparc32/soft-fp/q_ulltoq.c
> index 7512557..524fcb9 100644
> --- a/sysdeps/sparc/sparc32/soft-fp/q_ulltoq.c
> +++ b/sysdeps/sparc/sparc32/soft-fp/q_ulltoq.c
> @@ -19,19 +19,17 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> +#define FP_NO_EXCEPTIONS
OK.
> #include "soft-fp.h"
> #include "quad.h"
>
> long double _Q_ulltoq(const unsigned long long a)
> {
> - FP_DECL_EX;
> FP_DECL_Q(C);
> long double c;
> unsigned long long b = a;
>
> FP_FROM_INT_Q(C, b, 64, unsigned long long);
> FP_PACK_RAW_Q(c, C);
> - FP_CLEAR_EXCEPTIONS;
> - FP_HANDLE_EXCEPTIONS;
OK.
> return c;
> }
> diff --git a/sysdeps/sparc/sparc32/soft-fp/q_utoq.c b/sysdeps/sparc/sparc32/soft-fp/q_utoq.c
> index 0fb645c..0ba823e 100644
> --- a/sysdeps/sparc/sparc32/soft-fp/q_utoq.c
> +++ b/sysdeps/sparc/sparc32/soft-fp/q_utoq.c
> @@ -19,19 +19,17 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> +#define FP_NO_EXCEPTIONS
OK.
> #include "soft-fp.h"
> #include "quad.h"
>
> long double _Q_utoq(const unsigned int a)
> {
> - FP_DECL_EX;
> FP_DECL_Q(C);
> long double c;
> unsigned int b = a;
>
> FP_FROM_INT_Q(C, b, 32, unsigned int);
> FP_PACK_RAW_Q(c, C);
> - FP_CLEAR_EXCEPTIONS;
> - FP_HANDLE_EXCEPTIONS;
OK.
> return c;
> }
>
Cheers,
Carlos.