Re: Match-and-simplify and COND_EXPR

[Richard Biener <rguenther at suse dot de>]

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?

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 ();