This is the mail archive of the gcc@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: Match-and-simplify and COND_EXPR


On Fri, Nov 7, 2014 at 2:23 AM, Richard Biener <rguenther@suse.de> wrote:
> On Thu, 6 Nov 2014, Andrew Pinski wrote:
>
>> On Thu, Nov 6, 2014 at 2:40 AM, Richard Biener <rguenther@suse.de> wrote:
>> > On Thu, 6 Nov 2014, Richard Biener wrote:
>> >
>> >> On Wed, 5 Nov 2014, Andrew Pinski wrote:
>> >>
>> >> > Hi,
>> >> >   I was trying to hook up tree-ssa-phiopt to match-and-simplify using
>> >> > either gimple_build (or rather using gimple_simplify depending on if
>> >> > we want to produce cond_expr for conditional move).  I ran into a
>> >> > problem.
>> >> > With the pattern below:
>> >> > /* a ? 0 : 1 -> a if 0 and 1 are integral types. */
>> >> > (simplify
>> >> >   (cond_expr @0 integer_zerop integer_onep)
>> >> >   (if (INTEGRAL_TYPE_P (type))
>> >> >     (convert @0)))
>> >>
>> >> Ok, so you are capturing a GENERIC expr here but nothing knows that.
>> >> It would work if you'd do (ugh)
>> >>
>> >> (for op (lt le eq ne ge gt)
>> >>  (simplify
>> >>   (cond_expr (op @0 @1) integer_zerop integer_onep)
>> >>   (if (INTEGRAL_TYPE_P (type))
>> >>    (convert (op @0 @1)))))
>> >> (simplify
>> >>  (cond_expr SSA_NAME@0 integer_zerop integer_onep)
>> >>   (if (INTEGRAL_TYPE_P (type))
>> >>    (convert @0))))
>> >>
>> >> as a workaround.  To make your version work will require (quite)
>> >> some special-casing in the code generator or maybe the resimplify
>> >> helper.  Let me see if I can cook up a "simple" fix.
>> >
>> > Sth like below (for the real fix this has to be replicated in
>> > all gimple_resimplifyN functions).  I'm missing a testcase
>> > where the pattern would apply (and not be already folded by fold),
>> > so I didn't check if it actually works.
>>
>> You do need to check if seq is NULL though as gimple_build depends on
>> seq not being NULL.  But otherwise yes this works for me.
>
> Ok, here is a better and more complete fix.  The optimization
> whether a captured expression may be a GENERIC condition isn't
> implemented so a lot of checks are emitted right now.  Also
> if gimple_build would fail this terminates the whole matching
> process, not just matching of the current pattern (failing "late"
> isn't supported with the style of code we emit - well, without
> adding ugly gotos and labels).
>
> At least it avoids splitting up conditions if substituted into
> a COND_EXPR, so simplifications of COND_EXPRs in-place (w/o seq)
> should still work.
>
> Can you check whether this works for you?

This works for most cases but does not work for things like:
/* a != b ? a : b -> a */
(simplify
  (cond_expr (ne @0 @1) @0 @1)
  @0)

In gimple_simplify after it matches COND_EXPR. It does not look into
the operand 0 to see if it matches NE_EXPR, it just sees if it was a
SSA_NAME.  In this case I was trying to do a simple replacement of
value_replacement in tree-ssa-phiopt.c.

But for my purpose right now this is sufficient and will be posting a
patch which does not remove things from tree-ssa-phiopt.c just yet.

Thanks,
Andrew Pinski


