[PATCH] Avoid *ORDERED_EXPRs in the IL if !HONOR_NANS (PR target/92449)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Avoid *ORDERED_EXPRs in the IL if !HONOR_NANS (PR target/92449)

Jakub Jelinek
Hi!

The testcase from the PR ICEs on rs6000.  For __builtin_isunordered etc.
the folding makes sure that there is no *ORDERED_EXPR etc. in the IL
if !HONOR_NANS, but the complex multiplication can emit it when mixing
-Ofast with -fno-cx-limited-range.
The following patch makes sure we don't emit it even in that case.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I'll defer the addition of the testcase and rs6000 changes to Segher.

2019-11-11  Jakub Jelinek  <[hidden email]>

        PR target/92449
        * tree-complex.c (expand_complex_multiplication): If !HONOR_NANS,
        don't emit UNORDERED_EXPR guarded libcall.  Formatting fixes.

--- gcc/tree-complex.c.jj 2019-01-10 11:43:02.252577041 +0100
+++ gcc/tree-complex.c 2019-11-11 20:26:28.585315282 +0100
@@ -1144,6 +1144,16 @@ expand_complex_multiplication (gimple_st
       return;
     }
 
+  if (!HONOR_NANS (inner_type))
+    {
+      /* If we are not worrying about NaNs expand to
+ (ar*br - ai*bi) + i(ar*bi + br*ai) directly.  */
+      expand_complex_multiplication_components (gsi, inner_type,
+ ar, ai, br, bi,
+ &rr, &ri);
+      break;
+    }
+
   /* Else, expand x = a * b into
      x = (ar*br - ai*bi) + i(ar*bi + br*ai);
      if (isunordered (__real__ x, __imag__ x))
@@ -1151,7 +1161,7 @@ expand_complex_multiplication (gimple_st
 
   tree tmpr, tmpi;
   expand_complex_multiplication_components (gsi, inner_type, ar, ai,
-     br, bi, &tmpr, &tmpi);
+    br, bi, &tmpr, &tmpi);
 
   gimple *check
     = gimple_build_cond (UNORDERED_EXPR, tmpr, tmpi,
@@ -1167,13 +1177,12 @@ expand_complex_multiplication (gimple_st
     = insert_cond_bb (gsi_bb (*gsi), gsi_stmt (*gsi), check,
       profile_probability::very_unlikely ());
 
-
   gimple_stmt_iterator cond_bb_gsi = gsi_last_bb (cond_bb);
   gsi_insert_after (&cond_bb_gsi, gimple_build_nop (), GSI_NEW_STMT);
 
   tree libcall_res
     = expand_complex_libcall (&cond_bb_gsi, type, ar, ai, br,
-       bi, MULT_EXPR, false);
+      bi, MULT_EXPR, false);
   tree cond_real = gimplify_build1 (&cond_bb_gsi, REALPART_EXPR,
     inner_type, libcall_res);
   tree cond_imag = gimplify_build1 (&cond_bb_gsi, IMAGPART_EXPR,
@@ -1190,20 +1199,18 @@ expand_complex_multiplication (gimple_st
   edge orig_to_join = find_edge (orig_bb, join_bb);
 
   gphi *real_phi = create_phi_node (rr, gsi_bb (*gsi));
-  add_phi_arg (real_phi, cond_real, cond_to_join,
- UNKNOWN_LOCATION);
+  add_phi_arg (real_phi, cond_real, cond_to_join, UNKNOWN_LOCATION);
   add_phi_arg (real_phi, tmpr, orig_to_join, UNKNOWN_LOCATION);
 
   gphi *imag_phi = create_phi_node (ri, gsi_bb (*gsi));
-  add_phi_arg (imag_phi, cond_imag, cond_to_join,
- UNKNOWN_LOCATION);
+  add_phi_arg (imag_phi, cond_imag, cond_to_join, UNKNOWN_LOCATION);
   add_phi_arg (imag_phi, tmpi, orig_to_join, UNKNOWN_LOCATION);
  }
       else
  /* If we are not worrying about NaNs expand to
   (ar*br - ai*bi) + i(ar*bi + br*ai) directly.  */
  expand_complex_multiplication_components (gsi, inner_type, ar, ai,
-      br, bi, &rr, &ri);
+  br, bi, &rr, &ri);
       break;
 
     default:

        Jakub

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Avoid *ORDERED_EXPRs in the IL if !HONOR_NANS (PR target/92449)

