Message ID | 87y4e2kwcp.fsf@e105548-lin.cambridge.arm.com |
---|---|
State | New |
Headers | show |
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 --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 =