Re: [PR71078] x / abs(x) -> copysign (1.0, x)

[Richard Sandiford <richard dot sandiford at arm dot com>]

Richard Biener <rguenther@suse.de> writes:
> On Mon, 25 Jul 2016, Prathamesh Kulkarni wrote:
>
>> Hi,
>> The attached patch tries to fix PR71078.
>> I am not sure if I have got the converts right.
>> I put (convert? @0) and (convert1? (abs @1))
>> to match for cases when operands's types may
>> be different from outermost type like in pr71078-3.c
>
> Types of RDIV_EXPR have to be the same so as you have a
> match on @0 the converts need to be either both present
> or not present.
>
> +  (if (FLOAT_TYPE_P (type)
>
> as you special-case several types below please use SCALAR_FLOAT_TYPE_P
> here.
>
> +       && ! HONOR_NANS (type)
> +       && ! HONOR_INFINITIES (type))
> +   (switch
> +    (if (type == float_type_node)
> +     (BUILT_IN_COPYSIGNF { build_one_cst (type); } (convert @0)))
>
> please use if (types_match (type, float_type_node)) instead of
> pointer equality.

IMO it'd be better to have some way of using mathfn_builtin from match.pd.

> I _think_ you can do better here by using
> IFN_COPYSIGN but possibly only so if the target supports it.
> Richard - this seems to be the first pattern in need of
> generating a builtin where no other was there to match the type
> to - any idea how we can safely use the internal function here?

I don't think there's any benefit to using the internal function here.
At the moment we only use internal functions before cfgexpand if they
let us do something that we couldn't do otherwise, such as vectorising a
function or avoiding unnecessary effects on errno.  All other cases are
handled by cfgexpand itself.

> I see those do not have an expander that would fall back to
> expanding the regular builtin, correct?

Right.  In:

    https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01057.html

you weren't keen on the idea of tree codes introducing new dependencies
on libm, so I carried that priniple over to the internal functions
(which are really just extended tree codes).  I suppose copysign is
a special case since we can always open code it, but in general we
shouldn't fall back to something that could generate a call.

Thanks,
Richard