PHIOPT: move factor_out_conditional_operation over to use gimple_match_op

To start working on more with expressions with more than one operand, converting
over to use gimple_match_op is needed.
The added side-effect here is factor_out_conditional_operation can now support
builtins/internal calls that has one operand without any extra code added.

Note on the changed testcases:
* pr87007-5.c: the test was testing testing for avoiding partial register stalls
for the sqrt and making sure there is only one zero of the register before the
branch, the phiopt would now merge the sqrt's so disable phiopt.

Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* gimple-match-exports.cc (gimple_match_op::operands_occurs_in_abnormal_phi):
	New function.
	* gimple-match.h (gimple_match_op): Add operands_occurs_in_abnormal_phi.
	* tree-ssa-phiopt.cc (factor_out_conditional_operation): Use gimple_match_op
	instead of manually extracting from/creating the gimple.

gcc/testsuite/ChangeLog:

	* gcc.target/i386/pr87007-5.c: Disable phi-opt.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

diff --git a/gcc/gimple-match-exports.cc b/gcc/gimple-match-exports.cc
index aacf3ff04145662dc3a1352eceb0c8dc87fdab06..15d54b7d843835fffbd7d15515d3950ea0b846f6 100644 (file)
--- a/gcc/gimple-match-exports.cc
+++ b/gcc/gimple-match-exports.cc
@@ -126,6 +126,20 @@ gimple_match_op::resimplify (gimple_seq *seq, tree (*valueize)(tree))
     }
 }
 
+/* Returns true if any of the operands of THIS occurs
+   in abnormal phis. */
+bool
+gimple_match_op::operands_occurs_in_abnormal_phi() const
+{
+  for (unsigned int i = 0; i < num_ops; i++)
+    {
+       if (TREE_CODE (ops[i]) == SSA_NAME
+          && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[i]))
+       return true;
+    }
+  return false;
+}
+
 /* Return whether T is a constant that we'll dispatch to fold to
    evaluate fully constant expressions.  */
 

diff --git a/gcc/gimple-match.h b/gcc/gimple-match.h
index d710fcbace28e0bc29ca2bbbc6f05fae8c8cdc8a..8edff578ba9a14bfd529bb3ce8928612efa481ac 100644 (file)
--- a/gcc/gimple-match.h
+++ b/gcc/gimple-match.h
@@ -136,6 +136,8 @@ public:
 
   /* The operands to CODE.  Only the first NUM_OPS entries are meaningful.  */
   tree ops[MAX_NUM_OPS];
+
+  bool operands_occurs_in_abnormal_phi() const;
 };
 
 inline

diff --git a/gcc/testsuite/gcc.target/i386/pr87007-5.c b/gcc/testsuite/gcc.target/i386/pr87007-5.c
index 8f2dc947f6c4524c48aecd076e86b136ab785c7e..c696827df127acef05b0eacd201eefae9721b5a0 100644 (file)
--- a/gcc/testsuite/gcc.target/i386/pr87007-5.c
+++ b/gcc/testsuite/gcc.target/i386/pr87007-5.c
@@ -1,8 +1,11 @@
 /* { dg-do compile } */
-/* { dg-options "-Ofast -march=skylake-avx512 -mfpmath=sse -fno-tree-vectorize -fdump-tree-cddce3-details -fdump-tree-lsplit-optimized" } */
+/* { dg-options "-Ofast -march=skylake-avx512 -mfpmath=sse -fno-tree-vectorize -fdump-tree-cddce3-details -fdump-tree-lsplit-optimized -fno-ssa-phiopt" } */
 /* Load of d2/d3 is hoisted out, the loop is split, store of d1 and sqrt
    are sunk out of the loop and the loop is elided.  One vsqrtsd with
    memory operand needs a xor to avoid partial dependence.  */
