ifcvt: Reduce comparisons on conditionals by tracking truths [PR109154]

Following on from Jakub's patch in g:de0ee9d14165eebb3d31c84e98260c05c3b33acb
these two patches finishes the work fixing the regression and improves codegen.

As explained in that commit, ifconvert sorts PHI args in increasing number of
occurrences in order to reduce the number of comparisons done while
traversing the tree.

The remaining task that this patch fixes is dealing with the long chain of
comparisons that can be created from phi nodes, particularly when they share
any common successor (classical example is a diamond node).

on a PHI-node the true and else branches carry a condition, true will
carry `a` and false `~a`.  The issue is that at the moment GCC tests both `a`
and `~a` when the phi node has more than 2 arguments. Clearly this isn't
needed.  The deeper the nesting of phi nodes the larger the repetition.

As an example, for

foo (int *f, int d, int e)
{
  for (int i = 0; i < 1024; i++)
    {
      int a = f[i];
      int t;
      if (a < 0)
	t = 1;
      else if (a < e)
	t = 1 - a * d;
      else
	t = 0;
      f[i] = t;
    }
}

after Jakub's patch we generate:

  _7 = a_10 < 0;
  _21 = a_10 >= 0;
  _22 = a_10 < e_11(D);
  _23 = _21 & _22;
  _ifc__42 = _23 ? t_13 : 0;
  t_6 = _7 ? 1 : _ifc__42

but while better than before it is still inefficient, since in the false
branch, where we know ~_7 is true, we still test _21.

This leads to superfluous tests for every diamond node.  After this patch we
generate

 _7 = a_10 < 0;
 _22 = a_10 < e_11(D);
 _ifc__42 = _22 ? t_13 : 0;
 t_6 = _7 ? 1 : _ifc__42;

Which correctly elides the test of _21.  This is done by borrowing the
vectorizer's helper functions to limit predicate mask usages.  Ifcvt will chain
conditionals on the false edge (unless specifically inverted) so this patch on
creating cond a ? b : c, will register ~a when traversing c.  If c is a
conditional then c will be simplified to the smaller possible predicate given
the assumptions we already know to be true.

gcc/ChangeLog:

	PR tree-optimization/109154
	* tree-if-conv.cc (gen_simplified_condition,
	gen_phi_nest_statement): New.
	(gen_phi_arg_condition, predicate_scalar_phi): Use it.

gcc/testsuite/ChangeLog:

	PR tree-optimization/109154
	* gcc.dg/vect/vect-ifcvt-19.c: New test.

diff --git a/gcc/testsuite/gcc.dg/vect/vect-ifcvt-19.c b/gcc/testsuite/gcc.dg/vect/vect-ifcvt-19.c
new file mode 100644 (file)
index 0000000..e34bfa9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/vect-ifcvt-19.c
@@ -0,0 +1,63 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -Ofast -fdump-tree-ifcvt-raw" } */
+
+void __GIMPLE (ssa,guessed_local(10737414), startwith ("fix_loops"))
+foo (int * f, int d, int e)
+{
+  int t;
+  int a;
+  int i;
+  long unsigned int _1;
+  long unsigned int _2;
+  int * _3;
+  int _4;
+
+  __BB(2,guessed_local(10737414)):
+  goto __BB3(guessed(134217728));
+
+  __BB(3,loop_header(1),guessed_local(1063004408)):
+  i_18 = __PHI (__BB8: i_15, __BB2: 0);
+  _1 = (long unsigned int) i_18;
+  _2 = _1 * 4ul;
+  _3 = f_9(D) + _2;
+  a_10 = __MEM <int> (_3);
+  if (a_10 < 0)
+    goto __BB9(guessed(55029268));
+  else
+    goto __BB4(guessed(79188460));
+
+  __BB(9,guessed_local(435831803)):
+  goto __BB6(precise(134217728));
+
+  __BB(4,guessed_local(627172605)):
+  if (a_10 < e_11(D))
+    goto __BB5(guessed(67108864));
+  else
+    goto __BB10(guessed(67108864));
+
+  __BB(10,guessed_local(313586303)):
+  goto __BB6(precise(134217728));
+
+  __BB(5,guessed_local(313586302)):
+  _4 = a_10 * d_12(D);
+  t_13 = 1 - _4;
+  goto __BB6(precise(134217728));
+
+  __BB(6,guessed_local(1063004410)):
+  t_6 = __PHI (__BB9: 1, __BB5: t_13, __BB10: 0);
+  __MEM <int> (_3) = t_6;
+  i_15 = i_18 + 1;
+  if (i_15 != 1024)
+    goto __BB8(guessed(132875551));
+  else
+    goto __BB7(guessed(1342177));
+
+  __BB(8,guessed_local(1052374368)):
+  goto __BB3(precise(134217728));
+
+  __BB(7,guessed_local(10737416)):
+  return;
+
+}
+
+/* { dg-final { scan-tree-dump-times {<cond_expr,} 2 ifcvt { target vect_int } } } */

diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
index e342532a343a3c066142adeec5fdfaf736a653e5..4070aca6d54e14849e111a57364cfcdeb8c0a2bc 100644 (file)
--- a/gcc/tree-if-conv.cc
+++ b/gcc/tree-if-conv.cc
@@ -1870,12 +1870,44 @@ convert_scalar_cond_reduction (gimple *reduc, gimple_stmt_iterator *gsi,
   return rhs;
 }
 
+/* Generate a simplified conditional.  */
+
+static tree
+gen_simplified_condition (tree cond, scalar_cond_masked_set_type &cond_set)
+{
+  /* Check if the value is already live in a previous branch.  This resolves
+     nested conditionals from diamond PHI reductions.  */
+  if (TREE_CODE (cond) == SSA_NAME)
+    {
+      gimple *stmt = SSA_NAME_DEF_STMT (cond);
+      gassign *assign = NULL;
+      if ((assign = as_a <gassign *> (stmt))
+          && gimple_assign_rhs_code (assign) == BIT_AND_EXPR)
+       {
+         tree arg1 = gimple_assign_rhs1 (assign);
+         tree arg2 = gimple_assign_rhs2 (assign);
+         if (cond_set.contains ({ arg1, 1 }))
+           arg1 = boolean_true_node;
+         else
+           arg1 = gen_simplified_condition (arg1, cond_set);
+
+         if (cond_set.contains ({ arg2, 1 }))
+           arg2 = boolean_true_node;
+         else
+           arg2 = gen_simplified_condition (arg2, cond_set);
+
+         cond = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, arg1, arg2);
+       }
+    }
+  return cond;
+}
+
 /* Produce condition for all occurrences of ARG in PHI node.  Set *INVERT
    as to whether the condition is inverted.  */
 
 static tree
-gen_phi_arg_condition (gphi *phi, vec<int> *occur,
-                      gimple_stmt_iterator *gsi, bool *invert)
+gen_phi_arg_condition (gphi *phi, vec<int> *occur, gimple_stmt_iterator *gsi,
+                      scalar_cond_masked_set_type &cond_set, bool *invert)
 {
   int len;
   int i;
@@ -1902,6 +1934,8 @@ gen_phi_arg_condition (gphi *phi, vec<int> *occur,
          c = TREE_OPERAND (c, 0);
          *invert = true;
        }
+
+      c = gen_simplified_condition (c, cond_set);
       c = force_gimple_operand_gsi (gsi, unshare_expr (c),
                                    true, NULL_TREE, true, GSI_SAME_STMT);
       if (cond != NULL_TREE)
@@ -1913,11 +1947,79 @@ gen_phi_arg_condition (gphi *phi, vec<int> *occur,
        }
       else
        cond = c;
+
+      /* Register the new possibly simplified conditional.  When more than 2
+        entries in a phi node we chain entries in the false branch, so the
+        inverted condition is active.  */
+      scalar_cond_masked_key pred_cond ({ cond, 1 });
+      if (!*invert)
+       pred_cond.inverted_p = !pred_cond.inverted_p;
+      cond_set.add (pred_cond);
     }
   gcc_assert (cond != NULL_TREE);
   return cond;
 }
 
