[RFC] Drop GENERIC conds in [VEC_]COND_EXPRs

Message ID alpine.LSU.2.20.1806051627030.5043@zhemvz.fhfr.qr
State New
Headers show
Series
  • [RFC] Drop GENERIC conds in [VEC_]COND_EXPRs
Related show

Commit Message

Richard Biener June 5, 2018, 2:37 p.m.
This is a prototype enforcing is_gimple_val conditions in 
[VEC_]COND_EXPRs.  It shows quite some fallout - some likely
due to necessary testsuite adjustments like gcc.dg/tree-ssa/ssa-lim-12.c
others because some code simply doesn't handle conditions visible
only via following SSA defs like vect_recog_mixed_size_cond_pattern.

So on x86_64 there's quite some vect.exp fallout.

Anyhow, to assess the effect on other targets I'm throwing this
in here.

I've hopefully nailed all ICEs (or at least makes catching them
easy with some asserts in GIMPLE stmt building).

Full bootstrap and regtest for all languages running on 
x86_64-unknown-linux-gnu.

I didn't bother to axe all the ugliness out of genmatch that exists
because of those GENERIC exprs in [VEC_]COND_EXPRs yet.

Richard.

Comments

Richard Biener June 6, 2018, 11:02 a.m. | #1
On Tue, 5 Jun 2018, Richard Biener wrote:

> 

> This is a prototype enforcing is_gimple_val conditions in 

> [VEC_]COND_EXPRs.  It shows quite some fallout - some likely

> due to necessary testsuite adjustments like gcc.dg/tree-ssa/ssa-lim-12.c

> others because some code simply doesn't handle conditions visible

> only via following SSA defs like vect_recog_mixed_size_cond_pattern.

> 

> So on x86_64 there's quite some vect.exp fallout.

> 

> Anyhow, to assess the effect on other targets I'm throwing this

> in here.

> 

> I've hopefully nailed all ICEs (or at least makes catching them

> easy with some asserts in GIMPLE stmt building).

> 

> Full bootstrap and regtest for all languages running on 

> x86_64-unknown-linux-gnu.


Testing shows a few omissions in the patch (updated patch attached
below).  It also shows that expand_vec_cond_expr_p and friends
need some heavy lifting to deal with vcond* patterns which have
the comparison embedded.  There's vcond_mask* which would be
more suited to the GIMPLE we have after this patch and I suspect
we can always expand

 _1 = _2 < _3;
 _4 = _1 ? _5 : _66;

as

 _1 = _2 < _3 ? { -1,...} : {0,...}; via vcond_*
 _4 = _1 != 0 ? _5 : _6; via vcond_*

at expansion time TER might come to the rescue, if there are multiple
uses of _1 then there's cost issues to honor in case _1 = _2 < _3
can be expanded more efficiently than via such fake vcond_*.

In any case the expander now needs to be made more smart
(as well as the pattern recog part of the vectorizer).

Jakub - I remember you weren't too happy about splitting the
conditions out of [VEC_]COND_EXPRs this came up last time?
With the alternative idea of transitioning [VEC_]COND_EXPRs to
tcc_comparisons with conditional ops aka

 _1 = _2 < _3 ? _5 : _6;

with RHS code LT_EXPR and 4 operands what do you think of
Richards complaint about being forced to repeat _2 < _3?
A split out _2 < _3 would then look like

 _4 = _2 < _3 ? {-1,...} : {0,...};
 _1 = _4 != {0,...} ? _5 : _6;

so "plain" LT_EXPR rhs wouldn't be supported anymore but they'd
always have explicit true and false values attached.  With making
the last two ops optional we could make them implicit again of course.
We could also keep [VEC_]COND_EXPR then in 3-operand form with
a gimple-val predicate rather than a comparison.  Of course that
wouldn't really simplify the IL overall...

Richard.

From bf05ad5789bb79452364c0e22f798b66013a0919 Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguenther@suse.de>

Date: Tue, 5 Jun 2018 16:09:26 +0200
Subject: [PATCH] vec_cond_expr_gimple_val

vect_recog_mixed_size_cond_pattern will be disabled

diff --git a/gcc/gimple-loop-interchange.cc b/gcc/gimple-loop-interchange.cc
index eb35263e69c..23fbed590e0 100644
--- a/gcc/gimple-loop-interchange.cc
+++ b/gcc/gimple-loop-interchange.cc
@@ -891,7 +891,9 @@ loop_cand::undo_simple_reduction (reduction_p re, bitmap dce_seeds)
       /* Init new_var to MEM_REF or CONST depending on if it is the first
 	 iteration.  */
       induction_p iv = m_inductions[0];
-      cond = fold_build2 (NE_EXPR, boolean_type_node, iv->var, iv->init_val);
+      cond = make_ssa_name (boolean_type_node);
+      stmt = gimple_build_assign (cond, NE_EXPR, iv->var, iv->init_val);
+      gimple_seq_add_stmt_without_update (&stmts, stmt);
       new_var = copy_ssa_name (re->var);
       stmt = gimple_build_assign (new_var, COND_EXPR, cond, tmp, re->init);
       gimple_seq_add_stmt_without_update (&stmts, stmt);
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 4b91151873c..24d7963117e 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -442,6 +442,8 @@ gimple_build_assign_1 (tree lhs, enum tree_code subcode, tree op1,
         gimple_build_with_ops_stat (GIMPLE_ASSIGN, (unsigned)subcode, num_ops
 				    PASS_MEM_STAT));
   gimple_assign_set_lhs (p, lhs);
+  gcc_assert ((subcode != COND_EXPR && subcode != VEC_COND_EXPR)
+	      || is_gimple_val (op1));
   gimple_assign_set_rhs1 (p, op1);
   if (op2)
     {
@@ -1691,6 +1693,8 @@ gimple_assign_set_rhs_with_ops (gimple_stmt_iterator *gsi, enum tree_code code,
 
   gimple_set_num_ops (stmt, new_rhs_ops + 1);
   gimple_set_subcode (stmt, code);
+  gcc_assert ((code != COND_EXPR && code != VEC_COND_EXPR)
+	      || is_gimple_val (op1));
   gimple_assign_set_rhs1 (stmt, op1);
   if (new_rhs_ops > 1)
     gimple_assign_set_rhs2 (stmt, op2);
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 44cb784620a..b679241e377 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3899,7 +3899,7 @@ gimplify_pure_cond_expr (tree *expr_p, gimple_seq *pre_p)
     TREE_SET_CODE (cond, TRUTH_AND_EXPR);
   else if (code == TRUTH_ORIF_EXPR)
     TREE_SET_CODE (cond, TRUTH_OR_EXPR);
-  ret = gimplify_expr (&cond, pre_p, NULL, is_gimple_condexpr, fb_rvalue);
+  ret = gimplify_expr (&cond, pre_p, NULL, is_gimple_val, fb_rvalue);
   COND_EXPR_COND (*expr_p) = cond;
 
   tret = gimplify_expr (&COND_EXPR_THEN (expr), pre_p, NULL,
@@ -12078,7 +12078,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    enum gimplify_status r0, r1, r2;
 
 	    r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
-				post_p, is_gimple_condexpr, fb_rvalue);
+				post_p, is_gimple_val, fb_rvalue);
 	    r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
 				post_p, is_gimple_val, fb_rvalue);
 	    r2 = gimplify_expr (&TREE_OPERAND (*expr_p, 2), pre_p,
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 21b3fdffa59..5f6defa6fe7 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -4137,10 +4137,7 @@ verify_gimple_assign_ternary (gassign *stmt)
       return true;
     }
 
