diff mbox

PR68264: Use unordered comparisons for tree-call-cdce.c

Message ID 87y4e2kwcp.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford Nov. 13, 2015, 10:49 a.m. UTC
As reported in PR 68264, tree-call-cdce.c should be using unordered
comparisons for the range checks, in order to avoid raising FE_INVALID
for quiet NaNs.

Tested on x86_64-linux-gnu and aarch64-linux-gnu.  The test failed on
aarch64-linux-gnu before the patch, but it didn't on x86_64-linux-gnu
because it used unordered comparisons for the previous ordered tree codes.

OK to install?

Thanks,
Richard


gcc/
	PR tree-optimization/68264
	* tree-call-cdce.c (gen_one_condition): Update commentary.
	(gen_conditions_for_pow_int_base): Invert the sense of the tests
	passed to gen_one_condition.
	(gen_conditions_for_domain): Likewise.  Use unordered comparisons.
	(shrink_wrap_one_built_in_call): Invert the sense of the tests,
	using EDGE_FALSE_VALUE for edges to the call block and
	EDGE_TRUE_VALUE for the others.

gcc/testsuite/
	PR tree-optimization/68264
	* gcc.dg/torture/pr68264.c: New test.

Comments

Richard Biener Nov. 13, 2015, 11:21 a.m. UTC | #1
On Fri, Nov 13, 2015 at 11:49 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> As reported in PR 68264, tree-call-cdce.c should be using unordered
> comparisons for the range checks, in order to avoid raising FE_INVALID
> for quiet NaNs.
>
> Tested on x86_64-linux-gnu and aarch64-linux-gnu.  The test failed on
> aarch64-linux-gnu before the patch, but it didn't on x86_64-linux-gnu
> because it used unordered comparisons for the previous ordered tree codes.
>
> OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         PR tree-optimization/68264
>         * tree-call-cdce.c (gen_one_condition): Update commentary.
>         (gen_conditions_for_pow_int_base): Invert the sense of the tests
>         passed to gen_one_condition.
>         (gen_conditions_for_domain): Likewise.  Use unordered comparisons.
>         (shrink_wrap_one_built_in_call): Invert the sense of the tests,
>         using EDGE_FALSE_VALUE for edges to the call block and
>         EDGE_TRUE_VALUE for the others.
>
> gcc/testsuite/
>         PR tree-optimization/68264
>         * gcc.dg/torture/pr68264.c: New test.
>
> diff --git a/gcc/testsuite/gcc.dg/torture/pr68264.c b/gcc/testsuite/gcc.dg/torture/pr68264.c
> new file mode 100644
> index 0000000..c4e85e7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr68264.c
> @@ -0,0 +1,102 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target fenv_exceptions } */
> +
> +#include <fenv.h>
> +#include <math.h>
> +#include <errno.h>
> +
> +extern void abort (void) __attribute__ ((noreturn));
> +
> +#define LARGE_NEG_MAYBE_ERANGE 0x01
> +#define LARGE_NEG_ERANGE       0x02
> +#define LARGE_POS_ERANGE       0x04
> +#define LARGE_NEG_EDOM         0x08
> +#define LARGE_POS_EDOM         0x10
> +
> +#define LARGE_ERANGE (LARGE_NEG_ERANGE | LARGE_POS_ERANGE)
> +#define LARGE_EDOM (LARGE_NEG_EDOM | LARGE_POS_EDOM)
> +#define POWER_ERANGE (LARGE_NEG_MAYBE_ERANGE | LARGE_POS_ERANGE)
> +
> +#define TEST(CALL, FLAGS) (CALL, tester (FLAGS))
> +
> +volatile double d;
> +volatile int i;
> +
> +static void (*tester) (int);
> +
> +void
> +check_quiet_nan (int flags __attribute__ ((unused)))
> +{
> +  if (fetestexcept (FE_ALL_EXCEPT))
> +    abort ();
> +  if (errno)
> +    abort ();
> +}
> +
> +void
> +check_large_neg (int flags)
> +{
> +  if (flags & LARGE_NEG_MAYBE_ERANGE)
> +    return;
> +  int expected_errno = (flags & LARGE_NEG_ERANGE ? ERANGE
> +                       : flags & LARGE_NEG_EDOM ? EDOM
> +                       : 0);
> +  if (expected_errno != errno)
> +    abort ();
> +  errno = 0;
> +}
> +
> +void
> +check_large_pos (int flags)
> +{
> +  int expected_errno = (flags & LARGE_POS_ERANGE ? ERANGE
> +                       : flags & LARGE_POS_EDOM ? EDOM
> +                       : 0);
> +  if (expected_errno != errno)
> +    abort ();
> +  errno = 0;
> +}
> +
> +void
> +test (void)
> +{
> +  TEST (acos (d), LARGE_EDOM);
> +  TEST (asin (d), LARGE_EDOM);
> +  TEST (acosh (d), LARGE_NEG_EDOM);
> +  TEST (atanh (d), LARGE_EDOM);
> +  TEST (cosh (d), LARGE_ERANGE);
> +  TEST (sinh (d), LARGE_ERANGE);
> +  TEST (log (d), LARGE_NEG_EDOM);
> +  TEST (log2 (d), LARGE_NEG_EDOM);
> +  TEST (log10 (d), LARGE_NEG_EDOM);
> +  /* Disabled due to glibc PR 6792, fixed in Apr 2015.  */
> +  if (0)
> +    TEST (log1p (d), LARGE_NEG_EDOM);
> +  TEST (exp (d), POWER_ERANGE);
> +  TEST (exp2 (d), POWER_ERANGE);
> +  TEST (expm1 (d), POWER_ERANGE);
> +  TEST (sqrt (d), LARGE_NEG_EDOM);
> +  TEST (pow (100.0, d), POWER_ERANGE);
> +  TEST (pow (i, d), POWER_ERANGE);
> +}
> +
> +int
> +main (void)
> +{
> +  errno = 0;
> +  i = 100;
> +  d = __builtin_nan ("");
> +  tester = check_quiet_nan;
> +  feclearexcept (FE_ALL_EXCEPT);
> +  test ();
> +
> +  d = -1.0e80;
> +  tester = check_large_neg;
> +  errno = 0;
> +  test ();
> +
> +  d = 1.0e80;
> +  tester = check_large_pos;
> +  errno = 0;
> +  test ();
> +}
> diff --git a/gcc/tree-call-cdce.c b/gcc/tree-call-cdce.c
> index a5f38ce..fbcc70b 100644
> --- a/gcc/tree-call-cdce.c
> +++ b/gcc/tree-call-cdce.c
> @@ -51,10 +51,11 @@ along with GCC; see the file COPYING3.  If not see
>               built_in_call (args)
>
>      An actual simple example is :
> -         log (x);   // Mostly dead call
> +        log (x);   // Mostly dead call
>       ==>
> -         if (x <= 0)
> -             log (x);
> +        if (__builtin_islessequal (x, 0))
> +            log (x);
> +
>       With this change, call to log (x) is effectively eliminated, as
>       in majority of the cases, log won't be called with x out of
>       range.  The branch is totally predictable, so the branch cost
> @@ -306,15 +307,13 @@ is_call_dce_candidate (gcall *call)
>  }
>
>
> -/* A helper function to generate gimple statements for
> -   one bound comparison.  ARG is the call argument to
> -   be compared with the bound, LBUB is the bound value
> -   in integer, TCODE is the tree_code of the comparison,
> -   TEMP_NAME1/TEMP_NAME2 are names of the temporaries,
> -   CONDS is a vector holding the produced GIMPLE statements,
> -   and NCONDS points to the variable holding the number
> -   of logical comparisons.  CONDS is either empty or
> -   a list ended with a null tree.  */
> +/* A helper function to generate gimple statements for one bound
> +   comparison, so that the built-in function is called whenever
> +   TCODE <ARG, LBUB> is *false*.  TEMP_NAME1/TEMP_NAME2 are names
> +   of the temporaries, CONDS is a vector holding the produced GIMPLE
> +   statements, and NCONDS points to the variable holding the number of
> +   logical comparisons.  CONDS is either empty or a list ended with a
> +   null tree.  */
>
>  static void
>  gen_one_condition (tree arg, int lbub,
> @@ -371,7 +370,7 @@ gen_conditions_for_domain (tree arg, inp_domain domain,
>    if (domain.has_lb)
>      gen_one_condition (arg, domain.lb,
>                         (domain.is_lb_inclusive
> -                        ? LT_EXPR : LE_EXPR),
> +                        ? UNGE_EXPR : UNGT_EXPR),
>                         "DCE_COND_LB", "DCE_COND_LB_TEST",
>                         conds, nconds);
>
> @@ -383,7 +382,7 @@ gen_conditions_for_domain (tree arg, inp_domain domain,
>
>        gen_one_condition (arg, domain.ub,
>                           (domain.is_ub_inclusive
> -                          ? GT_EXPR : GE_EXPR),
> +                          ? UNLE_EXPR : UNLT_EXPR),
>                           "DCE_COND_UB", "DCE_COND_UB_TEST",
>                           conds, nconds);
>      }
> @@ -395,7 +394,7 @@ gen_conditions_for_domain (tree arg, inp_domain domain,
>     See candidate selection in check_pow.  Since the
>     candidates' base values have a limited range,
>     the guarded code generated for y are simple:
> -   if (y > max_y)
> +   if (__builtin_isgreater (y, max_y))
>       pow (const, y);
>     Note max_y can be computed separately for each
>     const base, but in this implementation, we
> @@ -480,11 +479,11 @@ gen_conditions_for_pow_int_base (tree base, tree expn,
>    /* For pow ((double)x, y), generate the following conditions:
>       cond 1:
>       temp1 = x;
> -     if (temp1 <= 0)
> +     if (__builtin_islessequal (temp1, 0))
>
>       cond 2:
>       temp2 = y;
> -     if (temp2 > max_exp_real_cst)  */
> +     if (__builtin_isgreater (temp2, max_exp_real_cst))  */
>
>    /* Generate condition in reverse order -- first
>       the condition for the exp argument.  */
> @@ -508,7 +507,7 @@ gen_conditions_for_pow_int_base (tree base, tree expn,
>    stmt1 = gimple_build_assign (temp, base_val0);
>    tempn = make_ssa_name (temp, stmt1);
>    gimple_assign_set_lhs (stmt1, tempn);
> -  stmt2 = gimple_build_cond (LE_EXPR, tempn, cst0, NULL_TREE, NULL_TREE);
> +  stmt2 = gimple_build_cond (GT_EXPR, tempn, cst0, NULL_TREE, NULL_TREE);
>
>    conds.quick_push (stmt1);
>    conds.quick_push (stmt2);
> @@ -797,11 +796,11 @@ shrink_wrap_one_built_in_call (gcall *bi_call)
>
>    bi_call_in_edge0 = split_block (bi_call_bb, cond_expr);
>    bi_call_in_edge0->flags &= ~EDGE_FALLTHRU;
> -  bi_call_in_edge0->flags |= EDGE_TRUE_VALUE;
> +  bi_call_in_edge0->flags |= EDGE_FALSE_VALUE;
>    guard_bb = bi_call_bb;
>    bi_call_bb = bi_call_in_edge0->dest;
>    join_tgt_in_edge_fall_thru = make_edge (guard_bb, join_tgt_bb,
> -                                          EDGE_FALSE_VALUE);
> +                                          EDGE_TRUE_VALUE);
>
>    bi_call_in_edge0->probability = REG_BR_PROB_BASE * ERR_PROB;
>    bi_call_in_edge0->count =
> @@ -834,9 +833,9 @@ shrink_wrap_one_built_in_call (gcall *bi_call)
>        gcc_assert (cond_expr && gimple_code (cond_expr) == GIMPLE_COND);
>        guard_bb_in_edge = split_block (guard_bb, cond_expr);
>        guard_bb_in_edge->flags &= ~EDGE_FALLTHRU;
> -      guard_bb_in_edge->flags |= EDGE_FALSE_VALUE;
> +      guard_bb_in_edge->flags |= EDGE_TRUE_VALUE;
>
> -      bi_call_in_edge = make_edge (guard_bb, bi_call_bb, EDGE_TRUE_VALUE);
> +      bi_call_in_edge = make_edge (guard_bb, bi_call_bb, EDGE_FALSE_VALUE);
>
>        bi_call_in_edge->probability = REG_BR_PROB_BASE * ERR_PROB;
>        bi_call_in_edge->count =
>
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/torture/pr68264.c b/gcc/testsuite/gcc.dg/torture/pr68264.c
new file mode 100644
index 0000000..c4e85e7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr68264.c
@@ -0,0 +1,102 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target fenv_exceptions } */
+
+#include <fenv.h>
+#include <math.h>
+#include <errno.h>
+
+extern void abort (void) __attribute__ ((noreturn));
+
+#define LARGE_NEG_MAYBE_ERANGE 0x01
+#define LARGE_NEG_ERANGE       0x02
+#define LARGE_POS_ERANGE       0x04
+#define LARGE_NEG_EDOM         0x08
+#define LARGE_POS_EDOM         0x10
+
+#define LARGE_ERANGE (LARGE_NEG_ERANGE | LARGE_POS_ERANGE)
+#define LARGE_EDOM (LARGE_NEG_EDOM | LARGE_POS_EDOM)
+#define POWER_ERANGE (LARGE_NEG_MAYBE_ERANGE | LARGE_POS_ERANGE)
+
+#define TEST(CALL, FLAGS) (CALL, tester (FLAGS))
+
+volatile double d;
+volatile int i;
+
+static void (*tester) (int);
+
+void
+check_quiet_nan (int flags __attribute__ ((unused)))
+{
+  if (fetestexcept (FE_ALL_EXCEPT))
+    abort ();
+  if (errno)
+    abort ();
+}
+
+void
+check_large_neg (int flags)
+{
+  if (flags & LARGE_NEG_MAYBE_ERANGE)
+    return;
+  int expected_errno = (flags & LARGE_NEG_ERANGE ? ERANGE
+			: flags & LARGE_NEG_EDOM ? EDOM
+			: 0);
+  if (expected_errno != errno)
+    abort ();
+  errno = 0;
+}
+
+void
+check_large_pos (int flags)
+{
+  int expected_errno = (flags & LARGE_POS_ERANGE ? ERANGE
+			: flags & LARGE_POS_EDOM ? EDOM
+			: 0);
+  if (expected_errno != errno)
+    abort ();
+  errno = 0;
+}
+
+void
+test (void)
+{
+  TEST (acos (d), LARGE_EDOM);
+  TEST (asin (d), LARGE_EDOM);
+  TEST (acosh (d), LARGE_NEG_EDOM);
+  TEST (atanh (d), LARGE_EDOM);
+  TEST (cosh (d), LARGE_ERANGE);
+  TEST (sinh (d), LARGE_ERANGE);
+  TEST (log (d), LARGE_NEG_EDOM);
+  TEST (log2 (d), LARGE_NEG_EDOM);
+  TEST (log10 (d), LARGE_NEG_EDOM);
+  /* Disabled due to glibc PR 6792, fixed in Apr 2015.  */
+  if (0)
+    TEST (log1p (d), LARGE_NEG_EDOM);
+  TEST (exp (d), POWER_ERANGE);
+  TEST (exp2 (d), POWER_ERANGE);
+  TEST (expm1 (d), POWER_ERANGE);
+  TEST (sqrt (d), LARGE_NEG_EDOM);
+  TEST (pow (100.0, d), POWER_ERANGE);
+  TEST (pow (i, d), POWER_ERANGE);
+}
+
+int
+main (void)
+{
+  errno = 0;
+  i = 100;
+  d = __builtin_nan ("");
+  tester = check_quiet_nan;
+  feclearexcept (FE_ALL_EXCEPT);
+  test ();
+
+  d = -1.0e80;
+  tester = check_large_neg;
+  errno = 0;
+  test ();
+
+  d = 1.0e80;
+  tester = check_large_pos;
+  errno = 0;
+  test ();
+}
diff --git a/gcc/tree-call-cdce.c b/gcc/tree-call-cdce.c
index a5f38ce..fbcc70b 100644
--- a/gcc/tree-call-cdce.c
+++ b/gcc/tree-call-cdce.c
@@ -51,10 +51,11 @@  along with GCC; see the file COPYING3.  If not see
              built_in_call (args)
 
     An actual simple example is :
-         log (x);   // Mostly dead call
+	 log (x);   // Mostly dead call
      ==>
-         if (x <= 0)
-             log (x);
+	 if (__builtin_islessequal (x, 0))
+	     log (x);
+
      With this change, call to log (x) is effectively eliminated, as
      in majority of the cases, log won't be called with x out of
      range.  The branch is totally predictable, so the branch cost
@@ -306,15 +307,13 @@  is_call_dce_candidate (gcall *call)
 }
 
 