+/* Create the smallest nested conditional possible.  On pre-order we record
+   which conditionals are live, and on post-order rewrite the chain by removing
+   already active conditions.
+
+   As an example we simplify:
+
+  _7 = a_10 < 0;
+  _21 = a_10 >= 0;
+  _22 = a_10 < e_11(D);
+  _23 = _21 & _22;
+  _ifc__42 = _23 ? t_13 : 0;
+  t_6 = _7 ? 1 : _ifc__42
+
+  into
+
+  _7 = a_10 < 0;
+  _22 = a_10 < e_11(D);
+  _ifc__42 = _22 ? t_13 : 0;
+  t_6 = _7 ? 1 : _ifc__42;
+
+  which produces better code.  */
+
+static tree
+gen_phi_nest_statement (gphi *phi, gimple_stmt_iterator *gsi,
+                       scalar_cond_masked_set_type &cond_set, tree type,
+                       hash_map<tree_operand_hash, auto_vec<int>> &phi_arg_map,
+                       gimple **res_stmt, tree lhs0, vec<tree> &args,
+                       unsigned idx)
+{
+  if (idx == args.length ())
+    return args[idx - 1];
+
+  vec<int> *indexes = phi_arg_map.get (args[idx - 1]);
+  bool invert;
+  tree cond = gen_phi_arg_condition (phi, indexes, gsi, cond_set, &invert);
+  tree arg1 = gen_phi_nest_statement (phi, gsi, cond_set, type, phi_arg_map,
+                                     res_stmt, lhs0, args, idx + 1);
+
+  unsigned prev = idx;
+  unsigned curr = prev - 1;
+  tree arg0 = args[curr];
+  tree rhs, lhs;
+  if (idx > 1)
+    lhs = make_temp_ssa_name (type, NULL, "_ifc_");
+  else
+    lhs = lhs0;
+
+  if (invert)
+    rhs = fold_build_cond_expr (type, unshare_expr (cond),
+                               arg1, arg0);
+  else
+    rhs = fold_build_cond_expr (type, unshare_expr (cond),
+                               arg0, arg1);
+  gassign *new_stmt = gimple_build_assign (lhs, rhs);
+  gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
+  update_stmt (new_stmt);
+  *res_stmt = new_stmt;
+  return lhs;
+}
+
 /* Replace a scalar PHI node with a COND_EXPR using COND as condition.
    This routine can handle PHI nodes with more than two arguments.
 
@@ -1968,6 +2070,8 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
     }
 
   bb = gimple_bb (phi);
+  /* Keep track of conditionals already seen.  */
+  scalar_cond_masked_set_type cond_set;
   if (EDGE_COUNT (bb->preds) == 2)
     {
       /* Predicate ordinary PHI node with 2 arguments.  */
@@ -1976,6 +2080,7 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
       first_edge = EDGE_PRED (bb, 0);
       second_edge = EDGE_PRED (bb, 1);
       cond = bb_predicate (first_edge->src);
+      cond_set.add ({ cond, 1 });
       if (TREE_CODE (cond) == TRUTH_NOT_EXPR)
        std::swap (first_edge, second_edge);
       if (EDGE_COUNT (first_edge->src->succs) > 1)
@@ -1988,7 +2093,9 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
        }
       else
        cond = bb_predicate (first_edge->src);
+
       /* Gimplify the condition to a valid cond-expr conditonal operand.  */
+      cond = gen_simplified_condition (cond, cond_set);
       cond = force_gimple_operand_gsi (gsi, unshare_expr (cond), true,
                                       NULL_TREE, true, GSI_SAME_STMT);
       true_bb = first_edge->src;
@@ -2110,31 +2217,9 @@ predicate_scalar_phi (gphi *phi, gimple_stmt_iterator *gsi)
   else
     {
       /* Common case.  */
-      vec<int> *indexes;
       tree type = TREE_TYPE (gimple_phi_result (phi));
-      tree lhs;
-      arg1 = args[args_len - 1];
-      for (i = args_len - 1; i > 0; i--)
-       {
-         arg0 = args[i - 1];
-         indexes = phi_arg_map.get (args[i - 1]);
-         if (i != 1)
-           lhs = make_temp_ssa_name (type, NULL, "_ifc_");
-         else
-           lhs = res;
-         bool invert;
-         cond = gen_phi_arg_condition (phi, indexes, gsi, &invert);
-         if (invert)
-           rhs = fold_build_cond_expr (type, unshare_expr (cond),
-                                       arg1, arg0);
-         else
-           rhs = fold_build_cond_expr (type, unshare_expr (cond),
-                                       arg0, arg1);
-         new_stmt = gimple_build_assign (lhs, rhs);
-         gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
-         update_stmt (new_stmt);
-         arg1 = lhs;
-       }
+      gen_phi_nest_statement (phi, gsi, cond_set, type, phi_arg_map,
+                             &new_stmt, res, args, 1);
     }
 
   if (dump_file && (dump_flags & TDF_DETAILS))