On Sun, Jun 25, 2017 at 1:28 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
> +(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.
Right. Fixed.
>
> 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).
Yes I noticed that while working on a different patch related to
copysign; The generic version of a*copysign(1.0, b) [see the other
thread where the ARM folks started a patch for it; yes it was by pure
accident that I was working on this and really did not notice that
thread until yesterday].
I was looking into a nice way of creating copysign without having to
do the switch but I could not find one. In the end I copied was done
already in a different location in match.pd; this is also the reason
why I had the build_one_cst there.
>
> + /* 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_minus_onep real_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); } (outn @0)))
> + (if (types_match (type, double_type_node))
> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0)))
> + (if (types_match (type, long_double_type_node))
> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0)))))))
> +
> +/* Transform X * copysign (1.0, X) into abs(X). */
> +(simplify
> + (mult:c @0 (COPYSIGN real_onep @0))
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
> + (abs @0)))
>
> I would have expected it do to the right thing for signed zero and qNaN. Can
> you describe a case where it would give the wrong result, or are the
> conditions just conservative?
I was just being conservative; maybe too conservative but I was a bit
worried I could get it incorrect.
>
> +/* Transform X * copysign (1.0, -X) into -abs(X). */
> +(simplify
> + (mult:c @0 (COPYSIGN real_onep (negate @0)))
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
> + (negate (abs @0))))
> +
> +/* Transform copysign (-1.0, X) into copysign (1.0, X). */
> +(simplify
> + (COPYSIGN real_minus_onep @0)
> + (COPYSIGN { build_one_cst (type); } @0))
>
> (simplify
> (COPYSIGN REAL_CST@0 @1)
> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
> (COPYSIGN (negate @0) @1)))
> ? Or does that create trouble with sNaN and only the 1.0 case is worth
> the trouble?
No that is the correct way; I Noticed the other thread about copysign
had something similar as what should be done too.
I will send out a new patch after testing soon.
Thanks,
Andrew
>
> --
> Marc Glisse
