Re: [newlib] Generally make all 'long double complex' methods available in

2022-11-08 Thread Jeff Johnston via Gcc-patches
LGTM.

-- Jeff J.

On Tue, Nov 8, 2022 at 1:31 PM Thomas Schwinge 
wrote:

> ..., not just '#if defined(__CYGWIN__)'.  (Exception: 'clog10l' which
> currently
> indeed is for Cygwin only.)
>
> This completes 2017-07-05 commit be3ca3947402827aa52709e677369bc7ad30aa1d
> "Fixed warnings for some long double complex methods" after Aditya
> Upadhyay's
> work on importing "Long double complex methods" from NetBSD.
>
> For example, this changes GCC/nvptx libgfortran 'configure' output as
> follows:
>
> [...]
> checking for ccosf... yes
> checking for ccos... yes
> checking for ccosl... [-no-]{+yes+}
> [...]
>
> ..., and correspondingly GCC/nvptx 'nvptx-none/libgfortran/config.h' as
> follows:
>
> [...]
>  /* Define to 1 if you have the `ccosl' function. */
> -/* #undef HAVE_CCOSL */
> +#define HAVE_CCOSL 1
> [...]
>
> Similarly for 'ccoshl', 'cexpl', 'cpowl', 'csinl', 'csinhl', 'ctanl',
> 'ctanhl',
> 'cacoshl', 'cacosl', 'casinhl', 'catanhl'.  ('conjl', 'cprojl' are not
> currently being used in libgfortran.)
>
> This in turn simplifies GCC/nvptx 'libgfortran/intrinsics/c99_functions.c'
> compilation such that this files doesn't have to provide its own
> "Implementation of various C99 functions" for those, when in fact they're
> available in newlib libm.
> ---
>
> A few more words on why this is relevant for GCC.
>
> For example, 'cexpl' usually is provided by libm, but if it isn't, the
> open-coded replacement function in
> 'libgfortran/intrinsics/c99_functions.c' is effective if it holds that
> 'defined(HAVE_COSL) && defined(HAVE_SINL) && defined(HAVE_EXPL)':
>
> long double complex
> cexpl (long double complex z)
> {
>   long double a, b;
>   long double complex v;
>
>   a = REALPART (z);
>   b = IMAGPART (z);
>   COMPLEX_ASSIGN (v, cosl (b), sinl (b));
>   return expl (a) * v;
> }
>
> This replacement code is active for current GCC/nvptx (... if no longer
> compiling GCC/nvptx libgfortran in "minimal" mode, 'LIBGFOR_MINIMAL',
> which I'm currently working on).
>
> Comparing the preceeding to the 'c99_functions.c.188t.sincos' dump, we see
> for
> that function:
>
>  __attribute__((nothrow, leaf, const))
>  complex long double cexpl (complex long double z)
>  {
>long double b;
>long double a;
>long double _1;
>long double _2;
>long double _4;
>long double _5;
>long double _11;
> +  complex long double sincostmp_13;
>
> [local count: 1073741824]:
>a_7 = REALPART_EXPR ;
>b_8 = IMAGPART_EXPR ;
> -  _1 = cosl (b_8);
> -  _2 = sinl (b_8);
> +  sincostmp_13 = __builtin_cexpil (b_8);
> +  _1 = REALPART_EXPR ;
> +  _2 = IMAGPART_EXPR ;
>_11 = expl (a_7);
>_4 = _1 * _11;
>_5 = _2 * _11;
>REALPART_EXPR <> = _4;
>IMAGPART_EXPR <> = _5;
>return ;
>
>  }
>
> That is, the 'cosl (b)', 'sinl (b)' sequence is replaced by
> '__builtin_cexpil'.  That '__builtin_cexpil' is then later mapped back
> into: 'cexpl'.  We've now got an infinitely-recursive 'cexpl' replacement
> function, "implemented via itself"; GCC/nvptx libgfortran assumes there
> is no 'cexpl' in libm, whereas this 'sincos' transformation does assume
> that there is.  (..., which looks like an additional bug on its own.)
>
> At the PTX-level, this leads to the following:
>
> [...]
> // BEGIN GLOBAL FUNCTION DECL: cexpl
> .visible .func cexpl (.param.u64 %in_ar0, .param.f64 %in_ar1,
> .param.f64 %in_ar2);
>
> // BEGIN GLOBAL FUNCTION DEF: cexpl
> .visible .func cexpl (.param.u64 %in_ar0, .param.f64 %in_ar1,
> .param.f64 %in_ar2)
> {
> [...]
> call cexpl, (%out_arg1, %out_arg2, %out_arg3);
> [...]
> ret;
> }
>
> [...]
> // BEGIN GLOBAL FUNCTION DECL: cexpl
> .extern .func cexpl (.param.u64 %in_ar0, .param.f64 %in_ar1,
> .param.f64 %in_ar2);
> [...]
>
> We see the '.visible .func cexpl' declaration and definition for the
> libgfortran replacement function and in the same compilation unit also
> the '.extern .func cexpl' declaration that implicitly gets introduced via
> the 'sincos' transformation (via the GCC/nvptx back end emitting an
> explicit declaration of any function referenced), and 'ptxas' then
> (rightfully so) complains about that mismatch:
>
> ptxas c99_functions.o, line 35; error   : Inconsistent redefinition of
> variable 'cexpl'
> ptxas fatal   : Ptx assembly aborted due to errors
> nvptx-as: ptxas returned 255 exit status
> make[2]: *** [c99_functions.lo] Error 1
>
> ---
>  newlib/libc/include/complex.h | 35 ---
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/newlib/libc/include/complex.h b/newlib/libc/include/complex.h
> index 0a3ea97ed..ad3028e4c 100644
> --- a/newlib/libc/include/complex.h
> +++ b/newlib/libc/include/complex.h
> @@ -20,6 +2

[newlib] Generally make all 'long double complex' methods available in

2022-11-08 Thread Thomas Schwinge
..., not just '#if defined(__CYGWIN__)'.  (Exception: 'clog10l' which currently
indeed is for Cygwin only.)

This completes 2017-07-05 commit be3ca3947402827aa52709e677369bc7ad30aa1d
"Fixed warnings for some long double complex methods" after Aditya Upadhyay's
work on importing "Long double complex methods" from NetBSD.

For example, this changes GCC/nvptx libgfortran 'configure' output as follows:

[...]
checking for ccosf... yes
checking for ccos... yes
checking for ccosl... [-no-]{+yes+}
[...]

..., and correspondingly GCC/nvptx 'nvptx-none/libgfortran/config.h' as
follows:

[...]
 /* Define to 1 if you have the `ccosl' function. */
-/* #undef HAVE_CCOSL */
+#define HAVE_CCOSL 1
[...]

Similarly for 'ccoshl', 'cexpl', 'cpowl', 'csinl', 'csinhl', 'ctanl', 'ctanhl',
'cacoshl', 'cacosl', 'casinhl', 'catanhl'.  ('conjl', 'cprojl' are not
currently being used in libgfortran.)

This in turn simplifies GCC/nvptx 'libgfortran/intrinsics/c99_functions.c'
compilation such that this files doesn't have to provide its own
"Implementation of various C99 functions" for those, when in fact they're
available in newlib libm.
---

A few more words on why this is relevant for GCC.

For example, 'cexpl' usually is provided by libm, but if it isn't, the
open-coded replacement function in
'libgfortran/intrinsics/c99_functions.c' is effective if it holds that
'defined(HAVE_COSL) && defined(HAVE_SINL) && defined(HAVE_EXPL)':

long double complex
cexpl (long double complex z)
{
  long double a, b;
  long double complex v;

  a = REALPART (z);
  b = IMAGPART (z);
  COMPLEX_ASSIGN (v, cosl (b), sinl (b));
  return expl (a) * v;
}

This replacement code is active for current GCC/nvptx (... if no longer
compiling GCC/nvptx libgfortran in "minimal" mode, 'LIBGFOR_MINIMAL',
which I'm currently working on).

Comparing the preceeding to the 'c99_functions.c.188t.sincos' dump, we see for
that function:

 __attribute__((nothrow, leaf, const))
 complex long double cexpl (complex long double z)
 {
   long double b;
   long double a;
   long double _1;
   long double _2;
   long double _4;
   long double _5;
   long double _11;
+  complex long double sincostmp_13;

[local count: 1073741824]:
   a_7 = REALPART_EXPR ;
   b_8 = IMAGPART_EXPR ;
-  _1 = cosl (b_8);
-  _2 = sinl (b_8);
+  sincostmp_13 = __builtin_cexpil (b_8);
+  _1 = REALPART_EXPR ;
+  _2 = IMAGPART_EXPR ;
   _11 = expl (a_7);
   _4 = _1 * _11;
   _5 = _2 * _11;
   REALPART_EXPR <> = _4;
   IMAGPART_EXPR <> = _5;
   return ;

 }

That is, the 'cosl (b)', 'sinl (b)' sequence is replaced by
'__builtin_cexpil'.  That '__builtin_cexpil' is then later mapped back
into: 'cexpl'.  We've now got an infinitely-recursive 'cexpl' replacement
function, "implemented via itself"; GCC/nvptx libgfortran assumes there
is no 'cexpl' in libm, whereas this 'sincos' transformation does assume
that there is.  (..., which looks like an additional bug on its own.)

At the PTX-level, this leads to the following:

[...]
// BEGIN GLOBAL FUNCTION DECL: cexpl
.visible .func cexpl (.param.u64 %in_ar0, .param.f64 %in_ar1, .param.f64 
%in_ar2);

// BEGIN GLOBAL FUNCTION DEF: cexpl
.visible .func cexpl (.param.u64 %in_ar0, .param.f64 %in_ar1, .param.f64 
%in_ar2)
{
[...]
call cexpl, (%out_arg1, %out_arg2, %out_arg3);
[...]
ret;
}

[...]
// BEGIN GLOBAL FUNCTION DECL: cexpl
.extern .func cexpl (.param.u64 %in_ar0, .param.f64 %in_ar1, .param.f64 
%in_ar2);
[...]

We see the '.visible .func cexpl' declaration and definition for the
libgfortran replacement function and in the same compilation unit also
the '.extern .func cexpl' declaration that implicitly gets introduced via
the 'sincos' transformation (via the GCC/nvptx back end emitting an
explicit declaration of any function referenced), and 'ptxas' then
(rightfully so) complains about that mismatch:

ptxas c99_functions.o, line 35; error   : Inconsistent redefinition of 
variable 'cexpl'
ptxas fatal   : Ptx assembly aborted due to errors
nvptx-as: ptxas returned 255 exit status
make[2]: *** [c99_functions.lo] Error 1

---
 newlib/libc/include/complex.h | 35 ---
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/newlib/libc/include/complex.h b/newlib/libc/include/complex.h
index 0a3ea97ed..ad3028e4c 100644
--- a/newlib/libc/include/complex.h
+++ b/newlib/libc/include/complex.h
@@ -20,6 +20,7 @@ __BEGIN_DECLS
 /* 7.3.5.1 The cacos functions */
 double complex cacos(double complex);
 float complex cacosf(float complex);
+long double complex cacosl(long double complex);

 /* 7.3.5.2 The casin functions */
 double complex casin(double complex);
@@ -34,44 +35,54 @@ long double complex catanl(long double complex);
 /* 7.3.5.1