Patchwork [2/4] Call maybe_fold_or_comparisons to fold OR-ed predicates.

login
register
mail settings
Submitter Sebastian Pop
Date July 7, 2010, 8:22 p.m.
Message ID <1278534137-22733-3-git-send-email-sebpop@gmail.com>
Download mbox | patch
Permalink /patch/58165/
State New
Headers show

Comments

Sebastian Pop - July 7, 2010, 8:22 p.m.
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
Richard Guenther - July 8, 2010, 9:57 a.m.
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.
Sebastian Pop - July 8, 2010, 4:40 p.m.
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.
Richard Guenther - July 9, 2010, 12:10 p.m.
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.
Sebastian Pop - July 9, 2010, 4:40 p.m.
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
Richard Guenther - July 9, 2010, 4:45 p.m.
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.

Patch

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