[PATCH v1 2/2] psxtests: Fix math function build warnings

[Gedare Bloom]

Hi Stephen,

On Thu, Mar 18, 2021 at 8:19 AM Stephen Clark <stephen.clark at oarcorp.com> wrote:
>
> Added conditionals to ensure that long double function tests were only
> built when newlib has long double math functions.

This patch is not related to the other patch in the series. I
recommend that you generally should send unrelated patches separately
from each other, rather than in a group like this. That makes the
review easier.  For example, I can't really give a good review on
patch 1/2.

> ---
>  testsuites/psxtests/psxhdrs/math/acoshl.c        |  3 +++
>  testsuites/psxtests/psxhdrs/math/acosl.c         |  3 +++

[...]

> diff --git a/testsuites/psxtests/psxhdrs/math/has_long_double.h b/testsuites/psxtests/psxhdrs/math/has_long_double.h
> new file mode 100644
> index 0000000000..d813d08a61
> --- /dev/null
> +++ b/testsuites/psxtests/psxhdrs/math/has_long_double.h
> @@ -0,0 +1,16 @@
> +#ifndef HAS_LONG_DOUBLE_H
> +#define HAS_LONG_DOUBLE_H
> +//copied from newlib math.h on 21 Jan 2021

The exact location might be helpful. Since this file wasn't copied
entirely, the file header block should be provided as per
https://docs.rtems.org/branches/master/eng/coding-file-hdr.html


> +
> +/* Newlib doesn't fully support long double math functions so far.
> +   On platforms where long double equals double the long double functions
> +   simply call the double functions.  On Cygwin the long double functions
> +   are implemented independently from newlib to be able to use optimized
> +   assembler functions despite using the Microsoft x86_64 ABI. */
> +#if defined (_LDBL_EQ_DBL) || defined (__CYGWIN__)

I guess __CYGWIN__ is never defined?

> +#ifndef __math_68881
> +#define NEWLIB_HAS_LONG_DOUBLE_MATH_FUNCTIONS 1
> +#endif
> +#endif
> +

This header reduces to
#if defined (_LDBL_EQ_DBL) && !defined(__math_68881)
#define NEWLIB_HAS_LONG_DOUBLE_MATH_FUNCTIONS 1
#endif

but, since you just use this as a feature check anyway, you could replace all
#if defined NEWLIB_HAS_LONG_DOUBLE_MATH_FUNCTIONS
with
#if defined (_LDBL_EQ_DBL) && !defined(__math_68881) /* newlib has
long double */
in this patch, and you don't need this header, unless the feature
check is likely to change?

> +#endif
> diff --git a/testsuites/psxtests/psxhdrs/math/hypotl.c b/testsuites/psxtests/psxhdrs/math/hypotl.c
> index bffc632c09..182e4028d8 100644
> --- a/testsuites/psxtests/psxhdrs/math/hypotl.c
> +++ b/testsuites/psxtests/psxhdrs/math/hypotl.c
> @@ -35,7 +35,9 @@
>  #endif
>
>  #include <math.h>
> +#include "has_long_double.h"
>
> +#if defined NEWLIB_HAS_LONG_DOUBLE_MATH_FUNCTIONS
>  int test( void );
>
>  int test( void )
> @@ -46,3 +48,4 @@ int test( void )
>
>    return (result);
>  }
> +#endif

It seems to me that this is doing something that might better be done
by the build system. Why even build these tests, if they are going to
be empty? I wonder if we can probe for the long double math support in
the build