+/* Phi-OPT needs to be disabled otherwise, sqrt calls are merged which is better
+   but we are testing to make sure the partial register stall for SSE is still avoided
+   for sqrts.  */
 
 #include<math.h>
 

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index aacccc414f6b8cec83fbd4b63fc9b5d6c1b0b85e..2d4aba5b087294fad64deab7b1b39ae64fb52e2e 100644 (file)
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -220,13 +220,12 @@ static gphi *
 factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
                                   tree arg0, tree arg1, gimple *cond_stmt)
 {
-  gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL, *new_stmt;
-  tree new_arg0 = NULL_TREE, new_arg1 = NULL_TREE;
+  gimple *arg0_def_stmt = NULL, *arg1_def_stmt = NULL;
   tree temp, result;
   gphi *newphi;
   gimple_stmt_iterator gsi, gsi_for_def;
   location_t locus = gimple_location (phi);
-  enum tree_code op_code;
+  gimple_match_op arg0_op, arg1_op;
 
   /* Handle only PHI statements with two arguments.  TODO: If all
      other arguments to PHI are INTEGER_CST or if their defining
@@ -250,31 +249,31 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
   /* Check if arg0 is an SSA_NAME and the stmt which defines arg0 is
      an unary operation.  */
   arg0_def_stmt = SSA_NAME_DEF_STMT (arg0);
-  if (!is_gimple_assign (arg0_def_stmt)
-      || (gimple_assign_rhs_class (arg0_def_stmt) != GIMPLE_UNARY_RHS
-         && gimple_assign_rhs_code (arg0_def_stmt) != VIEW_CONVERT_EXPR))
+  if (!gimple_extract_op (arg0_def_stmt, &arg0_op))
     return NULL;
 
-  /* Use the RHS as new_arg0.  */
-  op_code = gimple_assign_rhs_code (arg0_def_stmt);
-  new_arg0 = gimple_assign_rhs1 (arg0_def_stmt);
-  if (op_code == VIEW_CONVERT_EXPR)
-    {
-      new_arg0 = TREE_OPERAND (new_arg0, 0);
-      if (!is_gimple_reg_type (TREE_TYPE (new_arg0)))
-       return NULL;
-    }
-  if (TREE_CODE (new_arg0) == SSA_NAME
-      && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new_arg0))
+  /* Check to make sure none of the operands are in abnormal phis.  */
+  if (arg0_op.operands_occurs_in_abnormal_phi ())
+   return NULL;
+
+  /* Currently just support one operand expressions. */
+  if (arg0_op.num_ops != 1)
     return NULL;
 
+  tree new_arg0 = arg0_op.ops[0];
+  tree new_arg1;
+
   if (TREE_CODE (arg1) == SSA_NAME)
     {
-      /* Check if arg1 is an SSA_NAME and the stmt which defines arg1
-        is an unary operation.  */
+      /* Check if arg1 is an SSA_NAME.  */
       arg1_def_stmt = SSA_NAME_DEF_STMT (arg1);
-       if (!is_gimple_assign (arg1_def_stmt)
-          || gimple_assign_rhs_code (arg1_def_stmt) != op_code)
+      if (!gimple_extract_op (arg1_def_stmt, &arg1_op))
+       return NULL;
+      if (arg1_op.code != arg0_op.code)
+       return NULL;
+      if (arg1_op.num_ops != arg0_op.num_ops)
+       return NULL;
+      if (arg1_op.operands_occurs_in_abnormal_phi ())
        return NULL;
 
       /* Either arg1_def_stmt or arg0_def_stmt should be conditional.  */
@@ -282,14 +281,7 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
          && dominated_by_p (CDI_DOMINATORS,
                             gimple_bb (phi), gimple_bb (arg1_def_stmt)))
        return NULL;
-
-      /* Use the RHS as new_arg1.  */
-      new_arg1 = gimple_assign_rhs1 (arg1_def_stmt);
-      if (op_code == VIEW_CONVERT_EXPR)
-       new_arg1 = TREE_OPERAND (new_arg1, 0);
-      if (TREE_CODE (new_arg1) == SSA_NAME
-         && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (new_arg1))
-       return NULL;
+      new_arg1 = arg1_op.ops[0];
     }
   else
     {
@@ -300,6 +292,7 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
       /* arg0_def_stmt should be conditional.  */
       if (dominated_by_p (CDI_DOMINATORS, gimple_bb (phi), gimple_bb (arg0_def_stmt)))
        return NULL;
+
       /* If arg1 is an INTEGER_CST, fold it to new type.  */
       if (INTEGRAL_TYPE_P (TREE_TYPE (new_arg0))
          && (int_fits_type_p (arg1, TREE_TYPE (new_arg0))
@@ -405,16 +398,15 @@ factor_out_conditional_operation (edge e0, edge e1, gphi *phi,
   add_phi_arg (newphi, new_arg0, e0, locus);
   add_phi_arg (newphi, new_arg1, e1, locus);
 
+  gimple_match_op new_op = arg0_op;
+
   /* Create the operation stmt and insert it.  */
-  if (op_code == VIEW_CONVERT_EXPR)
-    {
-      temp = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (result), temp);
-      new_stmt = gimple_build_assign (result, temp);
-    }
-  else
-    new_stmt = gimple_build_assign (result, op_code, temp);
+  new_op.ops[0] = temp;
+  gimple_seq seq = NULL;
+  result = maybe_push_res_to_seq (&new_op, &seq, result);
+  gcc_assert (result);
   gsi = gsi_after_labels (gimple_bb (phi));
-  gsi_insert_before (&gsi, new_stmt, GSI_SAME_STMT);
+  gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING);
 
   /* Remove the original PHI stmt.  */
   gsi = gsi_for_stmt (phi);