Re: [PATCH][RFC] Drop GENERIC conds in [VEC_]COND_EXPRs

[Richard Biener <rguenther at suse dot de>]

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,