-/* A helper function to generate gimple statements for
-   one bound comparison.  ARG is the call argument to
-   be compared with the bound, LBUB is the bound value
-   in integer, TCODE is the tree_code of the comparison,
-   TEMP_NAME1/TEMP_NAME2 are names of the temporaries,
-   CONDS is a vector holding the produced GIMPLE statements,
-   and NCONDS points to the variable holding the number
-   of logical comparisons.  CONDS is either empty or
-   a list ended with a null tree.  */
+/* A helper function to generate gimple statements for one bound
+   comparison, so that the built-in function is called whenever
+   TCODE <ARG, LBUB> is *false*.  TEMP_NAME1/TEMP_NAME2 are names
+   of the temporaries, CONDS is a vector holding the produced GIMPLE
+   statements, and NCONDS points to the variable holding the number of
+   logical comparisons.  CONDS is either empty or a list ended with a
+   null tree.  */
 
 static void
 gen_one_condition (tree arg, int lbub,
@@ -371,7 +370,7 @@  gen_conditions_for_domain (tree arg, inp_domain domain,
   if (domain.has_lb)
     gen_one_condition (arg, domain.lb,
                        (domain.is_lb_inclusive
-                        ? LT_EXPR : LE_EXPR),
+                        ? UNGE_EXPR : UNGT_EXPR),
                        "DCE_COND_LB", "DCE_COND_LB_TEST",
                        conds, nconds);
 
@@ -383,7 +382,7 @@  gen_conditions_for_domain (tree arg, inp_domain domain,
 
       gen_one_condition (arg, domain.ub,
                          (domain.is_ub_inclusive
-                          ? GT_EXPR : GE_EXPR),
+                          ? UNLE_EXPR : UNLT_EXPR),
                          "DCE_COND_UB", "DCE_COND_UB_TEST",
                          conds, nconds);
     }
