Re: [PATCH] Fix PR tree-optimization/71077
On Mon, May 30, 2016 at 9:50 PM, Patrick Palkawrote: > In this PR the function simplify_control_stmt_condition_1(), which is > responsible for recursing into the operands of a GIMPLE_COND during jump > threading to check for dominating ASSERT_EXPRs, was erroneously > returning a VECTOR_CST when it should instead be returning an > INTEGER_CST. The problem is that the zero/one constants that this > function uses share the type of the GIMPLE_COND's operands, operands > which may be vectors. This makes no sense and there is no reason to > build and use such constants instead of using boolean_[true|false]_node > since all that matters is whether a constant returned by > simplify_control_stmt_condition() correctly satisfies > integer_[zero|one]p. So this patch makes the combining part of > simplify_control_stmt_condition_1() just use boolean_false_node or > boolean_true_node as the designated false/true values. > > Does this look OK to commit after bootstrap+regtest on > x86_64-pc-linux-gnu? (I'm not sure if I put the test in the proper > directory since no other test in tree-ssa uses -flto. Also not sure if > an i686 target accepts the -march=core-avx2 flag.) Yes. Ok. Thanks, Richard. > gcc/ChangeLog: > > PR tree-optimization/71077 > * tree-ssa-threadedge.c (simplify_control_stmt_condition_1): In > the combining step, use boolean_false_node and boolean_true_node > as the designated false/true return values. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/71077 > * gcc.dg/tree-ssa/pr71077.c: New test. > --- > gcc/testsuite/gcc.dg/tree-ssa/pr71077.c | 18 ++ > gcc/tree-ssa-threadedge.c | 26 -- > 2 files changed, 30 insertions(+), 14 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71077.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c > new file mode 100644 > index 000..4753740 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c > @@ -0,0 +1,18 @@ > +/* PR c++/71077 */ > +/* { dg-do link { target { i?86-*-* x86_64-*-* } } } */ > +/* { dg-options "-O3 -flto -march=core-avx2" } */ > + > +int *a; > +int b, c, d, e; > +int sched_analyze(void) { > + for (; b; b++) { > + c = 0; > + for (; c < 32; c++) > + if (b & 1 << c) > + a[b + c] = d; > + } > + return 0; > +} > + > +void schedule_insns(void) { e = sched_analyze(); } > +int main(void) { schedule_insns(); } > diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c > index eca3812..826d620 100644 > --- a/gcc/tree-ssa-threadedge.c > +++ b/gcc/tree-ssa-threadedge.c > @@ -573,8 +573,6 @@ simplify_control_stmt_condition_1 (edge e, > enum tree_code rhs_code = gimple_assign_rhs_code (def_stmt); > const tree rhs1 = gimple_assign_rhs1 (def_stmt); > const tree rhs2 = gimple_assign_rhs2 (def_stmt); > - const tree zero_cst = build_zero_cst (TREE_TYPE (op0)); > - const tree one_cst = build_one_cst (TREE_TYPE (op0)); > > /* Is A != 0 ? */ > const tree res1 > @@ -589,19 +587,19 @@ simplify_control_stmt_condition_1 (edge e, > { > /* If A == 0 then (A & B) != 0 is always false. */ > if (cond_code == NE_EXPR) > - return zero_cst; > + return boolean_false_node; > /* If A == 0 then (A & B) == 0 is always true. */ > if (cond_code == EQ_EXPR) > - return one_cst; > + return boolean_true_node; > } > else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res1)) > { > /* If A != 0 then (A | B) != 0 is always true. */ > if (cond_code == NE_EXPR) > - return one_cst; > + return boolean_true_node; > /* If A != 0 then (A | B) == 0 is always false. */ > if (cond_code == EQ_EXPR) > - return zero_cst; > + return boolean_false_node; > } > > /* Is B != 0 ? */ > @@ -617,19 +615,19 @@ simplify_control_stmt_condition_1 (edge e, > { > /* If B == 0 then (A & B) != 0 is always false. */ > if (cond_code == NE_EXPR) > - return zero_cst; > + return boolean_false_node; > /* If B == 0 then (A & B) == 0 is always true. */ > if (cond_code == EQ_EXPR) > - return one_cst; > + return boolean_true_node; > } > else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res2)) > { > /* If B != 0 then (A | B) != 0 is always true. */ > if (cond_code == NE_EXPR) > - return one_cst; > + return boolean_true_node; > /* If B != 0 then (A | B) == 0 is always false. */ > if
[PATCH] Fix PR tree-optimization/71077
In this PR the function simplify_control_stmt_condition_1(), which is responsible for recursing into the operands of a GIMPLE_COND during jump threading to check for dominating ASSERT_EXPRs, was erroneously returning a VECTOR_CST when it should instead be returning an INTEGER_CST. The problem is that the zero/one constants that this function uses share the type of the GIMPLE_COND's operands, operands which may be vectors. This makes no sense and there is no reason to build and use such constants instead of using boolean_[true|false]_node since all that matters is whether a constant returned by simplify_control_stmt_condition() correctly satisfies integer_[zero|one]p. So this patch makes the combining part of simplify_control_stmt_condition_1() just use boolean_false_node or boolean_true_node as the designated false/true values. Does this look OK to commit after bootstrap+regtest on x86_64-pc-linux-gnu? (I'm not sure if I put the test in the proper directory since no other test in tree-ssa uses -flto. Also not sure if an i686 target accepts the -march=core-avx2 flag.) gcc/ChangeLog: PR tree-optimization/71077 * tree-ssa-threadedge.c (simplify_control_stmt_condition_1): In the combining step, use boolean_false_node and boolean_true_node as the designated false/true return values. gcc/testsuite/ChangeLog: PR tree-optimization/71077 * gcc.dg/tree-ssa/pr71077.c: New test. --- gcc/testsuite/gcc.dg/tree-ssa/pr71077.c | 18 ++ gcc/tree-ssa-threadedge.c | 26 -- 2 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr71077.c diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c new file mode 100644 index 000..4753740 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71077.c @@ -0,0 +1,18 @@ +/* PR c++/71077 */ +/* { dg-do link { target { i?86-*-* x86_64-*-* } } } */ +/* { dg-options "-O3 -flto -march=core-avx2" } */ + +int *a; +int b, c, d, e; +int sched_analyze(void) { + for (; b; b++) { + c = 0; + for (; c < 32; c++) + if (b & 1 << c) + a[b + c] = d; + } + return 0; +} + +void schedule_insns(void) { e = sched_analyze(); } +int main(void) { schedule_insns(); } diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c index eca3812..826d620 100644 --- a/gcc/tree-ssa-threadedge.c +++ b/gcc/tree-ssa-threadedge.c @@ -573,8 +573,6 @@ simplify_control_stmt_condition_1 (edge e, enum tree_code rhs_code = gimple_assign_rhs_code (def_stmt); const tree rhs1 = gimple_assign_rhs1 (def_stmt); const tree rhs2 = gimple_assign_rhs2 (def_stmt); - const tree zero_cst = build_zero_cst (TREE_TYPE (op0)); - const tree one_cst = build_one_cst (TREE_TYPE (op0)); /* Is A != 0 ? */ const tree res1 @@ -589,19 +587,19 @@ simplify_control_stmt_condition_1 (edge e, { /* If A == 0 then (A & B) != 0 is always false. */ if (cond_code == NE_EXPR) - return zero_cst; + return boolean_false_node; /* If A == 0 then (A & B) == 0 is always true. */ if (cond_code == EQ_EXPR) - return one_cst; + return boolean_true_node; } else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res1)) { /* If A != 0 then (A | B) != 0 is always true. */ if (cond_code == NE_EXPR) - return one_cst; + return boolean_true_node; /* If A != 0 then (A | B) == 0 is always false. */ if (cond_code == EQ_EXPR) - return zero_cst; + return boolean_false_node; } /* Is B != 0 ? */ @@ -617,19 +615,19 @@ simplify_control_stmt_condition_1 (edge e, { /* If B == 0 then (A & B) != 0 is always false. */ if (cond_code == NE_EXPR) - return zero_cst; + return boolean_false_node; /* If B == 0 then (A & B) == 0 is always true. */ if (cond_code == EQ_EXPR) - return one_cst; + return boolean_true_node; } else if (rhs_code == BIT_IOR_EXPR && integer_nonzerop (res2)) { /* If B != 0 then (A | B) != 0 is always true. */ if (cond_code == NE_EXPR) - return one_cst; + return boolean_true_node; /* If B != 0 then (A | B) == 0 is always false. */ if (cond_code == EQ_EXPR) - return zero_cst; + return boolean_false_node; } if (res1 != NULL_TREE && res2 != NULL_TREE) @@ -641,10 +639,10 @@ simplify_control_stmt_condition_1 (edge e, { /* If A != 0 and B != 0 then (bool)(A & B) != 0 is true. */