* [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
@ 2017-10-03 14:03 Gabriel F. T. Gomes
2017-10-03 14:54 ` Joseph Myers
0 siblings, 1 reply; 10+ messages in thread
From: Gabriel F. T. Gomes @ 2017-10-03 14:03 UTC (permalink / raw)
To: libc-alpha
All representations of floating-point numbers in types with IEC 60559
binary exchange format are canonical. On the other hand, types with IEC
60559 extended formats, such as those implemented under ldbl-96 and
ldbl-128ibm, contain representations that are not canonical.
TS 18661-1 introduced the type-generic macro iscanonical, which returns
whether a floating-point value is canonical or not. In Glibc, this
type-generic macro is implemented using the macro __MATH_TG, which, when
support for float128 is enabled, relies on __builtin_types_compatible_p
to select between floating-point types. However, this use of
iscanonical breaks C++ applications, because the builtin is only
available in C mode.
This patch provides a C++ implementation of iscanonical that relies on
function overloading, rather than builtins, to select between
floating-point types.
Unlike the C++ implementations for iszero and issignaling, this
implementation ignores __NO_LONG_DOUBLE_MATH. The double type always
matches IEC 60559 double format, which is always canonical. Thus, when
double and long double are the same (__NO_LONG_DOUBLE_MATH), iscanonical
always returns 1 and is not implemented with __MATH_TG.
Tested for powerpc64, powerpc64le and x86_64.
[BZ #22235]
* math/Makefile [CXX] (tests): Add test-math-iscanonical.cc.
(CFLAGS-test-math-iscanonical.cc): New variable.
* math/test-math-iscanonical.cc: New file.
* sysdeps/ieee754/ldbl-96/bits/iscanonical.h (iscanonical):
Provide a C++ implementation based on function overloading,
rather than using __MATH_TG, which uses C-only builtins.
* sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h (iscanonical):
Likewise.
* sysdeps/powerpc/powerpc64le/Makefile
(CFLAGS-test-math-iscanonical.cc): New variable.
---
math/Makefile | 4 ++-
math/test-math-iscanonical.cc | 48 ++++++++++++++++++++++++++
sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h | 21 +++++++++--
sysdeps/ieee754/ldbl-96/bits/iscanonical.h | 19 +++++++++-
sysdeps/powerpc/powerpc64le/Makefile | 1 +
5 files changed, 89 insertions(+), 4 deletions(-)
create mode 100644 math/test-math-iscanonical.cc
diff --git a/math/Makefile b/math/Makefile
index 6c8aa3e413..008eeb2d18 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -208,7 +208,8 @@ tests-internal = test-matherr test-matherr-2
tests-static += atest-exp atest-sincos atest-exp2
ifneq (,$(CXX))
-tests += test-math-isinff test-math-iszero test-math-issignaling
+tests += test-math-isinff test-math-iszero test-math-issignaling \
+ test-math-iscanonical
endif
ifneq (no,$(PERL))
@@ -356,6 +357,7 @@ CFLAGS-test-signgam-ullong-init-static.c = -std=c99
CFLAGS-test-math-isinff.cc = -std=gnu++11
CFLAGS-test-math-iszero.cc = -std=gnu++11
CFLAGS-test-math-issignaling.cc = -std=gnu++11
+CFLAGS-test-math-iscanonical.cc = -std=gnu++11
CFLAGS-test-iszero-excess-precision.c = -fexcess-precision=standard
CFLAGS-test-iseqsig-excess-precision.c = -fexcess-precision=standard
diff --git a/math/test-math-iscanonical.cc b/math/test-math-iscanonical.cc
new file mode 100644
index 0000000000..aba68acb4f
--- /dev/null
+++ b/math/test-math-iscanonical.cc
@@ -0,0 +1,48 @@
+/* Test for the C++ implementation of iscanonical.
+ Copyright (C) 2017 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/>. */
+
+#define _GNU_SOURCE 1
+#include <math.h>
+#include <stdio.h>
+
+static bool errors;
+
+template <class T>
+static void
+check_type ()
+{
+ T val = 0;
+
+ /* Check if iscanonical is available in C++ mode (bug 22235). */
+ if (iscanonical (val) == 0)
+ errors++;
+}
+
+static int
+do_test (void)
+{
+ check_type<float> ();
+ check_type<double> ();
+ check_type<long double> ();
+#if __HAVE_DISTINCT_FLOAT128
+ check_type<_Float128> ();
+#endif
+ return errors;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h b/sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h
index 7ddb368d26..1741aac048 100644
--- a/sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h
+++ b/sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h
@@ -37,5 +37,22 @@ extern int __iscanonicall (long double __x)
conversion, before being discarded; in IBM long double, there are
encodings that are not consistently handled as corresponding to any
particular value of the type, and we return 0 for those. */
-# define iscanonical(x) __MATH_TG ((x), __iscanonical, (x))
-#endif
+# ifndef __cplusplus
+# define iscanonical(x) __MATH_TG ((x), __iscanonical, (x))
+# else
+/* In C++ mode, __MATH_TG cannot be used, because it relies on
+ __builtin_types_compatible_p, which is a C-only builtin. On the
+ other hand, overloading provides the means to distinguish between
+ the floating-point types. The overloading resolution will match
+ the correct parameter (regardless of type qualifiers (i.e.: const
+ and volatile). */
+extern "C++" {
+inline int iscanonical (float __val) { return __iscanonicalf (__val); }
+inline int iscanonical (double __val) { return __iscanonical (__val); }
+inline int iscanonical (long double __val) { return __iscanonicall (__val); }
+# if __HAVE_DISTINCT_FLOAT128
+inline int iscanonical (_Float128 __val) { return __iscanonicalf128 (__val); }
+# endif
+}
+# endif /* __cplusplus */
+#endif /* __NO_LONG_DOUBLE_MATH */
diff --git a/sysdeps/ieee754/ldbl-96/bits/iscanonical.h b/sysdeps/ieee754/ldbl-96/bits/iscanonical.h
index 4a4f4ad024..366125a2cc 100644
--- a/sysdeps/ieee754/ldbl-96/bits/iscanonical.h
+++ b/sysdeps/ieee754/ldbl-96/bits/iscanonical.h
@@ -34,4 +34,21 @@ extern int __iscanonicall (long double __x)
conversion, before being discarded; in extended precision, there
are encodings that are not consistently handled as corresponding to
any particular value of the type, and we return 0 for those. */
-#define iscanonical(x) __MATH_TG ((x), __iscanonical, (x))
+#ifndef __cplusplus
+# define iscanonical(x) __MATH_TG ((x), __iscanonical, (x))
+#else
+/* In C++ mode, __MATH_TG cannot be used, because it relies on
+ __builtin_types_compatible_p, which is a C-only builtin. On the
+ other hand, overloading provides the means to distinguish between
+ the floating-point types. The overloading resolution will match
+ the correct parameter (regardless of type qualifiers (i.e.: const
+ and volatile). */
+extern "C++" {
+inline int iscanonical (float __val) { return __iscanonicalf (__val); }
+inline int iscanonical (double __val) { return __iscanonical (__val); }
+inline int iscanonical (long double __val) { return __iscanonicall (__val); }
+# if __HAVE_DISTINCT_FLOAT128
+inline int iscanonical (_Float128 __val) { return __iscanonicalf128 (__val); }
+# endif
+}
+#endif /* __cplusplus */
diff --git a/sysdeps/powerpc/powerpc64le/Makefile b/sysdeps/powerpc/powerpc64le/Makefile
index 3fd9d9a715..f554a791b7 100644
--- a/sysdeps/powerpc/powerpc64le/Makefile
+++ b/sysdeps/powerpc/powerpc64le/Makefile
@@ -16,6 +16,7 @@ $(foreach suf,$(all-object-suffixes),%f128_r$(suf)): CFLAGS += -mfloat128
$(foreach suf,$(all-object-suffixes),$(objpfx)test-float128%$(suf)): CFLAGS += -mfloat128
$(foreach suf,$(all-object-suffixes),$(objpfx)test-ifloat128%$(suf)): CFLAGS += -mfloat128
CFLAGS-libm-test-support-float128.c += -mfloat128
+CFLAGS-test-math-iscanonical.cc += -mfloat128
CFLAGS-test-math-issignaling.cc += -mfloat128
CFLAGS-test-math-iszero.cc += -mfloat128
$(objpfx)test-float128% $(objpfx)test-ifloat128% $(objpfx)test-math-iszero: \
--
2.13.6
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-03 14:03 [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm Gabriel F. T. Gomes
@ 2017-10-03 14:54 ` Joseph Myers
2017-10-03 19:10 ` Gabriel F. T. Gomes
0 siblings, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2017-10-03 14:54 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
On Tue, 3 Oct 2017, Gabriel F. T. Gomes wrote:
> [BZ #22235]
> * math/Makefile [CXX] (tests): Add test-math-iscanonical.cc.
> (CFLAGS-test-math-iscanonical.cc): New variable.
> * math/test-math-iscanonical.cc: New file.
> * sysdeps/ieee754/ldbl-96/bits/iscanonical.h (iscanonical):
> Provide a C++ implementation based on function overloading,
> rather than using __MATH_TG, which uses C-only builtins.
> * sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h (iscanonical):
> Likewise.
> * sysdeps/powerpc/powerpc64le/Makefile
> (CFLAGS-test-math-iscanonical.cc): New variable.
OK, but note
> +/* In C++ mode, __MATH_TG cannot be used, because it relies on
> + __builtin_types_compatible_p, which is a C-only builtin. On the
> + other hand, overloading provides the means to distinguish between
> + the floating-point types. The overloading resolution will match
> + the correct parameter (regardless of type qualifiers (i.e.: const
> + and volatile). */
(twice in this patch, and once already in math.h) is missing a second
close parenthesis to match the first open parenthesis, and should be
fixed.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-03 14:54 ` Joseph Myers
@ 2017-10-03 19:10 ` Gabriel F. T. Gomes
2017-10-03 23:59 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: Gabriel F. T. Gomes @ 2017-10-03 19:10 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
On 03 Oct 2017, Joseph Myers wrote:
>> +/* In C++ mode, __MATH_TG cannot be used, because it relies on
>> + __builtin_types_compatible_p, which is a C-only builtin. On the
>> + other hand, overloading provides the means to distinguish between
>> + the floating-point types. The overloading resolution will match
>> + the correct parameter (regardless of type qualifiers (i.e.: const
>> + and volatile). */
>
>(twice in this patch, and once already in math.h) is missing a second
>close parenthesis to match the first open parenthesis, and should be
>fixed.
Thanks. Pushed with these changes.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-03 19:10 ` Gabriel F. T. Gomes
@ 2017-10-03 23:59 ` H.J. Lu
2017-10-04 0:01 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2017-10-03 23:59 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: Joseph Myers, libc-alpha
On 10/3/17, Gabriel F. T. Gomes <gabriel@inconstante.eti.br> wrote:
> On 03 Oct 2017, Joseph Myers wrote:
>
>>> +/* In C++ mode, __MATH_TG cannot be used, because it relies on
>>> + __builtin_types_compatible_p, which is a C-only builtin. On the
>>> + other hand, overloading provides the means to distinguish between
>>> + the floating-point types. The overloading resolution will match
>>> + the correct parameter (regardless of type qualifiers (i.e.: const
>>> + and volatile). */
>>
>>(twice in this patch, and once already in math.h) is missing a second
>>close parenthesis to match the first open parenthesis, and should be
>>fixed.
>
> Thanks. Pushed with these changes.
>
>
On i686 wit GCC 7, I got
test-math-iscanonical.cc: In function ‘void check_type()’:
test-math-iscanonical.cc:33:11: error: use of an operand of type
‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
errors++;
^~
test-math-iscanonical.cc: In instantiation of ‘void check_type() [with
T = float]’:
test-math-iscanonical.cc:39:22: required from here
test-math-iscanonical.cc:33:11: error: use of an operand of type
‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
errors++;
~~~~~~^~
--
H.J.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-03 23:59 ` H.J. Lu
@ 2017-10-04 0:01 ` H.J. Lu
2017-10-04 0:43 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2017-10-04 0:01 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: Joseph Myers, libc-alpha
On 10/3/17, H.J. Lu <hjl.tools@gmail.com> wrote:
> On 10/3/17, Gabriel F. T. Gomes <gabriel@inconstante.eti.br> wrote:
>> On 03 Oct 2017, Joseph Myers wrote:
>>
>>>> +/* In C++ mode, __MATH_TG cannot be used, because it relies on
>>>> + __builtin_types_compatible_p, which is a C-only builtin. On the
>>>> + other hand, overloading provides the means to distinguish between
>>>> + the floating-point types. The overloading resolution will match
>>>> + the correct parameter (regardless of type qualifiers (i.e.: const
>>>> + and volatile). */
>>>
>>>(twice in this patch, and once already in math.h) is missing a second
>>>close parenthesis to match the first open parenthesis, and should be
>>>fixed.
>>
>> Thanks. Pushed with these changes.
>>
>>
>
> On i686 wit GCC 7, I got
>
> test-math-iscanonical.cc: In function ‘void check_type()’:
> test-math-iscanonical.cc:33:11: error: use of an operand of type
> ‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
> errors++;
> ^~
> test-math-iscanonical.cc: In instantiation of ‘void check_type() [with
> T = float]’:
> test-math-iscanonical.cc:39:22: required from here
> test-math-iscanonical.cc:33:11: error: use of an operand of type
> ‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
> errors++;
> ~~~~~~^~
>
I am testing this:
diff --git a/math/test-math-iscanonical.cc b/math/test-math-iscanonical.cc
index aba68acb4f..8ced7a73b4 100644
--- a/math/test-math-iscanonical.cc
+++ b/math/test-math-iscanonical.cc
@@ -20,7 +20,7 @@
#include <math.h>
#include <stdio.h>
-static bool errors;
+static int errors;
template <class T>
static void
--
H.J.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-04 0:01 ` H.J. Lu
@ 2017-10-04 0:43 ` H.J. Lu
2017-10-04 1:21 ` Gabriel F. T. Gomes
2017-10-04 13:15 ` Joseph Myers
0 siblings, 2 replies; 10+ messages in thread
From: H.J. Lu @ 2017-10-04 0:43 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: Joseph Myers, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1895 bytes --]
On 10/3/17, H.J. Lu <hjl.tools@gmail.com> wrote:
> On 10/3/17, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On 10/3/17, Gabriel F. T. Gomes <gabriel@inconstante.eti.br> wrote:
>>> On 03 Oct 2017, Joseph Myers wrote:
>>>
>>>>> +/* In C++ mode, __MATH_TG cannot be used, because it relies on
>>>>> + __builtin_types_compatible_p, which is a C-only builtin. On the
>>>>> + other hand, overloading provides the means to distinguish between
>>>>> + the floating-point types. The overloading resolution will match
>>>>> + the correct parameter (regardless of type qualifiers (i.e.: const
>>>>> + and volatile). */
>>>>
>>>>(twice in this patch, and once already in math.h) is missing a second
>>>>close parenthesis to match the first open parenthesis, and should be
>>>>fixed.
>>>
>>> Thanks. Pushed with these changes.
>>>
>>>
>>
>> On i686 wit GCC 7, I got
>>
>> test-math-iscanonical.cc: In function ‘void check_type()’:
>> test-math-iscanonical.cc:33:11: error: use of an operand of type
>> ‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
>> errors++;
>> ^~
>> test-math-iscanonical.cc: In instantiation of ‘void check_type() [with
>> T = float]’:
>> test-math-iscanonical.cc:39:22: required from here
>> test-math-iscanonical.cc:33:11: error: use of an operand of type
>> ‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
>> errors++;
>> ~~~~~~^~
>>
>
> I am testing this:
>
> diff --git a/math/test-math-iscanonical.cc b/math/test-math-iscanonical.cc
> index aba68acb4f..8ced7a73b4 100644
> --- a/math/test-math-iscanonical.cc
> +++ b/math/test-math-iscanonical.cc
> @@ -20,7 +20,7 @@
> #include <math.h>
> #include <stdio.h>
>
> -static bool errors;
> +static int errors;
>
> template <class T>
> static void
>
>
This is what I checked in.
--
H.J.
[-- Attachment #2: 0001-test-math-iscanonical.cc-Replace-bool-with-int.patch --]
[-- Type: text/x-patch, Size: 1041 bytes --]
From 117353f294b215a164905ab6d412f3bf8d77ae5a Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 3 Oct 2017 17:11:55 -0700
Subject: [PATCH] test-math-iscanonical.cc: Replace bool with int
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix GCC 7 compilation error:
test-math-iscanonical.cc: In function ‘void check_type()’:
test-math-iscanonical.cc:33:11: error: use of an operand of type ‘bool’ in ‘operator++’ is deprecated [-Werror=deprecated]
errors++;
^~
* math/test-math-iscanonical.cc (error): Replace bool with int.
---
math/test-math-iscanonical.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/math/test-math-iscanonical.cc b/math/test-math-iscanonical.cc
index aba68acb4f..8ced7a73b4 100644
--- a/math/test-math-iscanonical.cc
+++ b/math/test-math-iscanonical.cc
@@ -20,7 +20,7 @@
#include <math.h>
#include <stdio.h>
-static bool errors;
+static int errors;
template <class T>
static void
--
2.13.6
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-04 0:43 ` H.J. Lu
@ 2017-10-04 1:21 ` Gabriel F. T. Gomes
2017-10-04 13:15 ` Joseph Myers
1 sibling, 0 replies; 10+ messages in thread
From: Gabriel F. T. Gomes @ 2017-10-04 1:21 UTC (permalink / raw)
To: H.J. Lu; +Cc: Joseph Myers, libc-alpha
On 03 Oct 2017, H.J. Lu wrote:
>On 10/3/17, H.J. Lu <hjl.tools@gmail.com> wrote:
>> --- a/math/test-math-iscanonical.cc
>> +++ b/math/test-math-iscanonical.cc
>> @@ -20,7 +20,7 @@
>> #include <math.h>
>> #include <stdio.h>
>>
>> -static bool errors;
>> +static int errors;
>>
>> template <class T>
>> static void
>>
>>
>
>This is what I checked in.
Thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-04 0:43 ` H.J. Lu
2017-10-04 1:21 ` Gabriel F. T. Gomes
@ 2017-10-04 13:15 ` Joseph Myers
2017-10-04 13:47 ` Florian Weimer
1 sibling, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2017-10-04 13:15 UTC (permalink / raw)
To: H.J. Lu; +Cc: Gabriel F. T. Gomes, libc-alpha
On Wed, 4 Oct 2017, H.J. Lu wrote:
> This is what I checked in.
This fix doesn't seem to be on 2.26 branch, but needs to go there as the
original patch went there.
I don't think using an int count of errors and returning it from do_test
is a good coding pattern, because if the count reaches 77 it will result
in a spurious UNSUPPORTED result. Of course in this particular test it
can't reach 77, but a better pattern is either a boolean error state (set
to true rather than using ++, given the warning quoted here), or a count
but with do_test returning errors != 0.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-04 13:15 ` Joseph Myers
@ 2017-10-04 13:47 ` Florian Weimer
2017-10-04 21:36 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2017-10-04 13:47 UTC (permalink / raw)
To: Joseph Myers, H.J. Lu; +Cc: Gabriel F. T. Gomes, libc-alpha
On 10/04/2017 03:15 PM, Joseph Myers wrote:
> On Wed, 4 Oct 2017, H.J. Lu wrote:
>
>> This is what I checked in.
>
> This fix doesn't seem to be on 2.26 branch, but needs to go there as the
> original patch went there.
>
> I don't think using an int count of errors and returning it from do_test
> is a good coding pattern, because if the count reaches 77 it will result
> in a spurious UNSUPPORTED result. Of course in this particular test it
> can't reach 77, but a better pattern is either a boolean error state (set
> to true rather than using ++, given the warning quoted here), or a count
> but with do_test returning errors != 0.
Agreed. Note that TEST_VERIFY allows the test to continue after a
failure, and it also arranges for a non-zero exit status (even across
fork, but currently not across dlopen). It's usually a good alternative
to such error variables.
Thanks,
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm
2017-10-04 13:47 ` Florian Weimer
@ 2017-10-04 21:36 ` H.J. Lu
0 siblings, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2017-10-04 21:36 UTC (permalink / raw)
To: Florian Weimer; +Cc: Joseph Myers, Gabriel F. T. Gomes, libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
On 10/4/17, Florian Weimer <fweimer@redhat.com> wrote:
> On 10/04/2017 03:15 PM, Joseph Myers wrote:
>> On Wed, 4 Oct 2017, H.J. Lu wrote:
>>
>>> This is what I checked in.
>>
>> This fix doesn't seem to be on 2.26 branch, but needs to go there as the
>> original patch went there.
>>
>> I don't think using an int count of errors and returning it from do_test
>> is a good coding pattern, because if the count reaches 77 it will result
>> in a spurious UNSUPPORTED result. Of course in this particular test it
>> can't reach 77, but a better pattern is either a boolean error state (set
>> to true rather than using ++, given the warning quoted here), or a count
>> but with do_test returning errors != 0.
>
> Agreed. Note that TEST_VERIFY allows the test to continue after a
> failure, and it also arranges for a non-zero exit status (even across
> fork, but currently not across dlopen). It's usually a good alternative
> to such error variables.
>
> Thanks,
> Florian
>
i am testing this patch and will backport these 2 patches to 2.26 branch.
--
H.J.
[-- Attachment #2: 0001-test-math-iscanonical.cc-Return-errors-0.patch --]
[-- Type: text/x-patch, Size: 1212 bytes --]
From 758f1bfa2a1bccb52f1b3e97444a367d35aceaee Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 4 Oct 2017 14:31:16 -0700
Subject: [PATCH] test-math-iscanonical.cc: Return errors != 0
Since not all non-zero error counts are errors, return errors != 0
instead.
* math/test-math-iscanonical.cc (do_test): Return errors != 0.
---
ChangeLog | 4 ++++
math/test-math-iscanonical.cc | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 4d94f133dc..c1fd8a7993 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-10-04 H.J. Lu <hongjiu.lu@intel.com>
+
+ * math/test-math-iscanonical.cc (do_test): Return errors != 0.
+
2017-10-04 Joseph Myers <joseph@codesourcery.com>
* sysdeps/ieee754/dbl-64/s_fma.c: Include <libm-alias-double.h>.
diff --git a/math/test-math-iscanonical.cc b/math/test-math-iscanonical.cc
index 8ced7a73b4..4cfb1c5055 100644
--- a/math/test-math-iscanonical.cc
+++ b/math/test-math-iscanonical.cc
@@ -42,7 +42,7 @@ do_test (void)
#if __HAVE_DISTINCT_FLOAT128
check_type<_Float128> ();
#endif
- return errors;
+ return errors != 0;
}
#include <support/test-driver.c>
--
2.13.6
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-10-04 21:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-03 14:03 [PATCH] Add C++ versions of iscanonical for ldbl-96 and ldbl-128ibm Gabriel F. T. Gomes
2017-10-03 14:54 ` Joseph Myers
2017-10-03 19:10 ` Gabriel F. T. Gomes
2017-10-03 23:59 ` H.J. Lu
2017-10-04 0:01 ` H.J. Lu
2017-10-04 0:43 ` H.J. Lu
2017-10-04 1:21 ` Gabriel F. T. Gomes
2017-10-04 13:15 ` Joseph Myers
2017-10-04 13:47 ` Florian Weimer
2017-10-04 21:36 ` H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).