-  if (((rhs_code == VEC_COND_EXPR || rhs_code == COND_EXPR)
-       ? !is_gimple_condexpr (rhs1) : !is_gimple_val (rhs1))
-      || !is_gimple_val (rhs2)
-      || !is_gimple_val (rhs3))
+  if (!is_gimple_val (rhs1) || !is_gimple_val (rhs2) || !is_gimple_val (rhs3))
     {
       error ("invalid operands in ternary operation");
       return true;
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 71dac4fb48a..fe9eb83876f 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -1762,6 +1762,9 @@ gen_phi_arg_condition (gphi *phi, vec<int> *occur,
 	cond = c;
     }
   gcc_assert (cond != NULL_TREE);
+  cond = force_gimple_operand_gsi_1 (gsi, cond,
+				     is_gimple_val, NULL_TREE,
+				     true, GSI_SAME_STMT);
   return cond;
 }
 
@@ -1844,7 +1847,7 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
 	cond = bb_predicate (first_edge->src);
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
       cond = force_gimple_operand_gsi_1 (gsi, unshare_expr (cond),
-					 is_gimple_condexpr, NULL_TREE,
+					 is_gimple_val, NULL_TREE,
 					 true, GSI_SAME_STMT);
       true_bb = first_edge->src;
       if (EDGE_PRED (bb, 1)->src == true_bb)
@@ -1940,7 +1943,7 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
 	}
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
       cond = force_gimple_operand_gsi_1 (gsi, unshare_expr (cond),
-					 is_gimple_condexpr, NULL_TREE,
+					 is_gimple_val, NULL_TREE,
 					 true, GSI_SAME_STMT);
       if (!(is_cond_scalar_reduction (phi, &reduc, arg0 , arg1,
 				      &op0, &op1, true)))
@@ -2323,7 +2326,7 @@ predicate_mem_writes (loop_p loop)
 	      if (swap)
 		std::swap (lhs, rhs);
 	      cond = force_gimple_operand_gsi_1 (&gsi, unshare_expr (cond),
-						 is_gimple_condexpr, NULL_TREE,
+						 is_gimple_val, NULL_TREE,
 						 true, GSI_SAME_STMT);
 	      rhs = fold_build_cond_expr (type, unshare_expr (cond), rhs, lhs);
 	      gimple_assign_set_rhs1 (stmt, ifc_temp_var (type, rhs, &gsi));
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index 2efb5acae8f..95621d1f58b 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -504,9 +504,7 @@ forward_propagate_into_comparison (gimple_stmt_iterator *gsi)
 /* Propagate from the ssa name definition statements of COND_EXPR
    in GIMPLE_COND statement STMT into the conditional if that simplifies it.
    Returns zero if no statement was changed, one if there were
-   changes and two if cfg_cleanup needs to run.
-
-   This must be kept in sync with forward_propagate_into_cond.  */
+   changes and two if cfg_cleanup needs to run.  */
 
 static int
 forward_propagate_into_gimple_cond (gcond *stmt)
@@ -565,70 +563,6 @@ forward_propagate_into_gimple_cond (gcond *stmt)
   return 0;
 }
 
-
-/* Propagate from the ssa name definition statements of COND_EXPR
-   in the rhs of statement STMT into the conditional if that simplifies it.
-   Returns true zero if the stmt was changed.  */
-
-static bool
-forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
-{
-  gimple *stmt = gsi_stmt (*gsi_p);
-  tree tmp = NULL_TREE;
-  tree cond = gimple_assign_rhs1 (stmt);
-  enum tree_code code = gimple_assign_rhs_code (stmt);
-
-  /* We can do tree combining on SSA_NAME and comparison expressions.  */
-  if (COMPARISON_CLASS_P (cond))
-    tmp = forward_propagate_into_comparison_1 (stmt, TREE_CODE (cond),
-					       TREE_TYPE (cond),
-					       TREE_OPERAND (cond, 0),
-					       TREE_OPERAND (cond, 1));
-  else if (TREE_CODE (cond) == SSA_NAME)
-    {
-      enum tree_code def_code;
-      tree name = cond;
-      gimple *def_stmt = get_prop_source_stmt (name, true, NULL);
-      if (!def_stmt || !can_propagate_from (def_stmt))
-	return 0;
-
-      def_code = gimple_assign_rhs_code (def_stmt);
-      if (TREE_CODE_CLASS (def_code) == tcc_comparison)
-	tmp = fold_build2_loc (gimple_location (def_stmt),
-			       def_code,
-			       TREE_TYPE (cond),
-			       gimple_assign_rhs1 (def_stmt),
-			       gimple_assign_rhs2 (def_stmt));
-    }
-
-  if (tmp
-      && is_gimple_condexpr (tmp))
-    {
-      if (dump_file && tmp)
-	{
-	  fprintf (dump_file, "  Replaced '");
-	  print_generic_expr (dump_file, cond);
-	  fprintf (dump_file, "' with '");
-	  print_generic_expr (dump_file, tmp);
-	  fprintf (dump_file, "'\n");
-	}
-
-      if ((code == VEC_COND_EXPR) ? integer_all_onesp (tmp)
-				  : integer_onep (tmp))
-	gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs2 (stmt));
-      else if (integer_zerop (tmp))
-	gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs3 (stmt));
-      else
-	gimple_assign_set_rhs1 (stmt, unshare_expr (tmp));
-      stmt = gsi_stmt (*gsi_p);
-      update_stmt (stmt);
-
-      return true;
-    }
-
-  return 0;
-}
-
 /* We've just substituted an ADDR_EXPR into stmt.  Update all the
    relevant data structures to match.  */
 
@@ -2482,17 +2416,7 @@ pass_forwprop::execute (function *fun)
 		tree rhs1 = gimple_assign_rhs1 (stmt);
 		enum tree_code code = gimple_assign_rhs_code (stmt);
 
