This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/4] Call maybe_fold_or_comparisons to fold OR-ed predicates.


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]