Richard Biener
On Tue, 12 Nov 2019, Jakub Jelinek wrote:

> Hi!
>
> The testcase from the PR ICEs on rs6000.  For __builtin_isunordered etc.
> the folding makes sure that there is no *ORDERED_EXPR etc. in the IL
> if !HONOR_NANS, but the complex multiplication can emit it when mixing
> -Ofast with -fno-cx-limited-range.
> The following patch makes sure we don't emit it even in that case.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK, but IMHO it is not a good idea to assert UNORDERED_EXPR cannot
appear with !HONOR_NANS, is it?

Richard.

> I'll defer the addition of the testcase and rs6000 changes to Segher.
>
> 2019-11-11  Jakub Jelinek  <[hidden email]>
>
> PR target/92449
> * tree-complex.c (expand_complex_multiplication): If !HONOR_NANS,
> don't emit UNORDERED_EXPR guarded libcall.  Formatting fixes.
>
> --- gcc/tree-complex.c.jj 2019-01-10 11:43:02.252577041 +0100
> +++ gcc/tree-complex.c 2019-11-11 20:26:28.585315282 +0100
> @@ -1144,6 +1144,16 @@ expand_complex_multiplication (gimple_st
>        return;
>      }
>  
> +  if (!HONOR_NANS (inner_type))
> +    {
> +      /* If we are not worrying about NaNs expand to
> + (ar*br - ai*bi) + i(ar*bi + br*ai) directly.  */
> +      expand_complex_multiplication_components (gsi, inner_type,
> + ar, ai, br, bi,
> + &rr, &ri);
> +      break;
> +    }
> +
>    /* Else, expand x = a * b into
>       x = (ar*br - ai*bi) + i(ar*bi + br*ai);
>       if (isunordered (__real__ x, __imag__ x))
> @@ -1151,7 +1161,7 @@ expand_complex_multiplication (gimple_st
>  
>    tree tmpr, tmpi;
>    expand_complex_multiplication_components (gsi, inner_type, ar, ai,
> -     br, bi, &tmpr, &tmpi);
> +    br, bi, &tmpr, &tmpi);
>  
>    gimple *check
>      = gimple_build_cond (UNORDERED_EXPR, tmpr, tmpi,
> @@ -1167,13 +1177,12 @@ expand_complex_multiplication (gimple_st
>      = insert_cond_bb (gsi_bb (*gsi), gsi_stmt (*gsi), check,
>        profile_probability::very_unlikely ());
>  
> -
>    gimple_stmt_iterator cond_bb_gsi = gsi_last_bb (cond_bb);
>    gsi_insert_after (&cond_bb_gsi, gimple_build_nop (), GSI_NEW_STMT);
>  
>    tree libcall_res
>      = expand_complex_libcall (&cond_bb_gsi, type, ar, ai, br,
> -       bi, MULT_EXPR, false);
> +      bi, MULT_EXPR, false);
>    tree cond_real = gimplify_build1 (&cond_bb_gsi, REALPART_EXPR,
>      inner_type, libcall_res);
>    tree cond_imag = gimplify_build1 (&cond_bb_gsi, IMAGPART_EXPR,
> @@ -1190,20 +1199,18 @@ expand_complex_multiplication (gimple_st
>    edge orig_to_join = find_edge (orig_bb, join_bb);
>  
>    gphi *real_phi = create_phi_node (rr, gsi_bb (*gsi));
> -  add_phi_arg (real_phi, cond_real, cond_to_join,
> - UNKNOWN_LOCATION);
> +  add_phi_arg (real_phi, cond_real, cond_to_join, UNKNOWN_LOCATION);
>    add_phi_arg (real_phi, tmpr, orig_to_join, UNKNOWN_LOCATION);
>  
>    gphi *imag_phi = create_phi_node (ri, gsi_bb (*gsi));
> -  add_phi_arg (imag_phi, cond_imag, cond_to_join,
> - UNKNOWN_LOCATION);
> +  add_phi_arg (imag_phi, cond_imag, cond_to_join, UNKNOWN_LOCATION);
>    add_phi_arg (imag_phi, tmpi, orig_to_join, UNKNOWN_LOCATION);
>   }
>        else
>   /* If we are not worrying about NaNs expand to
>    (ar*br - ai*bi) + i(ar*bi + br*ai) directly.  */
>   expand_complex_multiplication_components (gsi, inner_type, ar, ai,
> -      br, bi, &rr, &ri);
> +  br, bi, &rr, &ri);
>        break;
>  
>      default:
>
> Jakub
>
>
--
Richard Biener <[hidden email]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Avoid *ORDERED_EXPRs in the IL if !HONOR_NANS (PR target/92449)

