On Mon, Sep 28, 2020 at 7:28 PM Tom de Vries <tdevr...@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
