Re: [PATCH] MATCH: [PR111109] Fix bit_ior(cond, cond) when comparisons are fp

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Aug 23, 2023 at 11:51 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The patterns that were added in r13-4620-g4d9db4bdd458, missed that
> (a > b) and (a <= b) are not inverse of each other for floating point
> comparisons (if NaNs are supported). Even though there was a check for
> intergal types, it was only for the result of the cond rather for the
> type of what is being compared. The fix is to check to see if cmp and
> icmp are inverse of each other by using the invert_tree_comparison function.
>
> OK for trunk and GCC 13 branch? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

Thanks,
Richard.

> I added the testcase to execute/ieee as it requires support for NAN.
>
>         PR tree-optimization/111109
>
> gcc/ChangeLog:
>
>         * match.pd (ior(cond,cond), ior(vec_cond,vec_cond)):
>         Add check to make sure cmp and icmp are inverse.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.c-torture/execute/ieee/fp-cmp-cond-1.c: New test.
> ---
>  gcc/match.pd                                  | 11 ++-
>  .../execute/ieee/fp-cmp-cond-1.c              | 78 +++++++++++++++++++
>  2 files changed, 86 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.c-torture/execute/ieee/fp-cmp-cond-1.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 85b7d323a19..b666d73b189 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2087,6 +2087,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>     (bit_and:c (convert? (cmp@0  @01 @02)) @3)
>     (bit_and:c (convert? (icmp@4 @01 @02)) @5))
>      (if (INTEGRAL_TYPE_P (type)
> +        && invert_tree_comparison (cmp, HONOR_NANS (@01)) == icmp
>          /* The scalar version has to be canonicalized after vectorization
>             because it makes unconditional loads conditional ones, which
>             means we lose vectorization because the loads may trap.  */
> @@ -2101,6 +2102,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>     (cond (cmp@0  @01 @02) @3 zerop)
>     (cond (icmp@4 @01 @02) @5 zerop))
>      (if (INTEGRAL_TYPE_P (type)
> +        && invert_tree_comparison (cmp, HONOR_NANS (@01)) == icmp
>          /* The scalar version has to be canonicalized after vectorization
>             because it makes unconditional loads conditional ones, which
>             means we lose vectorization because the loads may trap.  */
> @@ -2113,13 +2115,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (bit_ior
>     (bit_and:c (vec_cond:s (cmp@0 @6 @7) @4 @5) @2)
>     (bit_and:c (vec_cond:s (icmp@1 @6 @7) @4 @5) @3))
> -    (if (integer_zerop (@5))
> +    (if (integer_zerop (@5)
> +        && invert_tree_comparison (cmp, HONOR_NANS (@6)) == icmp)
>       (switch
>        (if (integer_onep (@4))
>         (bit_and (vec_cond @0 @2 @3) @4))
>         (if (integer_minus_onep (@4))
>          (vec_cond @0 @2 @3)))
> -    (if (integer_zerop (@4))
> +    (if (integer_zerop (@4)
> +        && invert_tree_comparison (cmp, HONOR_NANS (@6)) == icmp)
>       (switch
>        (if (integer_onep (@5))
>         (bit_and (vec_cond @0 @3 @2) @5))
> @@ -2132,7 +2136,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (bit_ior
>     (vec_cond:s (cmp@0 @4 @5) @2 integer_zerop)
>     (vec_cond:s (icmp@1 @4 @5) @3 integer_zerop))
> -    (vec_cond @0 @2 @3)))
> +  (if (invert_tree_comparison (cmp, HONOR_NANS (@4)) == icmp)
> +   (vec_cond @0 @2 @3))))
>
>  /* Transform X & -Y into X * Y when Y is { 0 or 1 }.  */
>  (simplify
> diff --git a/gcc/testsuite/gcc.c-torture/execute/ieee/fp-cmp-cond-1.c b/gcc/testsuite/gcc.c-torture/execute/ieee/fp-cmp-cond-1.c
> new file mode 100644
> index 00000000000..4a3c4b0eee2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/ieee/fp-cmp-cond-1.c
> @@ -0,0 +1,78 @@
> +/* PR tree-optimization/111109 */
> +
> +/*
> +   f should return 0 if either fa and fb are a nan.
> +   Rather than the value of a or b.
> +*/
> +__attribute__((noipa))
> +int f(int a, int b, float fa, float fb) {
> +  const _Bool c = fa < fb;
> +  const _Bool c1 = fa >= fb;
> +  return (c * a) | (c1 * b);
> +}
> +
> +/*
> +   f1 should return 0 if either fa and fb are a nan.
> +   Rather than the value of a&1 or b&1.
> +*/
> +__attribute__((noipa))
> +int f1(int a, int b, float fa, float fb) {
> +  const _Bool c = fa < fb;
> +  const _Bool c1 = fa >= fb;
> +  return (c & a) | (c1 & b);
> +}
> +
> +#if __SIZEOF_INT__ == __SIZEOF_FLOAT__
> +typedef int v4si __attribute__ ((vector_size (1*sizeof(int))));
> +typedef float v4sf __attribute__ ((vector_size (1*sizeof(float))));
> +/*
> +   fvf0 should return {0} if either fa and fb are a nan.
> +   Rather than the value of a or b.
> +*/
> +__attribute__((noipa))
> +v4si vf0(v4si a, v4si b, v4sf fa, v4sf fb) {
> +  const v4si c = fa < fb;
> +  const v4si c1 = fa >= fb;
> +  return (c & a) | (c1 & b);
> +}
> +
> +
> +#endif
> +
> +int main(void)
> +{
> +  float a = __builtin_nan("");
> +
> +  if (f(-1,-1, a, a) != 0)
> +    __builtin_abort();
> +  if (f(-1,-1, a, 0) != 0)
> +    __builtin_abort();
> +  if (f(-1,-1, 0, a) != 0)
> +    __builtin_abort();
> +  if (f(-1,-1, 0, 0) != -1)
> +    __builtin_abort();
> +
> +
> +  if (f1(1,1, a, a) != 0)
> +    __builtin_abort();
> +  if (f1(1,1, a, 0) != 0)
> +    __builtin_abort();
> +  if (f1(1,1, 0, a) != 0)
> +    __builtin_abort();
> +  if (f1(1,1, 0, 0) != 1)
> +        __builtin_abort();
> +
> +#if __SIZEOF_INT__ == __SIZEOF_FLOAT__
> +  v4si b = {-1};
> +  v4sf c = {a};
> +  v4sf d = {0.0};
> +  if (vf0(b,b, c, c)[0] != 0)
> +    __builtin_abort();
> +  if (vf0(b,b, c, d)[0] != 0)
> +    __builtin_abort();
> +  if (vf0(b,b, d, c)[0] != 0)
> +    __builtin_abort();
> +  if (vf0(b,b, d, d)[0] != b[0])
> +        __builtin_abort();
> +#endif
> +}
> --
> 2.31.1
>