diff mbox series

[v3,3/3] alpha: classify floating-point values without comparing

Message ID 20260911233517.2113403-3-mattst88@gmail.com (mailing list archive)
State Under Review
Delegated to: Adhemerval Zanella Netto
Headers
Series [v3,1/3] math: move the classification macros to bits/fpclassify.h |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Test failed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Matt Turner Sept. 11, 2026, 11:35 p.m. UTC 
  The Alpha floating-point comparison instructions raise a denormal-operand
exception for a subnormal operand.  Unless the compiler arranges for software
completion (it only does so for -mieee, which is not the default), the
outcome depends on the FPCR denormal-operand-disable bit: either the operand
is flushed to zero, which makes a subnormal indistinguishable from a zero,
or the trap is delivered with no completion information, which is fatal.

GCC expands the classification builtins into such comparisons, so

  printf ("%d\n", fpclassify (0x1p-1050));

either prints FP_ZERO or dies with SIGFPE, depending on the trap mode.
iszero has the same problem without any builtin being involved, because it
expands to a comparison against zero written out in the header.

Add an Alpha bits/fpclassify.h that uses the out-of-line implementations,
which examine the encoding in an integer register.  Classifying out of line
means fpclassify, isnormal, issubnormal and iszero now need libm on Alpha,
since __fpclassify is exported from libm alone, unlike __isnan, __finite and
__isinf, which libc exports too.

This fixes C only.  libstdc++ undefines these macros in <cmath> and uses the
builtins itself, so C++ still classifies by comparing.

The new test is built with -mfp-trap-mode=n because glibc itself is built
with -mieee, which would hide the problem.  Without the Alpha header it fails
with

  Didn't expect signal from child: got `Floating point exception'

GCC PR target/127298 tracks expanding the classification builtins without
comparisons in the Alpha back end; once a release containing that fix exists,
the definitions in the Alpha header can be made conditional on the compiler
predating it.
---
 math/Makefile                   |  1 +
 math/test-classify-subnormal.c  | 63 +++++++++++++++++++++++++++++++++
 sysdeps/alpha/Makefile          |  6 ++++
 sysdeps/alpha/bits/fpclassify.h | 37 +++++++++++++++++++
 4 files changed, 107 insertions(+)
 create mode 100644 math/test-classify-subnormal.c
 create mode 100644 sysdeps/alpha/bits/fpclassify.h

Comments

Yury Khrustalev Sept. 16, 2026, 3:28 p.m. UTC | #1 
On Fri, Sep 11, 2026 at 07:35:17PM -0400, Matt Turner wrote:
> The Alpha floating-point comparison instructions raise a denormal-operand
> exception for a subnormal operand.  Unless the compiler arranges for software
> completion (it only does so for -mieee, which is not the default), the
> outcome depends on the FPCR denormal-operand-disable bit: either the operand
> is flushed to zero, which makes a subnormal indistinguishable from a zero,
> or the trap is delivered with no completion information, which is fatal.
> 
> GCC expands the classification builtins into such comparisons, so
> 
>   printf ("%d\n", fpclassify (0x1p-1050));
> 
> either prints FP_ZERO or dies with SIGFPE, depending on the trap mode.
> iszero has the same problem without any builtin being involved, because it
> expands to a comparison against zero written out in the header.
> 
> Add an Alpha bits/fpclassify.h that uses the out-of-line implementations,
> which examine the encoding in an integer register.  Classifying out of line
> means fpclassify, isnormal, issubnormal and iszero now need libm on Alpha,
> since __fpclassify is exported from libm alone, unlike __isnan, __finite and
> __isinf, which libc exports too.
> 
> This fixes C only.  libstdc++ undefines these macros in <cmath> and uses the
> builtins itself, so C++ still classifies by comparing.
> 
> The new test is built with -mfp-trap-mode=n because glibc itself is built
> with -mieee, which would hide the problem.  Without the Alpha header it fails
> with
> 
>   Didn't expect signal from child: got `Floating point exception'
> 
> GCC PR target/127298 tracks expanding the classification builtins without
> comparisons in the Alpha back end; once a release containing that fix exists,
> the definitions in the Alpha header can be made conditional on the compiler
> predating it.

There is failure report in Patchwork:

  linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Test failed

FWIW, I cannot reproduce these failures locally. Must be a false
positive.
Matt Turner Sept. 16, 2026, 8:41 p.m. UTC | #2 
On Wed, Sep 16, 2026 at 11:29 AM Yury Khrustalev
<yury.khrustalev@arm.com> wrote:
>
> On Fri, Sep 11, 2026 at 07:35:17PM -0400, Matt Turner wrote:
> > The Alpha floating-point comparison instructions raise a denormal-operand
> > exception for a subnormal operand.  Unless the compiler arranges for software
> > completion (it only does so for -mieee, which is not the default), the
> > outcome depends on the FPCR denormal-operand-disable bit: either the operand
> > is flushed to zero, which makes a subnormal indistinguishable from a zero,
> > or the trap is delivered with no completion information, which is fatal.
> >
> > GCC expands the classification builtins into such comparisons, so
> >
> >   printf ("%d\n", fpclassify (0x1p-1050));
> >
> > either prints FP_ZERO or dies with SIGFPE, depending on the trap mode.
> > iszero has the same problem without any builtin being involved, because it
> > expands to a comparison against zero written out in the header.
> >
> > Add an Alpha bits/fpclassify.h that uses the out-of-line implementations,
> > which examine the encoding in an integer register.  Classifying out of line
> > means fpclassify, isnormal, issubnormal and iszero now need libm on Alpha,
> > since __fpclassify is exported from libm alone, unlike __isnan, __finite and
> > __isinf, which libc exports too.
> >
> > This fixes C only.  libstdc++ undefines these macros in <cmath> and uses the
> > builtins itself, so C++ still classifies by comparing.
> >
> > The new test is built with -mfp-trap-mode=n because glibc itself is built
> > with -mieee, which would hide the problem.  Without the Alpha header it fails
> > with
> >
> >   Didn't expect signal from child: got `Floating point exception'
> >
> > GCC PR target/127298 tracks expanding the classification builtins without
> > comparisons in the Alpha back end; once a release containing that fix exists,
> > the definitions in the Alpha header can be made conditional on the compiler
> > predating it.
>
> There is failure report in Patchwork:
>
>   linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Test failed
>
> FWIW, I cannot reproduce these failures locally. Must be a false
> positive.