@@ -395,7 +394,7 @@  gen_conditions_for_domain (tree arg, inp_domain domain,
    See candidate selection in check_pow.  Since the
    candidates' base values have a limited range,
    the guarded code generated for y are simple:
-   if (y > max_y)
+   if (__builtin_isgreater (y, max_y))
      pow (const, y);
    Note max_y can be computed separately for each
    const base, but in this implementation, we
@@ -480,11 +479,11 @@  gen_conditions_for_pow_int_base (tree base, tree expn,
   /* For pow ((double)x, y), generate the following conditions:
      cond 1:
      temp1 = x;
-     if (temp1 <= 0)
+     if (__builtin_islessequal (temp1, 0))
 
      cond 2:
      temp2 = y;
-     if (temp2 > max_exp_real_cst)  */
+     if (__builtin_isgreater (temp2, max_exp_real_cst))  */
 
   /* Generate condition in reverse order -- first
      the condition for the exp argument.  */
@@ -508,7 +507,7 @@  gen_conditions_for_pow_int_base (tree base, tree expn,
   stmt1 = gimple_build_assign (temp, base_val0);
   tempn = make_ssa_name (temp, stmt1);
   gimple_assign_set_lhs (stmt1, tempn);
-  stmt2 = gimple_build_cond (LE_EXPR, tempn, cst0, NULL_TREE, NULL_TREE);
+  stmt2 = gimple_build_cond (GT_EXPR, tempn, cst0, NULL_TREE, NULL_TREE);
 
   conds.quick_push (stmt1);
   conds.quick_push (stmt2);
@@ -797,11 +796,11 @@  shrink_wrap_one_built_in_call (gcall *bi_call)
 
   bi_call_in_edge0 = split_block (bi_call_bb, cond_expr);
   bi_call_in_edge0->flags &= ~EDGE_FALLTHRU;
-  bi_call_in_edge0->flags |= EDGE_TRUE_VALUE;
+  bi_call_in_edge0->flags |= EDGE_FALSE_VALUE;
   guard_bb = bi_call_bb;
   bi_call_bb = bi_call_in_edge0->dest;
   join_tgt_in_edge_fall_thru = make_edge (guard_bb, join_tgt_bb,
-                                          EDGE_FALSE_VALUE);
+                                          EDGE_TRUE_VALUE);
 
   bi_call_in_edge0->probability = REG_BR_PROB_BASE * ERR_PROB;
   bi_call_in_edge0->count =
@@ -834,9 +833,9 @@  shrink_wrap_one_built_in_call (gcall *bi_call)
       gcc_assert (cond_expr && gimple_code (cond_expr) == GIMPLE_COND);
       guard_bb_in_edge = split_block (guard_bb, cond_expr);
       guard_bb_in_edge->flags &= ~EDGE_FALLTHRU;
-      guard_bb_in_edge->flags |= EDGE_FALSE_VALUE;
+      guard_bb_in_edge->flags |= EDGE_TRUE_VALUE;
 
-      bi_call_in_edge = make_edge (guard_bb, bi_call_bb, EDGE_TRUE_VALUE);
+      bi_call_in_edge = make_edge (guard_bb, bi_call_bb, EDGE_FALSE_VALUE);
 
       bi_call_in_edge->probability = REG_BR_PROB_BASE * ERR_PROB;
       bi_call_in_edge->count =