Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a)

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Marc Glisse <marc.glisse@inria.fr> writes:
> +(for cmp (gt ge lt le)
> +     outp (convert convert negate negate)
> +     outn (negate negate convert convert)
> + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
> + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
> + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
> + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
> + (simplify
> +  (cond (cmp @0 real_zerop) real_onep real_minus_onep)
> +  (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
> +       && types_match (type, TREE_TYPE (@0)))
> +   (switch
> +    (if (types_match (type, float_type_node))
> +     (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0)))
> +    (if (types_match (type, double_type_node))
> +     (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0)))
> +    (if (types_match (type, long_double_type_node))
> +     (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0))))))
>
> There is already a 1.0 of the right type in the input, it would be easier 
> to reuse it in the output than build a new one.
>
> Non-generic builtins like copysign are such a pain... We also end up 
> missing the 128-bit case that way (pre-existing problem, not your patch). 
> We seem to have a corresponding internal function, but apparently it is 
> not used until expansion (well, maybe during vectorization).

It should be OK to introduce uses of the internal functions whenever
it's useful.  The match code will check that the internal function is
implemented before allowing the transformation.

The idea was more-or-less:

- Leave calls to libm functions alone until expand if there's no
  particular benefit to converting them earlier.  This avoids introducing
  a gratuitous difference between targets that can and can't open-code the
  function.

- Fold calls to libm functions to calls to libm functions where
  possible, because these transformations work regardless of whether the
  function can be open-coded.

- When introducing new calls, use internal functions if we need to be
  sure that the target has an appropriate optab.

- Also use internal functions to represent the non-errno setting forms
  of an internal function, in cases where the built-in functions are
  assumed to set errno.

But IFN_COPYSIGN might not be useful as-is, since the copysign built-ins
are usually expanded without the help of an optab.  It should be possible
to change things so that IFN_COPYSIGN is supported in the same situations
as the built-in though.

Thanks,
Richard