Yes, I've seen mail with false positives in response to basically all
of my patches recently. I think there's something wrong with the
system.
diff mbox series

Patch

diff --git ./math/Makefile ./math/Makefile
index d9b17a8277..44c882063c 100644
--- ./math/Makefile
+++ ./math/Makefile
@@ -538,6 +538,7 @@  tests = \
   bug-nextafter \
   bug-nexttoward \
   test-ceil-except-2 \
+  test-classify-subnormal \
   test-femode \
   test-femode-traps \
   test-fenv basic-test \
diff --git ./math/test-classify-subnormal.c ./math/test-classify-subnormal.c
new file mode 100644
index 0000000000..eb874d00cc
--- /dev/null
+++ ./math/test-classify-subnormal.c
@@ -0,0 +1,63 @@ 
+/* Test classification macros with subnormal arguments.
+   Copyright (C) 2026 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
+   <https://www.gnu.org/licenses/>.  */
+
+/* The classification macros must distinguish a subnormal from a zero,
+   and must not trap.  */
+
+#include <bits/floatn.h>
+#include <float.h>
+#include <math.h>
+#include <support/check.h>
+
+#define TEST(TYPE, TRUE_MIN)						\
+  do									\
+    {									\
+      volatile TYPE sub = (TRUE_MIN);					\
+      TEST_VERIFY (fpclassify (sub) == FP_SUBNORMAL);			\
+      TEST_VERIFY (issubnormal (sub));					\
+      TEST_VERIFY (isfinite (sub));					\
+      TEST_VERIFY (!isnormal (sub));					\
+      TEST_VERIFY (!isnan (sub));					\
+      TEST_VERIFY (!isinf (sub));					\
+      TEST_VERIFY (!iszero (sub));					\
+      TEST_VERIFY (!signbit (sub));					\
+      TEST_VERIFY (fpclassify (-sub) == FP_SUBNORMAL);			\
+      TEST_VERIFY (signbit (-sub));					\
+    }									\
+  while (0)
+
+static int
+do_test (void)
+{
+  TEST (float, FLT_TRUE_MIN);
+  TEST (double, DBL_TRUE_MIN);
+  TEST (long double, LDBL_TRUE_MIN);
+#if __HAVE_DISTINCT_FLOAT128
+  TEST (_Float128, __FLT128_DENORM_MIN__);
+#endif
+
+  /* A zero must still classify as FP_ZERO.  */
+  volatile double zero = 0.0;
+  TEST_VERIFY (fpclassify (zero) == FP_ZERO);
+  TEST_VERIFY (iszero (zero));
+  TEST_VERIFY (!issubnormal (zero));
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git ./sysdeps/alpha/Makefile ./sysdeps/alpha/Makefile
index faa59ab27a..825c2351d2 100644
--- ./sysdeps/alpha/Makefile
+++ ./sysdeps/alpha/Makefile
@@ -54,6 +54,12 @@  CFLAGS-test-misc.c += -mieee-with-inexact
 # Avoid "conflicting types for built-in function" warnings
 CFLAGS-s_isnan.c += -fno-builtin-isnanf
 
+# glibc is built with -mieee (see below), which makes comparisons safe for
+# a subnormal operand; ordinary code is not, so test the default trap mode.
+# This must override the -mieee in sysdep-CFLAGS, so it has to come last on
+# the command line.
+CFLAGS-test-classify-subnormal.c += -mfp-trap-mode=n
+
 # _Float32x and _Float64 are both binary64 on Alpha, so this narrowing
 # divide is a plain divide and the hardware alone decides whether to
 # signal underflow.  IEEE 754 determines tininess after rounding from the
diff --git ./sysdeps/alpha/bits/fpclassify.h ./sysdeps/alpha/bits/fpclassify.h
new file mode 100644
index 0000000000..6efb1aee4a
--- /dev/null
+++ ./sysdeps/alpha/bits/fpclassify.h
@@ -0,0 +1,37 @@ 
+/* Define floating-point classification macros.  Alpha version.
+   Copyright (C) 2026 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
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _MATH_H
+# error "Never use <bits/fpclassify.h> directly; include <math.h> instead."
+#endif
+
+/* The Alpha floating-point comparison instructions raise a
+   denormal-operand exception for a subnormal operand.  With the default
+   trap mode the operand is either flushed to zero, which makes a
+   subnormal indistinguishable from a zero, or the trap is fatal.  Use
+   the out-of-line implementations, which examine the encoding in an
+   integer register, rather than the builtins, which compare.  GCC PR
+   target/127298 tracks expanding the builtins without comparisons.  */
+
+#define __MATH_CLASSIFY_NO_COMPARE 1
+
+#define fpclassify(x) __MATH_TG ((x), __fpclassify, (x))
+#define isfinite(x) __MATH_TG ((x), __finite, (x))
+#define isnormal(x) (fpclassify (x) == FP_NORMAL)
+#define isnan(x) __MATH_TG ((x), __isnan, (x))
+#define isinf(x) __MATH_TG ((x), __isinf, (x))