Segher Boessenkool
On Tue, Nov 12, 2019 at 08:47:55AM +0100, Richard Biener wrote:
> OK, but IMHO it is not a good idea to assert UNORDERED_EXPR cannot
> appear with !HONOR_NANS, is it?

If we allow that, we should allow UNLT/UNGT/UNLE/UNGE/UNEQ/LTGT as well,
which doesn't make much sense, especially because LTGT then is the same
as NE, but NE with HONOR_NANS means something different.

Without HONOR_NANS all FP comparisons (and FP math in most other aspects)
behaves like integers do.  Having to support ORDERED and UNORDERED for
!HONOR_NANS means we need a third codepath in many places.  Without this,
the !HONOR_NANS code is very close to the integer code, so it's not all
that bad.

Assert...  Should we have an assert in some strategic places that makes
sure we never try to create NaN stuff when NaNs are disabled?


Segher
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Avoid *ORDERED_EXPRs in the IL if !HONOR_NANS (PR target/92449)

Segher Boessenkool
In reply to this post by Jakub Jelinek
On Tue, Nov 12, 2019 at 01:06:55AM +0100, Jakub Jelinek wrote:
> The testcase from the PR ICEs on rs6000.  For __builtin_isunordered etc.
> the folding makes sure that there is no *ORDERED_EXPR etc. in the IL
> if !HONOR_NANS, but the complex multiplication can emit it when mixing
> -Ofast with -fno-cx-limited-range.
> The following patch makes sure we don't emit it even in that case.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> I'll defer the addition of the testcase and rs6000 changes to Segher.

Thanks for doing this.  Yeah, testcases + rs6000 builtin fix ready soon.


Segher
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Avoid *ORDERED_EXPRs in the IL if !HONOR_NANS (PR target/92449)

Richard Biener
In reply to this post by Segher Boessenkool
On Tue, 12 Nov 2019, Segher Boessenkool wrote:

> On Tue, Nov 12, 2019 at 08:47:55AM +0100, Richard Biener wrote:
> > OK, but IMHO it is not a good idea to assert UNORDERED_EXPR cannot
> > appear with !HONOR_NANS, is it?
>
> If we allow that, we should allow UNLT/UNGT/UNLE/UNGE/UNEQ/LTGT as well,
> which doesn't make much sense, especially because LTGT then is the same
> as NE, but NE with HONOR_NANS means something different.
>
> Without HONOR_NANS all FP comparisons (and FP math in most other aspects)
> behaves like integers do.  Having to support ORDERED and UNORDERED for
> !HONOR_NANS means we need a third codepath in many places.  Without this,
> the !HONOR_NANS code is very close to the integer code, so it's not all
> that bad.
>
> Assert...  Should we have an assert in some strategic places that makes
> sure we never try to create NaN stuff when NaNs are disabled?

We do have some asserts in buildN but I guess verify_gimple_comparison
might be a good place to catch things early.

Richard.