[PATCH 5/5] x86 long double: Add tests for pseudo normal numbers

[Siddhesh Poyarekar]

On 12/23/20 3:18 AM, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 15/12/2020 11:13, Siddhesh Poyarekar via Libc-alpha wrote:
>> Add some tests for fpclassify, isnanl, isinfl and issignaling.
>> ---
>>   sysdeps/x86/fpu/Makefile        |   3 +-
>>   sysdeps/x86/fpu/test-unnormal.c | 196 ++++++++++++++++++++++++++++++++
>>   2 files changed, 198 insertions(+), 1 deletion(-)
>>   create mode 100644 sysdeps/x86/fpu/test-unnormal.c
>>
>> diff --git a/sysdeps/x86/fpu/Makefile b/sysdeps/x86/fpu/Makefile
>> index 600e42c3db..e77de56d14 100644
>> --- a/sysdeps/x86/fpu/Makefile
>> +++ b/sysdeps/x86/fpu/Makefile
>> @@ -4,11 +4,12 @@ CPPFLAGS += -I../soft-fp
>>   
>>   libm-support += powl_helper
>>   tests += test-fenv-sse test-fenv-clear-sse test-fenv-x87 test-fenv-sse-2 \
>> -	 test-flt-eval-method-387 test-flt-eval-method-sse
>> +	 test-flt-eval-method-387 test-flt-eval-method-sse test-unnormal
>>   CFLAGS-test-fenv-sse.c += -msse2 -mfpmath=sse
>>   CFLAGS-test-fenv-clear-sse.c += -msse2 -mfpmath=sse
>>   CFLAGS-test-fenv-sse-2.c += -msse2 -mfpmath=sse
>>   CFLAGS-test-flt-eval-method-387.c += -fexcess-precision=standard -mfpmath=387
>>   CFLAGS-test-flt-eval-method-sse.c += -fexcess-precision=standard -msse2 \
>>   				     -mfpmath=sse
>> +CFLAGS-test-unnormal.c += -fsignaling-nans -std=c2x
>>   endif
> 
> A possibility is to hookup this tests on
> math/libm-test-{fpclassify,isnan,isinf,issignaling}.inc using the new define
> I suggested on the 4/5 part [1] so you can also check if no exceptions are being
> generated and errno is not set.
> 
> It increases the tests coverage and avoid a arch-specific tests.
> 
> [1] https://sourceware.org/pipermail/libc-alpha/2020-December/121004.html

OK, it will need changes to the driver.

>> diff --git a/sysdeps/x86/fpu/test-unnormal.c b/sysdeps/x86/fpu/test-unnormal.c
>> new file mode 100644
>> index 0000000000..fc65d9290f
>> --- /dev/null
>> +++ b/sysdeps/x86/fpu/test-unnormal.c
>> @@ -0,0 +1,196 @@
>> +/* Test long double classification with x86 pseudo normal numbers.
>> +   Copyright (C) 2020 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/>.  */
>> +
>> +#include <math.h>
>> +#include <stdio.h>
>> +#include <string.h>
>> +
>> +struct tests
>> +{
>> +  const char *val;
>> +  int class;
>> +} inputs[] = {
>> +      /* Normal.  */
>> +      {"\x00\x04\x00\x00\x00\x00\x00\x00\x00\x04", FP_NAN},
>> +      {"\x00\x04\x00\x00\x00\x00\x00\xf0\x00\x04", FP_NORMAL},
>> +      /* Pseudo-infinite.  */
>> +      {"\x00\x00\x00\x00\x00\x00\x00\x00\xff\x7f", FP_NAN},
>> +      {"\x00\x00\x00\x00\x00\x00\x00\x80\xff\x7f", FP_INFINITE},
>> +      /* Pseudo-zero.  */
>> +      {"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01", FP_NAN},
>> +      {"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", FP_ZERO},
>> +};
>> +
> 
> I find this quite confusing to parse the value represented. I think
> it would be way more readable to include <math_ldbl.h> and define the
> values using the ieee_long_double_shape_type 'parts' member.

That's a good idea.

> If the idea is also to check snprintf, I think it would be better to
> the tests to a different test.

The *printf already has its own test.

> Also make the inputs a 'static' variable.

OK.

>> +const char *classes[5];
>> +#define stringify(N) #N
>> +
>> +static void
>> +initialize (void)
>> +{
>> +  classes[FP_NAN] = stringify(FP_NAN);
>> +  classes[FP_INFINITE] = stringify(FP_INFINITY);
>> +  classes[FP_ZERO] = stringify(FP_ZERO);
>> +  classes[FP_SUBNORMAL] = stringify(FP_SUBNORMAL);
>> +  classes[FP_NORMAL] = stringify(FP_NORMAL);
>> +}
>> +
>> +static void
>> +unnormal_str (const char *val, char *ret)
>> +{
>> +  for (int i = 9; i >= 0; i--)
>> +    {
>> +      if (i == 7 || i == 3)
>> +	*ret++ = ' ';
>> +      snprintf(ret, 3, "%02x", (unsigned char) val[i]);
>> +      ret += 2;
>> +    }
>> +}
>> +
>> +static int
>> +test_fpclassify (void)
>> +{
>> +  int ret = 0;
>> +
>> +  printf ("* fpclassify tests:\n");
> 
> Maybe add the verbose output only when tests is invoke with --debug
> (same for other cases).

OK.

>> +  for (int i = 0; i < sizeof (inputs)/sizeof (struct tests); i++)
>> +    {
>> +      long double value;
>> +      char buf[22];
>> +
>> +      memcpy (&value, inputs[i].val, 10);
>> +      unnormal_str(inputs[i].val, buf);
>> +      int class = fpclassify(value);
>> +
>> +      if (class != inputs[i].class)
> 
> Use TEST_COMPARE.

OK.

Thanks,
Siddhesh