| Message ID | 1278534137-22733-3-git-send-email-sebpop@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 7 Jul 2010, Sebastian Pop wrote: > PR tree-optimization/44710 > * tree-if-conv.c (parse_predicate): New. > (add_to_predicate_list): Call it, call maybe_fold_or_comparisons. > Make sure that the predicates are either SSA_NAMEs or gimple_condexpr. > > * gcc.dg/tree-ssa/ifc-6.c: New. > --- > gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c | 15 +++++ > gcc/tree-if-conv.c | 92 ++++++++++++++++++++++++++++++--- > 2 files changed, 100 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c > new file mode 100644 > index 0000000..a9c5db3 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-options "-c -O2 -ftree-vectorize" { target *-*-* } } */ > + > +static int x; > +foo (int n, int *A) > +{ > + int i; > + for (i = 0; i < n; i++) > + { > + if (A[i]) > + x = 2; > + if (A[i + 1]) > + x = 1; > + } > +} > diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c > index ad106d7..03e453e 100644 > --- a/gcc/tree-if-conv.c > +++ b/gcc/tree-if-conv.c > @@ -259,17 +259,95 @@ is_predicated (basic_block bb) > return !is_true_predicate (bb_predicate (bb)); > } > > -/* Add condition NEW_COND to the predicate list of basic block BB. */ > +/* Parses the predicate COND and returns its comparison code and > + operands OP0 and OP1. */ > + > +static enum tree_code > +parse_predicate (tree cond, tree *op0, tree *op1) > +{ > + gimple s; > + > + if (TREE_CODE (cond) == SSA_NAME > + && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond))) > + { > + if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison) > + { > + *op0 = gimple_assign_rhs1 (s); > + *op1 = gimple_assign_rhs2 (s); > + return gimple_assign_rhs_code (s); > + } > + > + else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR) > + { > + tree op = gimple_assign_rhs1 (s); > + tree type = TREE_TYPE (op); > + enum tree_code code = parse_predicate (op, op0, op1); > + > + return code == ERROR_MARK ? ERROR_MARK : > + invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type))); The : goes on the next line. > + } > + > + return ERROR_MARK; > + } > + > + if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) > + { > + *op0 = TREE_OPERAND (cond, 0); > + *op1 = TREE_OPERAND (cond, 1); > + return TREE_CODE (cond); > + } > + > + return ERROR_MARK; > +} > + > +/* Add condition NC to the predicate list of basic block BB. */ > > static inline void > -add_to_predicate_list (basic_block bb, tree new_cond) > +add_to_predicate_list (basic_block bb, tree nc) > { > - tree cond = bb_predicate (bb); > + tree bc; > > - set_bb_predicate (bb, is_true_predicate (cond) ? new_cond : > - fold_build2_loc (EXPR_LOCATION (cond), > - TRUTH_OR_EXPR, boolean_type_node, > - cond, new_cond)); > + if (is_true_predicate (nc)) > + return; > + > + if (!is_predicated (bb)) > + bc = nc; > + else > + { > + enum tree_code code1, code2; > + tree op1a, op1b, op2a, op2b; > + > + bc = bb_predicate (bb); > + code1 = parse_predicate (bc, &op1a, &op1b); > + code2 = parse_predicate (nc, &op2a, &op2b); > + > + if (code1 != ERROR_MARK && code2 != ERROR_MARK) > + { > + bc = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); > + > + if (!bc) > + { > + bc = bb_predicate (bb); > + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, > + boolean_type_node, bc, nc); > + } > + } > + else > + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, > + boolean_type_node, bc, nc); your re-use of bc makes this overly complicated. Please use a new tem for the maybe_fold_or_comparisons result and commonize the !temp fold_build2_loc paths (and avoid re-loading bb_predicate) > + } > + > + if (!is_gimple_condexpr (bc)) > + { > + gimple_seq stmts; > + bc = force_gimple_operand (bc, &stmts, true, NULL_TREE); > + add_bb_predicate_gimplified_stmts (bb, stmts); > + } > + > + if (is_true_predicate (bc)) > + reset_bb_predicate (bb); > + else > + set_bb_predicate (bb, bc); > } > > /* Add the condition COND to the previous condition PREV_COND, and add > Ok with that change. Thanks, Richard.
On Thu, Jul 8, 2010 at 04:57, Richard Guenther <rguenther@suse.de> wrote: > On Wed, 7 Jul 2010, Sebastian Pop wrote: > >> PR tree-optimization/44710 >> * tree-if-conv.c (parse_predicate): New. >> (add_to_predicate_list): Call it, call maybe_fold_or_comparisons. >> Make sure that the predicates are either SSA_NAMEs or gimple_condexpr. >> >> * gcc.dg/tree-ssa/ifc-6.c: New. >> --- >> gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c | 15 +++++ >> gcc/tree-if-conv.c | 92 ++++++++++++++++++++++++++++++--- >> 2 files changed, 100 insertions(+), 7 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >> >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >> new file mode 100644 >> index 0000000..a9c5db3 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >> @@ -0,0 +1,15 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-c -O2 -ftree-vectorize" { target *-*-* } } */ >> + >> +static int x; >> +foo (int n, int *A) >> +{ >> + int i; >> + for (i = 0; i < n; i++) >> + { >> + if (A[i]) >> + x = 2; >> + if (A[i + 1]) >> + x = 1; >> + } >> +} >> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c >> index ad106d7..03e453e 100644 >> --- a/gcc/tree-if-conv.c >> +++ b/gcc/tree-if-conv.c >> @@ -259,17 +259,95 @@ is_predicated (basic_block bb) >> return !is_true_predicate (bb_predicate (bb)); >> } >> >> -/* Add condition NEW_COND to the predicate list of basic block BB. */ >> +/* Parses the predicate COND and returns its comparison code and >> + operands OP0 and OP1. */ >> + >> +static enum tree_code >> +parse_predicate (tree cond, tree *op0, tree *op1) >> +{ >> + gimple s; >> + >> + if (TREE_CODE (cond) == SSA_NAME >> + && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond))) >> + { >> + if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison) >> + { >> + *op0 = gimple_assign_rhs1 (s); >> + *op1 = gimple_assign_rhs2 (s); >> + return gimple_assign_rhs_code (s); >> + } >> + >> + else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR) >> + { >> + tree op = gimple_assign_rhs1 (s); >> + tree type = TREE_TYPE (op); >> + enum tree_code code = parse_predicate (op, op0, op1); >> + >> + return code == ERROR_MARK ? ERROR_MARK : >> + invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type))); > > The : goes on the next line. > >> + } >> + >> + return ERROR_MARK; >> + } >> + >> + if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) >> + { >> + *op0 = TREE_OPERAND (cond, 0); >> + *op1 = TREE_OPERAND (cond, 1); >> + return TREE_CODE (cond); >> + } >> + >> + return ERROR_MARK; >> +} >> + >> +/* Add condition NC to the predicate list of basic block BB. */ >> >> static inline void >> -add_to_predicate_list (basic_block bb, tree new_cond) >> +add_to_predicate_list (basic_block bb, tree nc) >> { >> - tree cond = bb_predicate (bb); >> + tree bc; >> >> - set_bb_predicate (bb, is_true_predicate (cond) ? new_cond : >> - fold_build2_loc (EXPR_LOCATION (cond), >> - TRUTH_OR_EXPR, boolean_type_node, >> - cond, new_cond)); >> + if (is_true_predicate (nc)) >> + return; >> + >> + if (!is_predicated (bb)) >> + bc = nc; >> + else >> + { >> + enum tree_code code1, code2; >> + tree op1a, op1b, op2a, op2b; >> + >> + bc = bb_predicate (bb); >> + code1 = parse_predicate (bc, &op1a, &op1b); >> + code2 = parse_predicate (nc, &op2a, &op2b); >> + >> + if (code1 != ERROR_MARK && code2 != ERROR_MARK) >> + { >> + bc = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); >> + >> + if (!bc) >> + { >> + bc = bb_predicate (bb); >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, >> + boolean_type_node, bc, nc); >> + } >> + } >> + else >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, >> + boolean_type_node, bc, nc); > > your re-use of bc makes this overly complicated. Please use a new > tem for the maybe_fold_or_comparisons result and commonize the > !temp fold_build2_loc paths (and avoid re-loading bb_predicate) > >> + } >> + >> + if (!is_gimple_condexpr (bc)) >> + { >> + gimple_seq stmts; >> + bc = force_gimple_operand (bc, &stmts, true, NULL_TREE); >> + add_bb_predicate_gimplified_stmts (bb, stmts); >> + } >> + >> + if (is_true_predicate (bc)) >> + reset_bb_predicate (bb); >> + else >> + set_bb_predicate (bb, bc); >> } >> >> /* Add the condition COND to the previous condition PREV_COND, and add >> > > Ok with that change. > Committed r161964.
On Thu, 8 Jul 2010, Sebastian Pop wrote: > On Thu, Jul 8, 2010 at 04:57, Richard Guenther <rguenther@suse.de> wrote: > > On Wed, 7 Jul 2010, Sebastian Pop wrote: > > > >> PR tree-optimization/44710 > >> * tree-if-conv.c (parse_predicate): New. > >> (add_to_predicate_list): Call it, call maybe_fold_or_comparisons. > >> Make sure that the predicates are either SSA_NAMEs or gimple_condexpr. > >> > >> * gcc.dg/tree-ssa/ifc-6.c: New. > >> --- > >> gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c | 15 +++++ > >> gcc/tree-if-conv.c | 92 ++++++++++++++++++++++++++++++--- > >> 2 files changed, 100 insertions(+), 7 deletions(-) > >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c > >> > >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c > >> new file mode 100644 > >> index 0000000..a9c5db3 > >> --- /dev/null > >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c > >> @@ -0,0 +1,15 @@ > >> +/* { dg-do compile } */ > >> +/* { dg-options "-c -O2 -ftree-vectorize" { target *-*-* } } */ > >> + > >> +static int x; > >> +foo (int n, int *A) > >> +{ > >> + int i; > >> + for (i = 0; i < n; i++) > >> + { > >> + if (A[i]) > >> + x = 2; > >> + if (A[i + 1]) > >> + x = 1; > >> + } > >> +} > >> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c > >> index ad106d7..03e453e 100644 > >> --- a/gcc/tree-if-conv.c > >> +++ b/gcc/tree-if-conv.c > >> @@ -259,17 +259,95 @@ is_predicated (basic_block bb) > >> return !is_true_predicate (bb_predicate (bb)); > >> } > >> > >> -/* Add condition NEW_COND to the predicate list of basic block BB. */ > >> +/* Parses the predicate COND and returns its comparison code and > >> + operands OP0 and OP1. */ > >> + > >> +static enum tree_code > >> +parse_predicate (tree cond, tree *op0, tree *op1) > >> +{ > >> + gimple s; > >> + > >> + if (TREE_CODE (cond) == SSA_NAME > >> + && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond))) > >> + { > >> + if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison) > >> + { > >> + *op0 = gimple_assign_rhs1 (s); > >> + *op1 = gimple_assign_rhs2 (s); > >> + return gimple_assign_rhs_code (s); > >> + } > >> + > >> + else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR) > >> + { > >> + tree op = gimple_assign_rhs1 (s); > >> + tree type = TREE_TYPE (op); > >> + enum tree_code code = parse_predicate (op, op0, op1); > >> + > >> + return code == ERROR_MARK ? ERROR_MARK : > >> + invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type))); > > > > The : goes on the next line. > > > >> + } > >> + > >> + return ERROR_MARK; > >> + } > >> + > >> + if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) > >> + { > >> + *op0 = TREE_OPERAND (cond, 0); > >> + *op1 = TREE_OPERAND (cond, 1); > >> + return TREE_CODE (cond); > >> + } > >> + > >> + return ERROR_MARK; > >> +} > >> + > >> +/* Add condition NC to the predicate list of basic block BB. */ > >> > >> static inline void > >> -add_to_predicate_list (basic_block bb, tree new_cond) > >> +add_to_predicate_list (basic_block bb, tree nc) > >> { > >> - tree cond = bb_predicate (bb); > >> + tree bc; > >> > >> - set_bb_predicate (bb, is_true_predicate (cond) ? new_cond : > >> - fold_build2_loc (EXPR_LOCATION (cond), > >> - TRUTH_OR_EXPR, boolean_type_node, > >> - cond, new_cond)); > >> + if (is_true_predicate (nc)) > >> + return; > >> + > >> + if (!is_predicated (bb)) > >> + bc = nc; > >> + else > >> + { > >> + enum tree_code code1, code2; > >> + tree op1a, op1b, op2a, op2b; > >> + > >> + bc = bb_predicate (bb); > >> + code1 = parse_predicate (bc, &op1a, &op1b); > >> + code2 = parse_predicate (nc, &op2a, &op2b); > >> + > >> + if (code1 != ERROR_MARK && code2 != ERROR_MARK) > >> + { > >> + bc = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); > >> + > >> + if (!bc) > >> + { > >> + bc = bb_predicate (bb); > >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, > >> + boolean_type_node, bc, nc); > >> + } > >> + } > >> + else > >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, > >> + boolean_type_node, bc, nc); > > > > your re-use of bc makes this overly complicated. Please use a new > > tem for the maybe_fold_or_comparisons result and commonize the > > !temp fold_build2_loc paths (and avoid re-loading bb_predicate) > > > >> + } > >> + > >> + if (!is_gimple_condexpr (bc)) > >> + { > >> + gimple_seq stmts; > >> + bc = force_gimple_operand (bc, &stmts, true, NULL_TREE); > >> + add_bb_predicate_gimplified_stmts (bb, stmts); > >> + } > >> + > >> + if (is_true_predicate (bc)) > >> + reset_bb_predicate (bb); > >> + else > >> + set_bb_predicate (bb, bc); > >> } > >> > >> /* Add the condition COND to the previous condition PREV_COND, and add > >> > > > > Ok with that change. > > > > Committed r161964. Why did you commit without the requested change? Richard.
On Fri, Jul 9, 2010 at 07:10, Richard Guenther <rguenther@suse.de> wrote: > On Thu, 8 Jul 2010, Sebastian Pop wrote: > >> On Thu, Jul 8, 2010 at 04:57, Richard Guenther <rguenther@suse.de> wrote: >> > On Wed, 7 Jul 2010, Sebastian Pop wrote: >> > >> >> PR tree-optimization/44710 >> >> * tree-if-conv.c (parse_predicate): New. >> >> (add_to_predicate_list): Call it, call maybe_fold_or_comparisons. >> >> Make sure that the predicates are either SSA_NAMEs or gimple_condexpr. >> >> >> >> * gcc.dg/tree-ssa/ifc-6.c: New. >> >> --- >> >> gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c | 15 +++++ >> >> gcc/tree-if-conv.c | 92 ++++++++++++++++++++++++++++++--- >> >> 2 files changed, 100 insertions(+), 7 deletions(-) >> >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >> >> >> >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >> >> new file mode 100644 >> >> index 0000000..a9c5db3 >> >> --- /dev/null >> >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >> >> @@ -0,0 +1,15 @@ >> >> +/* { dg-do compile } */ >> >> +/* { dg-options "-c -O2 -ftree-vectorize" { target *-*-* } } */ >> >> + >> >> +static int x; >> >> +foo (int n, int *A) >> >> +{ >> >> + int i; >> >> + for (i = 0; i < n; i++) >> >> + { >> >> + if (A[i]) >> >> + x = 2; >> >> + if (A[i + 1]) >> >> + x = 1; >> >> + } >> >> +} >> >> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c >> >> index ad106d7..03e453e 100644 >> >> --- a/gcc/tree-if-conv.c >> >> +++ b/gcc/tree-if-conv.c >> >> @@ -259,17 +259,95 @@ is_predicated (basic_block bb) >> >> return !is_true_predicate (bb_predicate (bb)); >> >> } >> >> >> >> -/* Add condition NEW_COND to the predicate list of basic block BB. */ >> >> +/* Parses the predicate COND and returns its comparison code and >> >> + operands OP0 and OP1. */ >> >> + >> >> +static enum tree_code >> >> +parse_predicate (tree cond, tree *op0, tree *op1) >> >> +{ >> >> + gimple s; >> >> + >> >> + if (TREE_CODE (cond) == SSA_NAME >> >> + && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond))) >> >> + { >> >> + if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison) >> >> + { >> >> + *op0 = gimple_assign_rhs1 (s); >> >> + *op1 = gimple_assign_rhs2 (s); >> >> + return gimple_assign_rhs_code (s); >> >> + } >> >> + >> >> + else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR) >> >> + { >> >> + tree op = gimple_assign_rhs1 (s); >> >> + tree type = TREE_TYPE (op); >> >> + enum tree_code code = parse_predicate (op, op0, op1); >> >> + >> >> + return code == ERROR_MARK ? ERROR_MARK : >> >> + invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type))); >> > >> > The : goes on the next line. >> > >> >> + } >> >> + >> >> + return ERROR_MARK; >> >> + } >> >> + >> >> + if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) >> >> + { >> >> + *op0 = TREE_OPERAND (cond, 0); >> >> + *op1 = TREE_OPERAND (cond, 1); >> >> + return TREE_CODE (cond); >> >> + } >> >> + >> >> + return ERROR_MARK; >> >> +} >> >> + >> >> +/* Add condition NC to the predicate list of basic block BB. */ >> >> >> >> static inline void >> >> -add_to_predicate_list (basic_block bb, tree new_cond) >> >> +add_to_predicate_list (basic_block bb, tree nc) >> >> { >> >> - tree cond = bb_predicate (bb); >> >> + tree bc; >> >> >> >> - set_bb_predicate (bb, is_true_predicate (cond) ? new_cond : >> >> - fold_build2_loc (EXPR_LOCATION (cond), >> >> - TRUTH_OR_EXPR, boolean_type_node, >> >> - cond, new_cond)); >> >> + if (is_true_predicate (nc)) >> >> + return; >> >> + >> >> + if (!is_predicated (bb)) >> >> + bc = nc; >> >> + else >> >> + { >> >> + enum tree_code code1, code2; >> >> + tree op1a, op1b, op2a, op2b; >> >> + >> >> + bc = bb_predicate (bb); >> >> + code1 = parse_predicate (bc, &op1a, &op1b); >> >> + code2 = parse_predicate (nc, &op2a, &op2b); >> >> + >> >> + if (code1 != ERROR_MARK && code2 != ERROR_MARK) >> >> + { >> >> + bc = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); >> >> + >> >> + if (!bc) >> >> + { >> >> + bc = bb_predicate (bb); >> >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, >> >> + boolean_type_node, bc, nc); >> >> + } >> >> + } >> >> + else >> >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, >> >> + boolean_type_node, bc, nc); >> > >> > your re-use of bc makes this overly complicated. Please use a new >> > tem for the maybe_fold_or_comparisons result and commonize the >> > !temp fold_build2_loc paths (and avoid re-loading bb_predicate) >> > >> >> + } >> >> + >> >> + if (!is_gimple_condexpr (bc)) >> >> + { >> >> + gimple_seq stmts; >> >> + bc = force_gimple_operand (bc, &stmts, true, NULL_TREE); >> >> + add_bb_predicate_gimplified_stmts (bb, stmts); >> >> + } >> >> + >> >> + if (is_true_predicate (bc)) >> >> + reset_bb_predicate (bb); >> >> + else >> >> + set_bb_predicate (bb, bc); >> >> } >> >> >> >> /* Add the condition COND to the previous condition PREV_COND, and add >> >> >> > >> > Ok with that change. >> > >> >> Committed r161964. > > Why did you commit without the requested change? The two changes that you requested are implemented in the patch that I committed. I addressed this in the exact way you described: using a temp. + if (code1 != ERROR_MARK && code2 != ERROR_MARK) + { + tree t = maybe_fold_or_comparisons (code1, op1a, op1b, + code2, op2a, op2b); + if (!t) + t = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, + boolean_type_node, bc, nc); + bc = t; + } Sebastian
On Fri, Jul 9, 2010 at 6:40 PM, Sebastian Pop <sebpop@gmail.com> wrote: > On Fri, Jul 9, 2010 at 07:10, Richard Guenther <rguenther@suse.de> wrote: >> On Thu, 8 Jul 2010, Sebastian Pop wrote: >> >>> On Thu, Jul 8, 2010 at 04:57, Richard Guenther <rguenther@suse.de> wrote: >>> > On Wed, 7 Jul 2010, Sebastian Pop wrote: >>> > >>> >> PR tree-optimization/44710 >>> >> * tree-if-conv.c (parse_predicate): New. >>> >> (add_to_predicate_list): Call it, call maybe_fold_or_comparisons. >>> >> Make sure that the predicates are either SSA_NAMEs or gimple_condexpr. >>> >> >>> >> * gcc.dg/tree-ssa/ifc-6.c: New. >>> >> --- >>> >> gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c | 15 +++++ >>> >> gcc/tree-if-conv.c | 92 ++++++++++++++++++++++++++++++--- >>> >> 2 files changed, 100 insertions(+), 7 deletions(-) >>> >> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >>> >> >>> >> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >>> >> new file mode 100644 >>> >> index 0000000..a9c5db3 >>> >> --- /dev/null >>> >> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c >>> >> @@ -0,0 +1,15 @@ >>> >> +/* { dg-do compile } */ >>> >> +/* { dg-options "-c -O2 -ftree-vectorize" { target *-*-* } } */ >>> >> + >>> >> +static int x; >>> >> +foo (int n, int *A) >>> >> +{ >>> >> + int i; >>> >> + for (i = 0; i < n; i++) >>> >> + { >>> >> + if (A[i]) >>> >> + x = 2; >>> >> + if (A[i + 1]) >>> >> + x = 1; >>> >> + } >>> >> +} >>> >> diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c >>> >> index ad106d7..03e453e 100644 >>> >> --- a/gcc/tree-if-conv.c >>> >> +++ b/gcc/tree-if-conv.c >>> >> @@ -259,17 +259,95 @@ is_predicated (basic_block bb) >>> >> return !is_true_predicate (bb_predicate (bb)); >>> >> } >>> >> >>> >> -/* Add condition NEW_COND to the predicate list of basic block BB. */ >>> >> +/* Parses the predicate COND and returns its comparison code and >>> >> + operands OP0 and OP1. */ >>> >> + >>> >> +static enum tree_code >>> >> +parse_predicate (tree cond, tree *op0, tree *op1) >>> >> +{ >>> >> + gimple s; >>> >> + >>> >> + if (TREE_CODE (cond) == SSA_NAME >>> >> + && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond))) >>> >> + { >>> >> + if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison) >>> >> + { >>> >> + *op0 = gimple_assign_rhs1 (s); >>> >> + *op1 = gimple_assign_rhs2 (s); >>> >> + return gimple_assign_rhs_code (s); >>> >> + } >>> >> + >>> >> + else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR) >>> >> + { >>> >> + tree op = gimple_assign_rhs1 (s); >>> >> + tree type = TREE_TYPE (op); >>> >> + enum tree_code code = parse_predicate (op, op0, op1); >>> >> + >>> >> + return code == ERROR_MARK ? ERROR_MARK : >>> >> + invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type))); >>> > >>> > The : goes on the next line. >>> > >>> >> + } >>> >> + >>> >> + return ERROR_MARK; >>> >> + } >>> >> + >>> >> + if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) >>> >> + { >>> >> + *op0 = TREE_OPERAND (cond, 0); >>> >> + *op1 = TREE_OPERAND (cond, 1); >>> >> + return TREE_CODE (cond); >>> >> + } >>> >> + >>> >> + return ERROR_MARK; >>> >> +} >>> >> + >>> >> +/* Add condition NC to the predicate list of basic block BB. */ >>> >> >>> >> static inline void >>> >> -add_to_predicate_list (basic_block bb, tree new_cond) >>> >> +add_to_predicate_list (basic_block bb, tree nc) >>> >> { >>> >> - tree cond = bb_predicate (bb); >>> >> + tree bc; >>> >> >>> >> - set_bb_predicate (bb, is_true_predicate (cond) ? new_cond : >>> >> - fold_build2_loc (EXPR_LOCATION (cond), >>> >> - TRUTH_OR_EXPR, boolean_type_node, >>> >> - cond, new_cond)); >>> >> + if (is_true_predicate (nc)) >>> >> + return; >>> >> + >>> >> + if (!is_predicated (bb)) >>> >> + bc = nc; >>> >> + else >>> >> + { >>> >> + enum tree_code code1, code2; >>> >> + tree op1a, op1b, op2a, op2b; >>> >> + >>> >> + bc = bb_predicate (bb); >>> >> + code1 = parse_predicate (bc, &op1a, &op1b); >>> >> + code2 = parse_predicate (nc, &op2a, &op2b); >>> >> + >>> >> + if (code1 != ERROR_MARK && code2 != ERROR_MARK) >>> >> + { >>> >> + bc = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); >>> >> + >>> >> + if (!bc) >>> >> + { >>> >> + bc = bb_predicate (bb); >>> >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, >>> >> + boolean_type_node, bc, nc); >>> >> + } >>> >> + } >>> >> + else >>> >> + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, >>> >> + boolean_type_node, bc, nc); >>> > >>> > your re-use of bc makes this overly complicated. Please use a new >>> > tem for the maybe_fold_or_comparisons result and commonize the >>> > !temp fold_build2_loc paths (and avoid re-loading bb_predicate) >>> > >>> >> + } >>> >> + >>> >> + if (!is_gimple_condexpr (bc)) >>> >> + { >>> >> + gimple_seq stmts; >>> >> + bc = force_gimple_operand (bc, &stmts, true, NULL_TREE); >>> >> + add_bb_predicate_gimplified_stmts (bb, stmts); >>> >> + } >>> >> + >>> >> + if (is_true_predicate (bc)) >>> >> + reset_bb_predicate (bb); >>> >> + else >>> >> + set_bb_predicate (bb, bc); >>> >> } >>> >> >>> >> /* Add the condition COND to the previous condition PREV_COND, and add >>> >> >>> > >>> > Ok with that change. >>> > >>> >>> Committed r161964. >> >> Why did you commit without the requested change? > > The two changes that you requested are implemented in the > patch that I committed. I addressed this in the exact way > you described: using a temp. > > + if (code1 != ERROR_MARK && code2 != ERROR_MARK) > + { > + tree t = maybe_fold_or_comparisons (code1, op1a, op1b, > + code2, op2a, op2b); > + if (!t) > + t = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, > + boolean_type_node, bc, nc); > + bc = t; > + } I see in my tree enum tree_code code1, code2; tree op1a, op1b, op2a, op2b; bc = bb_predicate (bb); code1 = parse_predicate (bc, &op1a, &op1b); code2 = parse_predicate (nc, &op2a, &op2b); if (code1 != ERROR_MARK && code2 != ERROR_MARK) { tree t = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); if (!t) t = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, boolean_type_node, bc, nc); bc = t; } else bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, boolean_type_node, bc, nc); which is indeed different from the version you proposed initially (sorry for not noticing), but there appearantly was a misunderstanding as that code still calls fold_build2_loc twice with the same args. Sth you fixed with the followup patch. Sorry for the noise, Richard.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c new file mode 100644 index 0000000..a9c5db3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ifc-6.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-c -O2 -ftree-vectorize" { target *-*-* } } */ + +static int x; +foo (int n, int *A) +{ + int i; + for (i = 0; i < n; i++) + { + if (A[i]) + x = 2; + if (A[i + 1]) + x = 1; + } +} diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c index ad106d7..03e453e 100644 --- a/gcc/tree-if-conv.c +++ b/gcc/tree-if-conv.c @@ -259,17 +259,95 @@ is_predicated (basic_block bb) return !is_true_predicate (bb_predicate (bb)); } -/* Add condition NEW_COND to the predicate list of basic block BB. */ +/* Parses the predicate COND and returns its comparison code and + operands OP0 and OP1. */ + +static enum tree_code +parse_predicate (tree cond, tree *op0, tree *op1) +{ + gimple s; + + if (TREE_CODE (cond) == SSA_NAME + && is_gimple_assign (s = SSA_NAME_DEF_STMT (cond))) + { + if (TREE_CODE_CLASS (gimple_assign_rhs_code (s)) == tcc_comparison) + { + *op0 = gimple_assign_rhs1 (s); + *op1 = gimple_assign_rhs2 (s); + return gimple_assign_rhs_code (s); + } + + else if (gimple_assign_rhs_code (s) == TRUTH_NOT_EXPR) + { + tree op = gimple_assign_rhs1 (s); + tree type = TREE_TYPE (op); + enum tree_code code = parse_predicate (op, op0, op1); + + return code == ERROR_MARK ? ERROR_MARK : + invert_tree_comparison (code, HONOR_NANS (TYPE_MODE (type))); + } + + return ERROR_MARK; + } + + if (TREE_CODE_CLASS (TREE_CODE (cond)) == tcc_comparison) + { + *op0 = TREE_OPERAND (cond, 0); + *op1 = TREE_OPERAND (cond, 1); + return TREE_CODE (cond); + } + + return ERROR_MARK; +} + +/* Add condition NC to the predicate list of basic block BB. */ static inline void -add_to_predicate_list (basic_block bb, tree new_cond) +add_to_predicate_list (basic_block bb, tree nc) { - tree cond = bb_predicate (bb); + tree bc; - set_bb_predicate (bb, is_true_predicate (cond) ? new_cond : - fold_build2_loc (EXPR_LOCATION (cond), - TRUTH_OR_EXPR, boolean_type_node, - cond, new_cond)); + if (is_true_predicate (nc)) + return; + + if (!is_predicated (bb)) + bc = nc; + else + { + enum tree_code code1, code2; + tree op1a, op1b, op2a, op2b; + + bc = bb_predicate (bb); + code1 = parse_predicate (bc, &op1a, &op1b); + code2 = parse_predicate (nc, &op2a, &op2b); + + if (code1 != ERROR_MARK && code2 != ERROR_MARK) + { + bc = maybe_fold_or_comparisons (code1, op1a, op1b, code2, op2a, op2b); + + if (!bc) + { + bc = bb_predicate (bb); + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, + boolean_type_node, bc, nc); + } + } + else + bc = fold_build2_loc (EXPR_LOCATION (bc), TRUTH_OR_EXPR, + boolean_type_node, bc, nc); + } + + if (!is_gimple_condexpr (bc)) + { + gimple_seq stmts; + bc = force_gimple_operand (bc, &stmts, true, NULL_TREE); + add_bb_predicate_gimplified_stmts (bb, stmts); + } + + if (is_true_predicate (bc)) + reset_bb_predicate (bb); + else + set_bb_predicate (bb, bc); } /* Add the condition COND to the previous condition PREV_COND, and add