>
> Thanks,
> Richard.
>
>
>
> Index: gcc/genmatch.c
> ===================================================================
> --- gcc/genmatch.c      (revision 217213)
> +++ gcc/genmatch.c      (working copy)
> @@ -419,7 +419,8 @@ struct operand {
>    operand (enum op_type type_) : type (type_) {}
>    enum op_type type;
>    virtual void gen_transform (FILE *, const char *, bool, int,
> -                             const char *, dt_operand ** = 0)
> +                             const char *, dt_operand ** = 0,
> +                             bool = true)
>      { gcc_unreachable  (); }
>  };
>
> @@ -449,7 +450,7 @@ struct expr : public operand
>       later lowered to two separate patterns.  */
>    bool is_commutative;
>    virtual void gen_transform (FILE *f, const char *, bool, int,
> -                             const char *, dt_operand ** = 0);
> +                             const char *, dt_operand ** = 0, bool = true);
>  };
>
>  /* An operator that is represented by native C code.  This is always
> @@ -479,7 +480,7 @@ struct c_expr : public operand
>    /* The identifier replacement vector.  */
>    vec<id_tab> ids;
>    virtual void gen_transform (FILE *f, const char *, bool, int,
> -                             const char *, dt_operand **);
> +                             const char *, dt_operand ** = 0, bool = true);
>  };
>
>  /* A wrapper around another operand that captures its value.  */
> @@ -493,7 +494,7 @@ struct capture : public operand
>    /* The captured value.  */
>    operand *what;
>    virtual void gen_transform (FILE *f, const char *, bool, int,
> -                             const char *, dt_operand ** = 0);
> +                             const char *, dt_operand ** = 0, bool = true);
>  };
>
>  template<>
> @@ -1358,7 +1359,7 @@ get_operand_type (id_base *op, const cha
>
>  void
>  expr::gen_transform (FILE *f, const char *dest, bool gimple, int depth,
> -                    const char *in_type, dt_operand **indexes)
> +                    const char *in_type, dt_operand **indexes, bool)
>  {
>    bool conversion_p = is_conversion (operation);
>    const char *type = expr_type;
> @@ -1405,7 +1406,10 @@ expr::gen_transform (FILE *f, const char
>        const char *optype
>         = get_operand_type (operation, in_type, expr_type,
>                             i == 0 ? NULL : op0type);
> -      ops[i]->gen_transform (f, dest, gimple, depth + 1, optype, indexes);
> +      ops[i]->gen_transform (f, dest, gimple, depth + 1, optype, indexes,
> +                            ((!(*operation == COND_EXPR)
> +                              && !(*operation == VEC_COND_EXPR))
> +                             || i != 0));
>      }
>
>    const char *opr;
> @@ -1455,7 +1459,7 @@ expr::gen_transform (FILE *f, const char
>
>  void
>  c_expr::gen_transform (FILE *f, const char *dest,
> -                      bool, int, const char *, dt_operand **)
> +                      bool, int, const char *, dt_operand **, bool)
>  {
>    if (dest && nr_stmts == 1)
>      fprintf (f, "%s = ", dest);
> @@ -1524,7 +1528,8 @@ c_expr::gen_transform (FILE *f, const ch
>
>  void
>  capture::gen_transform (FILE *f, const char *dest, bool gimple, int depth,
> -                       const char *in_type, dt_operand **indexes)
> +                       const char *in_type, dt_operand **indexes,
> +                       bool expand_compares)
>  {
>    if (what && is_a<expr *> (what))
>      {
> @@ -1537,6 +1542,24 @@ capture::gen_transform (FILE *f, const c
>      }
>
>    fprintf (f, "%s = captures[%u];\n", dest, where);
> +
> +  /* ???  Stupid tcc_comparison GENERIC trees in COND_EXPRs.  Deal
> +     with substituting a capture of that.
> +     ???  We can optimize this in the generator because we should
> +     know whether this capture is from a COND_EXPR condition.  Also
> +     technically splitting the comparison up isn't required if we
> +     are substituting into a COND_EXPR condition (so simplification
> +     may unnecessarily fail if seq is NULL and a toplevel cond result).
> +     ???  Returning false here will also not allow any other patterns
> +     to match.  */
> +  if (gimple && expand_compares)
> +    fprintf (f, "if (COMPARISON_CLASS_P (%s))\n"
> +            "  {\n"
> +            "    if (!seq) return false;\n"
> +            "    %s = gimple_build (seq, TREE_CODE (%s),"
> +            " TREE_TYPE (%s), TREE_OPERAND (%s, 0),"
> +            " TREE_OPERAND (%s, 1));\n"
> +            "  }\n", dest, dest, dest, dest, dest, dest);
>  }
>
>  /* Return the name of the operand representing the decision tree node.
> @@ -1999,7 +2022,8 @@ capture_info::walk_match (operand *o, un
>         {
>           bool cond_p = conditional_p;
>           if (i == 0
> -             && *e->operation == COND_EXPR)
> +             && (*e->operation == COND_EXPR
> +                 || *e->operation == VEC_COND_EXPR))
>             cond_p = true;
>           else if (*e->operation == TRUTH_ANDIF_EXPR
>                    || *e->operation == TRUTH_ORIF_EXPR)
> @@ -2046,7 +2070,8 @@ capture_info::walk_result (operand *o, b
>         {
>           bool cond_p = conditional_p;
>           if (i == 0
> -             && *e->operation == COND_EXPR)
> +             && (*e->operation == COND_EXPR
> +                 || *e->operation == VEC_COND_EXPR))
>             cond_p = true;
>           else if (*e->operation == TRUTH_ANDIF_EXPR
>                    || *e->operation == TRUTH_ORIF_EXPR)
> @@ -2226,7 +2251,11 @@ dt_simplify::gen (FILE *f, bool gimple)
>                                     "type", e->expr_type,
>                                     j == 0
>                                     ? NULL : "TREE_TYPE (res_ops[0])");
> -             e->ops[j]->gen_transform (f, dest, true, 1, optype, indexes);
> +             e->ops[j]->gen_transform (f, dest, true, 1, optype, indexes,
> +                                       !is_predicate
> +                                       && ((!(*e->operation == COND_EXPR)
> +                                            && !(*e->operation == VEC_COND_EXPR))
> +                                           || j != 0));
>             }
>
>           /* Re-fold the toplevel result.  It's basically an embedded
> @@ -2238,8 +2267,21 @@ dt_simplify::gen (FILE *f, bool gimple)
>        else if (result->type == operand::OP_CAPTURE
>                || result->type == operand::OP_C_EXPR)
>         {
> -         result->gen_transform (f, "res_ops[0]", true, 1, "type", indexes);
> -         fprintf (f, "*res_code = TREE_CODE (res_ops[0]);\n");
> +         result->gen_transform (f, "res_ops[0]", true, 1, "type",
> +                                indexes, false);
> +         /* ???  Stupid tcc_comparison GENERIC trees in COND_EXPRs.  Deal
> +            with substituting a capture of that.
> +            ???  We can optimize this in the generator because we should
> +            know whether this capture is from a COND_EXPR condition.  */
> +         fprintf (f, "if (COMPARISON_CLASS_P (res_ops[0]))\n"
> +                  "  {\n"
> +                  "    tree tem = res_ops[0];\n"
> +                  "    res_ops[0] = TREE_OPERAND (tem, 0);\n"
> +                  "    res_ops[1] = TREE_OPERAND (tem, 1);\n"
> +                  "    *res_code = TREE_CODE (tem);\n"
> +                  "  }\n"
> +                  "else\n"
> +                  "  *res_code = TREE_CODE (res_ops[0]);\n");
>         }
>        else
>         gcc_unreachable ();


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