[PATCH] Add type arg to TARGET_LIBC_HAS_FUNCTION

[Richard Biener]

On Mon, Sep 28, 2020 at 7:28 PM Tom de Vries <tdevries@suse.de> wrote:
>
> [ was: Re: [Patch][nvptx] return true in libc_has_function for
> function_sincos ]
>
> On 9/26/20 6:47 PM, Tobias Burnus wrote:
> > Found when looking at PR97203 (but having no effect there).
> >
> > The GCC ME optimizes with -O1 (or higher) the
> >   a = sinf(x)
> >   b = cosf(x)
> > to __builtin_cexpi(x, &a, &b)
> > (...i as in internal; like cexp(z) but with with __real__ z == 0)
> >
> >
> > In expand_builtin_cexpi, that is handles as:
> >   if (optab_handler (sincos_optab, mode) != CODE_FOR_nothing)
> >     ...
> >   else if (targetm.libc_has_function (function_sincos))
> >     ...
> >   else
> >         fn = builtin_decl_explicit (BUILT_IN_CEXPF);
> >
> > And the latter is done. As newlib's cexpf does not know that
> > __real__ z == 0, it calculates 'r = expf (__real__ z)' before
> > invoking sinf and cosf on __imag__ z.
> >
> > Thus, it is much faster to call 'sincosf', which also exists
> > in newlib.
> >
> > Solution: Return true for targetm.libc_has_function (function_sincos).
> >
> >
> > NOTE: With -funsafe-math-optimizations (-O0 or higher),
> > sinf/cosf and sincosf invoke .sin.approx/.cos/.approx instead of
> > doing a library call.
>
> This version takes care to enable sincos and sincosf, but not sincosl.
>
> Target hook changes OK for trunk?

@@ -9770,7 +9770,7 @@ fold_builtin_sincos (location_t loc,
     }
   if (!call)
     {
-      if (!targetm.libc_has_function (function_c99_math_complex)
+      if (!targetm.libc_has_function (function_c99_math_complex, NULL_TREE)

why pass NULL_TREE and not 'type' here?

          || !builtin_decl_implicit_p (fn))
        return NULL_TREE;

similar for the builtins.def change for the cases where math functions
are affected?  I guess it's a bit awkward to make it work there, so OK.

 bool
-darwin_libc_has_function (enum function_class fn_class)
+darwin_libc_has_function (enum function_class fn_class, tree type)
 {
-  if (fn_class == function_sincos)
+  if (type != NULL_TREE)
+    {
+      switch (fn_class)
+       {
+       case function_sincos:
+         break;
+       default:
+         /* Not implemented.  */
+         gcc_unreachable ();
+       }
+    }

huh.  I think special-casing this just for sincos is a bit awkward,
esp. ICEing for other queries with a type.  Specifically

-@deftypefn {Target Hook} bool TARGET_LIBC_HAS_FUNCTION (enum
function_class @var{fn_class})
+@deftypefn {Target Hook} bool TARGET_LIBC_HAS_FUNCTION (enum
function_class @var{fn_class}, tree @var{type})
 This hook determines whether a function from a class of functions
-@var{fn_class} is present in the target C library.
+@var{fn_class} is present in the target C library.  The @var{type} argument
+can be used to distinguish between float, double and long double versions.
 @end deftypefn

This doesn't mention we'll ICE for anything but sincos.  A sensible
semantics would be that if TYPE is NULL the caller asks for support
for all standard (float, double, long double) types while with TYPE
non-NULL it can ask for a specific type including for example the
new _FloatN, etc. types.

Richard.

> Thanks,
> - Tom