-		if (code == COND_EXPR
-		    || code == VEC_COND_EXPR)
-		  {
-		    /* In this case the entire COND_EXPR is in rhs1. */
-		    if (forward_propagate_into_cond (&gsi))
-		      {
-			changed = true;
-			stmt = gsi_stmt (gsi);
-		      }
-		  }
-		else if (TREE_CODE_CLASS (code) == tcc_comparison)
+		if (TREE_CODE_CLASS (code) == tcc_comparison)
 		  {
 		    int did_something;
 		    did_something = forward_propagate_into_comparison (&gsi);
diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 01a954eeb1e..df010e1087f 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -1140,8 +1140,11 @@ move_computations_worker (basic_block bb)
 	     edges of COND.  */
 	  extract_true_false_args_from_phi (dom, stmt, &arg0, &arg1);
 	  gcc_assert (arg0 && arg1);
-	  t = build2 (gimple_cond_code (cond), boolean_type_node,
-		      gimple_cond_lhs (cond), gimple_cond_rhs (cond));
+	  t = make_ssa_name (boolean_type_node);
+	  new_stmt = gimple_build_assign (t, gimple_cond_code (cond),
+					  gimple_cond_lhs (cond),
+					  gimple_cond_rhs (cond));
+	  gsi_insert_on_edge (loop_preheader_edge (level), new_stmt);
 	  new_stmt = gimple_build_assign (gimple_phi_result (stmt),
 					  COND_EXPR, t, arg0, arg1);
 	  todo |= TODO_cleanup_cfg;
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index 46502c42c74..66c6fe2237a 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -655,7 +655,9 @@ expand_vector_divmod (gimple_stmt_iterator *gsi, tree type, tree op0,
 
 	      mask_type = build_same_sized_truth_vector_type (type);
 	      zero = build_zero_cst (type);
-	      cond = build2 (LT_EXPR, mask_type, op0, zero);
+	      cond = make_ssa_name (mask_type);
+	      stmt = gimple_build_assign (cond, LT_EXPR, op0, zero);
+	      gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
 	      tree_vector_builder vec (type, nunits, 1);
 	      for (i = 0; i < nunits; i++)
 		vec.quick_push (build_int_cst (TREE_TYPE (type),
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index f99484f1da5..1418c83690b 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -2701,8 +2701,10 @@ vect_recog_divmod_pattern (vec<gimple *> *stmts,
         dump_printf_loc (MSG_NOTE, vect_location,
                          "vect_recog_divmod_pattern: detected:\n");
 
-      cond = build2 (LT_EXPR, boolean_type_node, oprnd0,
-		     build_int_cst (itype, 0));
+      cond = make_ssa_name (boolean_type_node);
+      def_stmt = gimple_build_assign (cond, LT_EXPR, oprnd0,
+				      build_int_cst (itype, 0));
+      new_pattern_def_seq (stmt_vinfo, def_stmt);
       if (rhs_code == TRUNC_DIV_EXPR)
 	{
 	  tree var = vect_recog_temp_ssa_var (itype, NULL);
@@ -2712,7 +2714,7 @@ vect_recog_divmod_pattern (vec<gimple *> *stmts,
 				   fold_build2 (MINUS_EXPR, itype, oprnd1,
 						build_int_cst (itype, 1)),
 				   build_int_cst (itype, 0));
-	  new_pattern_def_seq (stmt_vinfo, def_stmt);
+	  append_pattern_def_seq (stmt_vinfo, def_stmt);
 	  var = vect_recog_temp_ssa_var (itype, NULL);
 	  def_stmt
 	    = gimple_build_assign (var, PLUS_EXPR, oprnd0,
@@ -2727,7 +2729,6 @@ vect_recog_divmod_pattern (vec<gimple *> *stmts,
       else
 	{
 	  tree signmask;
-	  STMT_VINFO_PATTERN_DEF_SEQ (stmt_vinfo) = NULL;
 	  if (compare_tree_int (oprnd1, 2) == 0)
 	    {
 	      signmask = vect_recog_temp_ssa_var (itype, NULL);
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 74f813e7334..9956b453fd5 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -4160,7 +4160,9 @@ vr_values::simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
 	     in divide by zero, new_rhs1 / new_rhs will be NULL_TREE.  */
 	  if (new_rhs1 && new_rhs2)
 	    {
-	      tree cond = build2 (EQ_EXPR, boolean_type_node, cmp_var, val1);
+	      tree cond = make_ssa_name (boolean_type_node);
+	      gimple *def = gimple_build_assign (cond, EQ_EXPR, cmp_var, val1);
+	      gsi_insert_before (gsi, def, GSI_SAME_STMT);
 	      gimple_assign_set_rhs_with_ops (gsi,
 					      COND_EXPR, cond,
 					      new_rhs1,
Richard Sandiford June 6, 2018, 8:59 p.m. | #2
Richard Biener <rguenther@suse.de> writes:
> On Tue, 5 Jun 2018, Richard Biener wrote:

>

>> 

>> This is a prototype enforcing is_gimple_val conditions in 

>> [VEC_]COND_EXPRs.  It shows quite some fallout - some likely

>> due to necessary testsuite adjustments like gcc.dg/tree-ssa/ssa-lim-12.c

>> others because some code simply doesn't handle conditions visible

>> only via following SSA defs like vect_recog_mixed_size_cond_pattern.

>> 

>> So on x86_64 there's quite some vect.exp fallout.

>> 

>> Anyhow, to assess the effect on other targets I'm throwing this

>> in here.

>> 

>> I've hopefully nailed all ICEs (or at least makes catching them

>> easy with some asserts in GIMPLE stmt building).

>> 

>> Full bootstrap and regtest for all languages running on 

>> x86_64-unknown-linux-gnu.


FWIW, I ran the original version through our internal SVE benchmarks.
There were quite a few ICEs from:

      pattern_stmt
        = gimple_build_assign (vect_recog_temp_ssa_var (itype, NULL),
                               COND_EXPR, cond_expr, trueval,
                               build_int_cst (itype, 0));

in adjust_bool_pattern and some from loop_cand::undo_simple_reduction
(which it looks like you fixed in the updated patch).  But of the
tests that successfully built, it looks like this is neutral (good!).

I can try to fix the adjust_bool_pattern thing if you don't see it for AVX.

> Testing shows a few omissions in the patch (updated patch attached

> below).  It also shows that expand_vec_cond_expr_p and friends

> need some heavy lifting to deal with vcond* patterns which have

> the comparison embedded.  There's vcond_mask* which would be

> more suited to the GIMPLE we have after this patch and I suspect

> we can always expand

>

>  _1 = _2 < _3;

>  _4 = _1 ? _5 : _66;

>

> as

>

>  _1 = _2 < _3 ? { -1,...} : {0,...}; via vcond_*

>  _4 = _1 != 0 ? _5 : _6; via vcond_*

>

> at expansion time TER might come to the rescue, if there are multiple

> uses of _1 then there's cost issues to honor in case _1 = _2 < _3

> can be expanded more efficiently than via such fake vcond_*.

>

> In any case the expander now needs to be made more smart

> (as well as the pattern recog part of the vectorizer).


Why not match them as internal functions, like we now do for FMA* etc.?
It seems like a similar case: for FMAs we wanted to fold in negates while
here we want to fold in the comparison.  The fold could be handled by
match.pd, gated on a suitably late predicate.  (Unless the vectoriser
really does have to generate it directly, in which case maybe it could
also/instead be done as a pattern.  That might even make the generation
side simpler, since the pattern would just go through vectorizable_call.)

Using one internal function per comparison type would also help to clean
up the optabs interface.  At the moment, targets basically have to
accept any comparison type that's going, regardless of what the target
can actually support.  If we use internal functions that directly map to
individual optabs, then we can use target-independent code to make the
comparison fit.  (At least for the obvious cases.  Some FP comparisons
might still be better emulated directly by the target.)

> Jakub - I remember you weren't too happy about splitting the

> conditions out of [VEC_]COND_EXPRs this came up last time?

> With the alternative idea of transitioning [VEC_]COND_EXPRs to

> tcc_comparisons with conditional ops aka

>

>  _1 = _2 < _3 ? _5 : _6;

>

> with RHS code LT_EXPR and 4 operands what do you think of

> Richards complaint about being forced to repeat _2 < _3?

> A split out _2 < _3 would then look like

>

>  _4 = _2 < _3 ? {-1,...} : {0,...};

>  _1 = _4 != {0,...} ? _5 : _6;

>

> so "plain" LT_EXPR rhs wouldn't be supported anymore but they'd

> always have explicit true and false values attached.  With making

> the last two ops optional we could make them implicit again of course.

> We could also keep [VEC_]COND_EXPR then in 3-operand form with

> a gimple-val predicate rather than a comparison.  Of course that

> wouldn't really simplify the IL overall...


Yeah, this still doesn't seem like an improvement to me TBH.  For one
thing it'd be confusing for LT_EXPR to have 2 operands for generic and
4 for gimple: are there any other cases where we do that?  I suppose the
GIMPLE_SINGLE_RHS stuff is a special case too, but it sounded like you
saw that was a wart and wanted to move away from it.

Thanks,
Richard
Richard Biener June 7, 2018, 11:03 a.m. | #3
On Wed, 6 Jun 2018, Richard Sandiford wrote:

> Richard Biener <rguenther@suse.de> writes:

> > On Tue, 5 Jun 2018, Richard Biener wrote:

> >

> >> 

> >> This is a prototype enforcing is_gimple_val conditions in 

> >> [VEC_]COND_EXPRs.  It shows quite some fallout - some likely

> >> due to necessary testsuite adjustments like gcc.dg/tree-ssa/ssa-lim-12.c

> >> others because some code simply doesn't handle conditions visible

> >> only via following SSA defs like vect_recog_mixed_size_cond_pattern.

> >> 

> >> So on x86_64 there's quite some vect.exp fallout.

> >> 

> >> Anyhow, to assess the effect on other targets I'm throwing this

> >> in here.

> >> 

> >> I've hopefully nailed all ICEs (or at least makes catching them

> >> easy with some asserts in GIMPLE stmt building).

> >> 

> >> Full bootstrap and regtest for all languages running on 

> >> x86_64-unknown-linux-gnu.

> 

> FWIW, I ran the original version through our internal SVE benchmarks.

> There were quite a few ICEs from:

> 

>       pattern_stmt

>         = gimple_build_assign (vect_recog_temp_ssa_var (itype, NULL),

>                                COND_EXPR, cond_expr, trueval,

>                                build_int_cst (itype, 0));

> 

> in adjust_bool_pattern and some from loop_cand::undo_simple_reduction

> (which it looks like you fixed in the updated patch).  But of the

> tests that successfully built, it looks like this is neutral (good!).

> 

> I can try to fix the adjust_bool_pattern thing if you don't see it for AVX.


Didn't run into it indeed.  A fix would probably look like

diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 1418c83690b..507c5b94f07 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -3484,7 +3484,9 @@ adjust_bool_pattern (tree var, tree out_type,
        }
       else
        itype = TREE_TYPE (rhs1);
-      cond_expr = build2_loc (loc, rhs_code, itype, rhs1, rhs2);
+      cond_expr = make_ssa_name (itype);
+      pattern_stmt = gimple_build_assign (cond_expr, rhs_code, rhs1, 
rhs2);
+      append_pattern_def_seq (stmt_info, pattern_stmt);
       if (trueval == NULL_TREE)
        trueval = build_int_cst (itype, 1);
       else

included in the updated version below.

> > Testing shows a few omissions in the patch (updated patch attached

> > below).  It also shows that expand_vec_cond_expr_p and friends

> > need some heavy lifting to deal with vcond* patterns which have

> > the comparison embedded.  There's vcond_mask* which would be

> > more suited to the GIMPLE we have after this patch and I suspect

> > we can always expand

> >

> >  _1 = _2 < _3;

> >  _4 = _1 ? _5 : _66;

> >

> > as

> >

> >  _1 = _2 < _3 ? { -1,...} : {0,...}; via vcond_*

> >  _4 = _1 != 0 ? _5 : _6; via vcond_*

> >

> > at expansion time TER might come to the rescue, if there are multiple

> > uses of _1 then there's cost issues to honor in case _1 = _2 < _3

> > can be expanded more efficiently than via such fake vcond_*.

> >

> > In any case the expander now needs to be made more smart

> > (as well as the pattern recog part of the vectorizer).

> 

> Why not match them as internal functions, like we now do for FMA* etc.?

> It seems like a similar case: for FMAs we wanted to fold in negates while

> here we want to fold in the comparison.  The fold could be handled by

> match.pd, gated on a suitably late predicate.  (Unless the vectoriser

> really does have to generate it directly, in which case maybe it could

> also/instead be done as a pattern.  That might even make the generation

> side simpler, since the pattern would just go through vectorizable_call.)


I think the vectorizer needs to generate it directly since the invariant
is it emits code the targets can actually handle (so tree-vect-generic.c
doesn't undo things).  So that would basically mean to scrap
VEC_COND_EXPR and instead have direct optabs IFNs for
vcond*_* and eventually vec_cmp*.  Of course the disadvantage is that
we need to handle those in followup optimiziation (we're not too good
in generating optimal predicated code...).

> Using one internal function per comparison type would also help to clean

> up the optabs interface.  At the moment, targets basically have to

> accept any comparison type that's going, regardless of what the target

> can actually support.  If we use internal functions that directly map to

> individual optabs, then we can use target-independent code to make the

> comparison fit.  (At least for the obvious cases.  Some FP comparisons

> might still be better emulated directly by the target.)


We could try at least handling the TER/combine issue by a late
pass doing the matching to IFNs for the vcond*_* optabs.  Or have
tree-vect-generic.c do it (and lower the rest).  Still the
vectorizer needs to make sure to generate only target supported
stuff - not sure how good of a job it does here given later
optimizations might mangle the GIMPLE and we're only careful
when folding VEC_PERM_EXPRs.

> > Jakub - I remember you weren't too happy about splitting the

> > conditions out of [VEC_]COND_EXPRs this came up last time?

> > With the alternative idea of transitioning [VEC_]COND_EXPRs to

> > tcc_comparisons with conditional ops aka

> >

> >  _1 = _2 < _3 ? _5 : _6;

> >

> > with RHS code LT_EXPR and 4 operands what do you think of

> > Richards complaint about being forced to repeat _2 < _3?

> > A split out _2 < _3 would then look like

> >

> >  _4 = _2 < _3 ? {-1,...} : {0,...};

> >  _1 = _4 != {0,...} ? _5 : _6;

> >

> > so "plain" LT_EXPR rhs wouldn't be supported anymore but they'd

> > always have explicit true and false values attached.  With making

> > the last two ops optional we could make them implicit again of course.

> > We could also keep [VEC_]COND_EXPR then in 3-operand form with

> > a gimple-val predicate rather than a comparison.  Of course that

> > wouldn't really simplify the IL overall...

> 

> Yeah, this still doesn't seem like an improvement to me TBH.  For one

> thing it'd be confusing for LT_EXPR to have 2 operands for generic and

> 4 for gimple: are there any other cases where we do that?  I suppose the

> GIMPLE_SINGLE_RHS stuff is a special case too, but it sounded like you

> saw that was a wart and wanted to move away from it.


Aww, didn't think of GENERIC here ;)

But yes, just doing the splitting out of the GENERIC comparison
op of [VEC_]COND_EXPR and somehow dealing with the fallout would be
my preference at this point.  I'll work some more of this after
I return from a short vacation mid next week.  At least if we can
agree on that simple plan.

GIMPLE_SINGLE_RHS stuff is special but unrelated.  There's currently
only WITH_SIZE_EXPR and OBJ_TYPE_REF that could be moved out
but IIRC those only appear in operand context and not standalone.

Note that all this rhs-class stuff is really a useless indirection
over the num_ops interface we already have... (or over the
tree code class you can query from the rhs code).  I'd love to
get rid of it ;)

Richard.

From b3785e1148f2dbbbac82d544ea4efe09d25378de Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguenther@suse.de>

Date: Tue, 5 Jun 2018 16:09:26 +0200
Subject: [PATCH] vec_cond_expr_gimple_val

vect_recog_mixed_size_cond_pattern will be disabled

diff --git a/gcc/gimple-loop-interchange.cc b/gcc/gimple-loop-interchange.cc
index eb35263e69c..23fbed590e0 100644
--- a/gcc/gimple-loop-interchange.cc
+++ b/gcc/gimple-loop-interchange.cc
@@ -891,7 +891,9 @@ loop_cand::undo_simple_reduction (reduction_p re, bitmap dce_seeds)
       /* Init new_var to MEM_REF or CONST depending on if it is the first
 	 iteration.  */
       induction_p iv = m_inductions[0];
-      cond = fold_build2 (NE_EXPR, boolean_type_node, iv->var, iv->init_val);
+      cond = make_ssa_name (boolean_type_node);
+      stmt = gimple_build_assign (cond, NE_EXPR, iv->var, iv->init_val);
+      gimple_seq_add_stmt_without_update (&stmts, stmt);
       new_var = copy_ssa_name (re->var);
       stmt = gimple_build_assign (new_var, COND_EXPR, cond, tmp, re->init);
       gimple_seq_add_stmt_without_update (&stmts, stmt);
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 4b91151873c..24d7963117e 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -442,6 +442,8 @@ gimple_build_assign_1 (tree lhs, enum tree_code subcode, tree op1,
         gimple_build_with_ops_stat (GIMPLE_ASSIGN, (unsigned)subcode, num_ops
 				    PASS_MEM_STAT));
   gimple_assign_set_lhs (p, lhs);
+  gcc_assert ((subcode != COND_EXPR && subcode != VEC_COND_EXPR)
+	      || is_gimple_val (op1));
   gimple_assign_set_rhs1 (p, op1);
   if (op2)
     {
@@ -1691,6 +1693,8 @@ gimple_assign_set_rhs_with_ops (gimple_stmt_iterator *gsi, enum tree_code code,
 
   gimple_set_num_ops (stmt, new_rhs_ops + 1);
   gimple_set_subcode (stmt, code);
+  gcc_assert ((code != COND_EXPR && code != VEC_COND_EXPR)
+	      || is_gimple_val (op1));
   gimple_assign_set_rhs1 (stmt, op1);
   if (new_rhs_ops > 1)
     gimple_assign_set_rhs2 (stmt, op2);
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 44cb784620a..b679241e377 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3899,7 +3899,7 @@ gimplify_pure_cond_expr (tree *expr_p, gimple_seq *pre_p)
     TREE_SET_CODE (cond, TRUTH_AND_EXPR);
   else if (code == TRUTH_ORIF_EXPR)
     TREE_SET_CODE (cond, TRUTH_OR_EXPR);
-  ret = gimplify_expr (&cond, pre_p, NULL, is_gimple_condexpr, fb_rvalue);
+  ret = gimplify_expr (&cond, pre_p, NULL, is_gimple_val, fb_rvalue);
   COND_EXPR_COND (*expr_p) = cond;
 
   tret = gimplify_expr (&COND_EXPR_THEN (expr), pre_p, NULL,
@@ -12078,7 +12078,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    enum gimplify_status r0, r1, r2;
 
 	    r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
-				post_p, is_gimple_condexpr, fb_rvalue);
+				post_p, is_gimple_val, fb_rvalue);
 	    r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
 				post_p, is_gimple_val, fb_rvalue);
 	    r2 = gimplify_expr (&TREE_OPERAND (*expr_p, 2), pre_p,
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 21b3fdffa59..5f6defa6fe7 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -4137,10 +4137,7 @@ verify_gimple_assign_ternary (gassign *stmt)
       return true;
     }
 
-  if (((rhs_code == VEC_COND_EXPR || rhs_code == COND_EXPR)
-       ? !is_gimple_condexpr (rhs1) : !is_gimple_val (rhs1))
-      || !is_gimple_val (rhs2)
-      || !is_gimple_val (rhs3))
+  if (!is_gimple_val (rhs1) || !is_gimple_val (rhs2) || !is_gimple_val (rhs3))
     {
       error ("invalid operands in ternary operation");
       return true;
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 71dac4fb48a..fe9eb83876f 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -1762,6 +1762,9 @@ gen_phi_arg_condition (gphi *phi, vec<int> *occur,
 	cond = c;
     }
   gcc_assert (cond != NULL_TREE);
+  cond = force_gimple_operand_gsi_1 (gsi, cond,
+				     is_gimple_val, NULL_TREE,
+				     true, GSI_SAME_STMT);
   return cond;
 }
 
@@ -1844,7 +1847,7 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
 	cond = bb_predicate (first_edge->src);
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
       cond = force_gimple_operand_gsi_1 (gsi, unshare_expr (cond),
-					 is_gimple_condexpr, NULL_TREE,
+					 is_gimple_val, NULL_TREE,
 					 true, GSI_SAME_STMT);
       true_bb = first_edge->src;
       if (EDGE_PRED (bb, 1)->src == true_bb)
@@ -1940,7 +1943,7 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
 	}
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
       cond = force_gimple_operand_gsi_1 (gsi, unshare_expr (cond),
-					 is_gimple_condexpr, NULL_TREE,
+					 is_gimple_val, NULL_TREE,
 					 true, GSI_SAME_STMT);
       if (!(is_cond_scalar_reduction (phi, &reduc, arg0 , arg1,
 				      &op0, &op1, true)))
@@ -2323,7 +2326,7 @@ predicate_mem_writes (loop_p loop)
 	      if (swap)
 		std::swap (lhs, rhs);
 	      cond = force_gimple_operand_gsi_1 (&gsi, unshare_expr (cond),
-						 is_gimple_condexpr, NULL_TREE,
+						 is_gimple_val, NULL_TREE,
 						 true, GSI_SAME_STMT);
 	      rhs = fold_build_cond_expr (type, unshare_expr (cond), rhs, lhs);
 	      gimple_assign_set_rhs1 (stmt, ifc_temp_var (type, rhs, &gsi));
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index 2efb5acae8f..95621d1f58b 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -504,9 +504,7 @@ forward_propagate_into_comparison (gimple_stmt_iterator *gsi)
 /* Propagate from the ssa name definition statements of COND_EXPR
    in GIMPLE_COND statement STMT into the conditional if that simplifies it.
    Returns zero if no statement was changed, one if there were
-   changes and two if cfg_cleanup needs to run.
-
-   This must be kept in sync with forward_propagate_into_cond.  */
+   changes and two if cfg_cleanup needs to run.  */
 
 static int
 forward_propagate_into_gimple_cond (gcond *stmt)
@@ -565,70 +563,6 @@ forward_propagate_into_gimple_cond (gcond *stmt)
   return 0;
 }
 
-
-/* Propagate from the ssa name definition statements of COND_EXPR
-   in the rhs of statement STMT into the conditional if that simplifies it.
-   Returns true zero if the stmt was changed.  */
-
-static bool
-forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
-{
-  gimple *stmt = gsi_stmt (*gsi_p);
-  tree tmp = NULL_TREE;
-  tree cond = gimple_assign_rhs1 (stmt);
-  enum tree_code code = gimple_assign_rhs_code (stmt);
-
-  /* We can do tree combining on SSA_NAME and comparison expressions.  */
-  if (COMPARISON_CLASS_P (cond))
-    tmp = forward_propagate_into_comparison_1 (stmt, TREE_CODE (cond),
-					       TREE_TYPE (cond),
-					       TREE_OPERAND (cond, 0),
-					       TREE_OPERAND (cond, 1));
-  else if (TREE_CODE (cond) == SSA_NAME)
-    {
-      enum tree_code def_code;
-      tree name = cond;
-      gimple *def_stmt = get_prop_source_stmt (name, true, NULL);
-      if (!def_stmt || !can_propagate_from (def_stmt))
-	return 0;
-
-      def_code = gimple_assign_rhs_code (def_stmt);
-      if (TREE_CODE_CLASS (def_code) == tcc_comparison)
-	tmp = fold_build2_loc (gimple_location (def_stmt),
-			       def_code,
-			       TREE_TYPE (cond),
-			       gimple_assign_rhs1 (def_stmt),
-			       gimple_assign_rhs2 (def_stmt));
-    }
-
-  if (tmp
-      && is_gimple_condexpr (tmp))
-    {
-      if (dump_file && tmp)
-	{
-	  fprintf (dump_file, "  Replaced '");
-	  print_generic_expr (dump_file, cond);
-	  fprintf (dump_file, "' with '");
-	  print_generic_expr (dump_file, tmp);
-	  fprintf (dump_file, "'\n");
-	}
-
-      if ((code == VEC_COND_EXPR) ? integer_all_onesp (tmp)
-				  : integer_onep (tmp))
-	gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs2 (stmt));
-      else if (integer_zerop (tmp))
-	gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs3 (stmt));
-      else
-	gimple_assign_set_rhs1 (stmt, unshare_expr (tmp));
-      stmt = gsi_stmt (*gsi_p);
-      update_stmt (stmt);
-
-      return true;
-    }
-
-  return 0;
-}
-
 /* We've just substituted an ADDR_EXPR into stmt.  Update all the
    relevant data structures to match.  */
 
@@ -2482,17 +2416,7 @@ pass_forwprop::execute (function *fun)
 		tree rhs1 = gimple_assign_rhs1 (stmt);
 		enum tree_code code = gimple_assign_rhs_code (stmt);
 
-		if (code == COND_EXPR
-		    || code == VEC_COND_EXPR)
-		  {
-		    /* In this case the entire COND_EXPR is in rhs1. */
-		    if (forward_propagate_into_cond (&gsi))
-		      {
-			changed = true;
-			stmt = gsi_stmt (gsi);
-		      }
-		  }
-		else if (TREE_CODE_CLASS (code) == tcc_comparison)
+		if (TREE_CODE_CLASS (code) == tcc_comparison)
 		  {
 		    int did_something;
 		    did_something = forward_propagate_into_comparison (&gsi);
diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 01a954eeb1e..df010e1087f 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -1140,8 +1140,11 @@ move_computations_worker (basic_block bb)
 	     edges of COND.  */
 	  extract_true_false_args_from_phi (dom, stmt, &arg0, &arg1);
 	  gcc_assert (arg0 && arg1);
-	  t = build2 (gimple_cond_code (cond), boolean_type_node,
-		      gimple_cond_lhs (cond), gimple_cond_rhs (cond));
+	  t = make_ssa_name (boolean_type_node);
+	  new_stmt = gimple_build_assign (t, gimple_cond_code (cond),
+					  gimple_cond_lhs (cond),
+					  gimple_cond_rhs (cond));
+	  gsi_insert_on_edge (loop_preheader_edge (level), new_stmt);
 	  new_stmt = gimple_build_assign (gimple_phi_result (stmt),
 					  COND_EXPR, t, arg0, arg1);
 	  todo |= TODO_cleanup_cfg;
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index 46502c42c74..66c6fe2237a 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -655,7 +655,9 @@ expand_vector_divmod (gimple_stmt_iterator *gsi, tree type, tree op0,
 
 	      mask_type = build_same_sized_truth_vector_type (type);
 	      zero = build_zero_cst (type);
-	      cond = build2 (LT_EXPR, mask_type, op0, zero);
+	      cond = make_ssa_name (mask_type);
+	      stmt = gimple_build_assign (cond, LT_EXPR, op0, zero);
+	      gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
 	      tree_vector_builder vec (type, nunits, 1);
 	      for (i = 0; i < nunits; i++)
 		vec.quick_push (build_int_cst (TREE_TYPE (type),
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index f99484f1da5..507c5b94f07 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -2701,8 +2701,10 @@ vect_recog_divmod_pattern (vec<gimple *> *stmts,
         dump_printf_loc (MSG_NOTE, vect_location,
                          "vect_recog_divmod_pattern: detected:\n");
 
-      cond = build2 (LT_EXPR, boolean_type_node, oprnd0,
-		     build_int_cst (itype, 0));
+      cond = make_ssa_name (boolean_type_node);
+      def_stmt = gimple_build_assign (cond, LT_EXPR, oprnd0,
+				      build_int_cst (itype, 0));
+      new_pattern_def_seq (stmt_vinfo, def_stmt);
       if (rhs_code == TRUNC_DIV_EXPR)
 	{
 	  tree var = vect_recog_temp_ssa_var (itype, NULL);
@@ -2712,7 +2714,7 @@ vect_recog_divmod_pattern (vec<gimple *> *stmts,
 				   fold_build2 (MINUS_EXPR, itype, oprnd1,
 						build_int_cst (itype, 1)),
 				   build_int_cst (itype, 0));
-	  new_pattern_def_seq (stmt_vinfo, def_stmt);
+	  append_pattern_def_seq (stmt_vinfo, def_stmt);
 	  var = vect_recog_temp_ssa_var (itype, NULL);
 	  def_stmt
 	    = gimple_build_assign (var, PLUS_EXPR, oprnd0,
@@ -2727,7 +2729,6 @@ vect_recog_divmod_pattern (vec<gimple *> *stmts,
       else
 	{
 	  tree signmask;
-	  STMT_VINFO_PATTERN_DEF_SEQ (stmt_vinfo) = NULL;
 	  if (compare_tree_int (oprnd1, 2) == 0)
 	    {
 	      signmask = vect_recog_temp_ssa_var (itype, NULL);
@@ -3483,7 +3484,9 @@ adjust_bool_pattern (tree var, tree out_type,
 	}
       else
 	itype = TREE_TYPE (rhs1);
-      cond_expr = build2_loc (loc, rhs_code, itype, rhs1, rhs2);
+      cond_expr = make_ssa_name (itype);
+      pattern_stmt = gimple_build_assign (cond_expr, rhs_code, rhs1, rhs2);
+      append_pattern_def_seq (stmt_info, pattern_stmt);
       if (trueval == NULL_TREE)
 	trueval = build_int_cst (itype, 1);
       else
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 74f813e7334..9956b453fd5 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -4160,7 +4160,9 @@ vr_values::simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
 	     in divide by zero, new_rhs1 / new_rhs will be NULL_TREE.  */
 	  if (new_rhs1 && new_rhs2)
 	    {
-	      tree cond = build2 (EQ_EXPR, boolean_type_node, cmp_var, val1);
+	      tree cond = make_ssa_name (boolean_type_node);
+	      gimple *def = gimple_build_assign (cond, EQ_EXPR, cmp_var, val1);
+	      gsi_insert_before (gsi, def, GSI_SAME_STMT);
 	      gimple_assign_set_rhs_with_ops (gsi,
 					      COND_EXPR, cond,
 					      new_rhs1,

Patch

diff --git a/gcc/gimple.c b/gcc/gimple.c
index 4b91151873c..24d7963117e 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -442,6 +442,8 @@  gimple_build_assign_1 (tree lhs, enum tree_code subcode, tree op1,
         gimple_build_with_ops_stat (GIMPLE_ASSIGN, (unsigned)subcode, num_ops
 				    PASS_MEM_STAT));
   gimple_assign_set_lhs (p, lhs);
+  gcc_assert ((subcode != COND_EXPR && subcode != VEC_COND_EXPR)
+	      || is_gimple_val (op1));
   gimple_assign_set_rhs1 (p, op1);
   if (op2)
     {
@@ -1691,6 +1693,8 @@  gimple_assign_set_rhs_with_ops (gimple_stmt_iterator *gsi, enum tree_code code,
 
   gimple_set_num_ops (stmt, new_rhs_ops + 1);
   gimple_set_subcode (stmt, code);
+  gcc_assert ((code != COND_EXPR && code != VEC_COND_EXPR)
+	      || is_gimple_val (op1));
   gimple_assign_set_rhs1 (stmt, op1);
   if (new_rhs_ops > 1)
     gimple_assign_set_rhs2 (stmt, op2);
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 44cb784620a..b679241e377 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3899,7 +3899,7 @@  gimplify_pure_cond_expr (tree *expr_p, gimple_seq *pre_p)
     TREE_SET_CODE (cond, TRUTH_AND_EXPR);
   else if (code == TRUTH_ORIF_EXPR)
     TREE_SET_CODE (cond, TRUTH_OR_EXPR);
-  ret = gimplify_expr (&cond, pre_p, NULL, is_gimple_condexpr, fb_rvalue);
+  ret = gimplify_expr (&cond, pre_p, NULL, is_gimple_val, fb_rvalue);
   COND_EXPR_COND (*expr_p) = cond;
 
   tret = gimplify_expr (&COND_EXPR_THEN (expr), pre_p, NULL,
@@ -12078,7 +12078,7 @@  gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    enum gimplify_status r0, r1, r2;
 
 	    r0 = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p,
-				post_p, is_gimple_condexpr, fb_rvalue);
+				post_p, is_gimple_val, fb_rvalue);
 	    r1 = gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p,
 				post_p, is_gimple_val, fb_rvalue);
 	    r2 = gimplify_expr (&TREE_OPERAND (*expr_p, 2), pre_p,
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 21b3fdffa59..5f6defa6fe7 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -4137,10 +4137,7 @@  verify_gimple_assign_ternary (gassign *stmt)
       return true;
     }
 
-  if (((rhs_code == VEC_COND_EXPR || rhs_code == COND_EXPR)
-       ? !is_gimple_condexpr (rhs1) : !is_gimple_val (rhs1))
-      || !is_gimple_val (rhs2)
-      || !is_gimple_val (rhs3))
+  if (!is_gimple_val (rhs1) || !is_gimple_val (rhs2) || !is_gimple_val (rhs3))
     {
       error ("invalid operands in ternary operation");
       return true;
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 71dac4fb48a..fe9eb83876f 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -1762,6 +1762,9 @@  gen_phi_arg_condition (gphi *phi, vec<int> *occur,
 	cond = c;
     }
   gcc_assert (cond != NULL_TREE);
+  cond = force_gimple_operand_gsi_1 (gsi, cond,
+				     is_gimple_val, NULL_TREE,
+				     true, GSI_SAME_STMT);
   return cond;
 }
 
@@ -1844,7 +1847,7 @@  predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
 	cond = bb_predicate (first_edge->src);
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
       cond = force_gimple_operand_gsi_1 (gsi, unshare_expr (cond),
-					 is_gimple_condexpr, NULL_TREE,
+					 is_gimple_val, NULL_TREE,
 					 true, GSI_SAME_STMT);
       true_bb = first_edge->src;
       if (EDGE_PRED (bb, 1)->src == true_bb)
@@ -1940,7 +1943,7 @@  predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
 	}
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
       cond = force_gimple_operand_gsi_1 (gsi, unshare_expr (cond),
-					 is_gimple_condexpr, NULL_TREE,
+					 is_gimple_val, NULL_TREE,
 					 true, GSI_SAME_STMT);
       if (!(is_cond_scalar_reduction (phi, &reduc, arg0 , arg1,
 				      &op0, &op1, true)))
@@ -2323,7 +2326,7 @@  predicate_mem_writes (loop_p loop)
 	      if (swap)
 		std::swap (lhs, rhs);
 	      cond = force_gimple_operand_gsi_1 (&gsi, unshare_expr (cond),
-						 is_gimple_condexpr, NULL_TREE,
+						 is_gimple_val, NULL_TREE,
 						 true, GSI_SAME_STMT);
 	      rhs = fold_build_cond_expr (type, unshare_expr (cond), rhs, lhs);
 	      gimple_assign_set_rhs1 (stmt, ifc_temp_var (type, rhs, &gsi));
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index 2efb5acae8f..95621d1f58b 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -504,9 +504,7 @@  forward_propagate_into_comparison (gimple_stmt_iterator *gsi)
 /* Propagate from the ssa name definition statements of COND_EXPR
    in GIMPLE_COND statement STMT into the conditional if that simplifies it.
    Returns zero if no statement was changed, one if there were
-   changes and two if cfg_cleanup needs to run.
-
-   This must be kept in sync with forward_propagate_into_cond.  */
+   changes and two if cfg_cleanup needs to run.  */
 
 static int
 forward_propagate_into_gimple_cond (gcond *stmt)
@@ -565,70 +563,6 @@  forward_propagate_into_gimple_cond (gcond *stmt)
   return 0;
 }
 
-
-/* Propagate from the ssa name definition statements of COND_EXPR
-   in the rhs of statement STMT into the conditional if that simplifies it.
-   Returns true zero if the stmt was changed.  */
-
-static bool
-forward_propagate_into_cond (gimple_stmt_iterator *gsi_p)
-{
-  gimple *stmt = gsi_stmt (*gsi_p);
-  tree tmp = NULL_TREE;
-  tree cond = gimple_assign_rhs1 (stmt);
-  enum tree_code code = gimple_assign_rhs_code (stmt);
-
-  /* We can do tree combining on SSA_NAME and comparison expressions.  */
-  if (COMPARISON_CLASS_P (cond))
-    tmp = forward_propagate_into_comparison_1 (stmt, TREE_CODE (cond),
-					       TREE_TYPE (cond),
-					       TREE_OPERAND (cond, 0),
-					       TREE_OPERAND (cond, 1));
-  else if (TREE_CODE (cond) == SSA_NAME)
-    {
-      enum tree_code def_code;
-      tree name = cond;
-      gimple *def_stmt = get_prop_source_stmt (name, true, NULL);
-      if (!def_stmt || !can_propagate_from (def_stmt))
-	return 0;
-
-      def_code = gimple_assign_rhs_code (def_stmt);
-      if (TREE_CODE_CLASS (def_code) == tcc_comparison)
-	tmp = fold_build2_loc (gimple_location (def_stmt),
-			       def_code,
-			       TREE_TYPE (cond),
-			       gimple_assign_rhs1 (def_stmt),
-			       gimple_assign_rhs2 (def_stmt));
-    }
-
-  if (tmp
-      && is_gimple_condexpr (tmp))
-    {
-      if (dump_file && tmp)
-	{
-	  fprintf (dump_file, "  Replaced '");
-	  print_generic_expr (dump_file, cond);
-	  fprintf (dump_file, "' with '");
-	  print_generic_expr (dump_file, tmp);
-	  fprintf (dump_file, "'\n");
-	}
-
-      if ((code == VEC_COND_EXPR) ? integer_all_onesp (tmp)
-				  : integer_onep (tmp))
-	gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs2 (stmt));
-      else if (integer_zerop (tmp))
-	gimple_assign_set_rhs_from_tree (gsi_p, gimple_assign_rhs3 (stmt));
-      else
-	gimple_assign_set_rhs1 (stmt, unshare_expr (tmp));
-      stmt = gsi_stmt (*gsi_p);
-      update_stmt (stmt);
-
-      return true;
-    }
-
-  return 0;
-}
-
 /* We've just substituted an ADDR_EXPR into stmt.  Update all the
    relevant data structures to match.  */
 
@@ -2482,17 +2416,7 @@  pass_forwprop::execute (function *fun)
 		tree rhs1 = gimple_assign_rhs1 (stmt);
 		enum tree_code code = gimple_assign_rhs_code (stmt);
 
-		if (code == COND_EXPR
-		    || code == VEC_COND_EXPR)
-		  {
-		    /* In this case the entire COND_EXPR is in rhs1. */
-		    if (forward_propagate_into_cond (&gsi))
-		      {
-			changed = true;
-			stmt = gsi_stmt (gsi);
-		      }
-		  }
-		else if (TREE_CODE_CLASS (code) == tcc_comparison)
+		if (TREE_CODE_CLASS (code) == tcc_comparison)
 		  {
 		    int did_something;
 		    did_something = forward_propagate_into_comparison (&gsi);
diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 01a954eeb1e..df010e1087f 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -1140,8 +1140,11 @@  move_computations_worker (basic_block bb)
 	     edges of COND.  */
 	  extract_true_false_args_from_phi (dom, stmt, &arg0, &arg1);
 	  gcc_assert (arg0 && arg1);
-	  t = build2 (gimple_cond_code (cond), boolean_type_node,
-		      gimple_cond_lhs (cond), gimple_cond_rhs (cond));
+	  t = make_ssa_name (boolean_type_node);
+	  new_stmt = gimple_build_assign (t, gimple_cond_code (cond),
+					  gimple_cond_lhs (cond),
+					  gimple_cond_rhs (cond));
+	  gsi_insert_on_edge (loop_preheader_edge (level), new_stmt);
 	  new_stmt = gimple_build_assign (gimple_phi_result (stmt),
 					  COND_EXPR, t, arg0, arg1);
 	  todo |= TODO_cleanup_cfg;
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index f99484f1da5..1418c83690b 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -2701,8 +2701,10 @@  vect_recog_divmod_pattern (vec<gimple *> *stmts,
         dump_printf_loc (MSG_NOTE, vect_location,
                          "vect_recog_divmod_pattern: detected:\n");
 
-      cond = build2 (LT_EXPR, boolean_type_node, oprnd0,
-		     build_int_cst (itype, 0));
+      cond = make_ssa_name (boolean_type_node);
+      def_stmt = gimple_build_assign (cond, LT_EXPR, oprnd0,
+				      build_int_cst (itype, 0));
+      new_pattern_def_seq (stmt_vinfo, def_stmt);
       if (rhs_code == TRUNC_DIV_EXPR)
 	{
 	  tree var = vect_recog_temp_ssa_var (itype, NULL);
@@ -2712,7 +2714,7 @@  vect_recog_divmod_pattern (vec<gimple *> *stmts,
 				   fold_build2 (MINUS_EXPR, itype, oprnd1,
 						build_int_cst (itype, 1)),
 				   build_int_cst (itype, 0));
-	  new_pattern_def_seq (stmt_vinfo, def_stmt);
+	  append_pattern_def_seq (stmt_vinfo, def_stmt);
 	  var = vect_recog_temp_ssa_var (itype, NULL);
 	  def_stmt
 	    = gimple_build_assign (var, PLUS_EXPR, oprnd0,
@@ -2727,7 +2729,6 @@  vect_recog_divmod_pattern (vec<gimple *> *stmts,
       else
 	{
 	  tree signmask;
-	  STMT_VINFO_PATTERN_DEF_SEQ (stmt_vinfo) = NULL;
 	  if (compare_tree_int (oprnd1, 2) == 0)
 	    {
 	      signmask = vect_recog_temp_ssa_var (itype, NULL);
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 74f813e7334..9956b453fd5 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -4160,7 +4160,9 @@  vr_values::simplify_stmt_using_ranges (gimple_stmt_iterator *gsi)
 	     in divide by zero, new_rhs1 / new_rhs will be NULL_TREE.  */
 	  if (new_rhs1 && new_rhs2)
 	    {
-	      tree cond = build2 (EQ_EXPR, boolean_type_node, cmp_var, val1);
+	      tree cond = make_ssa_name (boolean_type_node);
+	      gimple *def = gimple_build_assign (cond, EQ_EXPR, cmp_var, val1);
+	      gsi_insert_before (gsi, def, GSI_SAME_STMT);
 	      gimple_assign_set_rhs_with_ops (gsi,
 					      COND_EXPR, cond,
 					      new_rhs1,