diff mbox

Try to expand COND_EXPR using addcc

Message ID CA+=Sn1=+Ew_eegQ=Dw4akc6r=5Vr9wyhFLFm9Xowebs8ZMcf1g@mail.gmail.com
State New
Headers show

Commit Message

Andrew Pinski May 16, 2012, 7:48 p.m. UTC 
Hi,
  This is just like my previous patch to expand COND_EXPR using
conditional moves but this time using addcc instead.
I had to fix a bug in emit_conditional_add where it was swapping op2
and op3 which can never happen.  Also the documentation for both
emit_conditional_add and add@var{mode}cc was in correct in saying the
first (non comparison) operand was the result when the condition was
true, it should have been when it was false.  So it does op2 if cond
is false and op2+op3 when the cond is true.  This actually makes
better sense than what the documentation was before too.

OK? Bootstrapped and tested on x86_64-linux-gnu?

Thanks,
Andrew Pinski

ChangeLog:
 * optabs.c (emit_conditional_add): Correct comment about the arguments.
Remove code which might swap op2 and op3 since they cannot be swapped.
* expr.c (get_condition_from_operand): New function.
(get_def_noter_for_expr_with_code): New function.
(expand_cond_expr_using_addcc): New function.
(expand_cond_expr_using_cmove): Use get_condition_from_operand instead
of doing it inline.
(expand_expr_real_2 <case COND_EXPR>): Call
expand_cond_expr_using_addcc before trying conditional moves.
* doc/md.texi (add@var{mode}cc): Fix document about how the arguments are used.

Comments

Richard Henderson May 22, 2012, 8:32 p.m. UTC | #1 
On 05/16/2012 12:48 PM, Andrew Pinski wrote:
> +  /* Handle 0/1 specially because boolean types and precision of one types,
> +     will cause the diff to always be 1.  Note this really should have
> +     simplified before reaching here.  */
> +  /* A ? 1 : 0 is just (type)(A!=0). */
> +  if (integer_zerop (treeop2)&&  integer_onep (treeop1))
> +    {
> +      tree t = fold_convert (TREE_TYPE (treeop0), integer_zero_node);
> +      tree tmp = fold_build2 (NE_EXPR, TREE_TYPE (treeop0), treeop0, t);
> +      tmp = fold_convert (type, tmp);
> +      return expand_normal (tmp);
> +    }
> +
> +  /* A ? 0 : 1 is just (type)(A==0). */
> +  if (integer_zerop (treeop1)&&  integer_onep (treeop0))
> +    {
> +      tree t = fold_convert (TREE_TYPE (treeop0), integer_zero_node);
> +      tree tmp = fold_build2 (EQ_EXPR, TREE_TYPE (treeop0), treeop0, t);
> +      tmp = fold_convert (type, tmp);
> +      return expand_normal (tmp);
> +    }

Well, why *don't* you handle them before here?  I completely agree that
they seem out of place in an _addcc function.

> +  /* A ? CST1 : CST2 can be expanded as CST2 + (!A)*(CST1 - CST2) */
> +  if (TREE_CODE (treeop1) == INTEGER_CST
> +&&  TREE_CODE (treeop2) == INTEGER_CST)
> +    diff = int_const_binop (MINUS_EXPR, treeop1, treeop2);

Here you're bypassing the large amount of logic we've got in
e.g. ix86_expand_int_movcc for handling exactly this case, and
in more clever ways than you're doing here.

Please continue to defer to cmov for this.

> +  /* A ? b : b+c can be expanded as b + (!A)*(c) */
> +  /* A ? b + c : b can be expanded as b + (!A)*(c) */

What has MULT got to do with it?  I think these comments are just confusing.


r~
diff mbox

Patch

Index: optabs.c
===================================================================
--- optabs.c	(revision 187579)
+++ optabs.c	(working copy)
@@ -4565,7 +4565,7 @@  can_conditionally_move_p (enum machine_m
    the mode to use should they be constants.  If it is VOIDmode, they cannot
    both be constants.
 
-   OP2 should be stored in TARGET if the comparison is true, otherwise OP2+OP3
+   OP2 should be stored in TARGET if the comparison is false, otherwise OP2+OP3
    should be stored there.  MODE is the mode to use should they be constants.
    If it is VOIDmode, they cannot both be constants.
 
@@ -4579,7 +4579,6 @@  emit_conditional_add (rtx target, enum r
 {
   rtx tem, comparison, last;
   enum insn_code icode;
-  enum rtx_code reversed;
 
   /* If one operand is constant, make it the second one.  Only do this
      if the other operand is not constant as well.  */
@@ -4603,16 +4602,6 @@  emit_conditional_add (rtx target, enum r
   if (cmode == VOIDmode)
     cmode = GET_MODE (op0);
 
-  if (swap_commutative_operands_p (op2, op3)
-      && ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL))
-          != UNKNOWN))
-    {
-      tem = op2;
-      op2 = op3;
-      op3 = tem;
-      code = reversed;
-    }
-
   if (mode == VOIDmode)
     mode = GET_MODE (op2);
 
Index: expr.c
===================================================================
--- expr.c	(revision 187579)
+++ expr.c	(working copy)
@@ -7867,6 +7867,181 @@  expand_expr_real (tree exp, rtx target,
   return ret;
 }
 
+/* Try to get the condition expression from the OP operand.  Setting the OP0
+   to the lhs and OP1 to the rhs and CODE to the code of the condition. */
+
+static void
+get_condition_from_operand (tree op, tree *op0, tree *op1,
+                            enum tree_code *code)
+{
+  gimple srcstmt;
+
+  if (TREE_CODE (op) == SSA_NAME
+      && (srcstmt = get_def_for_expr_class (op, tcc_comparison)))
+    {
+      *op0 = gimple_assign_rhs1 (srcstmt);
+      *op1 = gimple_assign_rhs2 (srcstmt);
+      *code = gimple_assign_rhs_code (srcstmt);
+    }
+  else if (TREE_CODE (op) == SSA_NAME
+      && (srcstmt = get_def_for_expr (op, BIT_NOT_EXPR)))
+    {
+      *op0 = gimple_assign_rhs1 (srcstmt);
+      *op1 = fold_convert (TREE_TYPE (op), integer_zero_node);
+      *code = EQ_EXPR;
+    }
+  else if (TREE_CODE_CLASS (TREE_CODE (op)) == tcc_comparison)
+    {
+      *op0 = TREE_OPERAND (op, 0);
+      *op1 = TREE_OPERAND (op, 1);
+      *code = TREE_CODE (op);
+    }
+  /* If it is neither, just assume OP != 0. */
+  else
+    {
+      *op0 = op;
+      *op1 = fold_convert (TREE_TYPE (op), integer_zero_node);
+      *code = NE_EXPR;
+    }
+}
+
+/* Given a ssa_name in NAME see if it was defined by an assignment and
+   set CODE to be the code and ARG1 to the first operand on the rhs and ARG2
+   to the second operand on the rhs. */
+
+static gimple
+get_def_noter_for_expr_with_code (tree name, enum tree_code code)
+{
+  gimple def;
+
+  if (TREE_CODE (name) == SSA_NAME)
+    {
+      def = SSA_NAME_DEF_STMT (name);
+
+      if (def && is_gimple_assign (def)
+          && gimple_assign_rhs_code (def) == code)
+        return def;
+    }
+  return NULL;
+}
+
+/* Try to expand the conditional expression which is represented by
+   TREEOP0 ? TREEOP1 : TREEOP2 using conditonal adds.  If succeseds
+   return the rtl reg which represents the result.  Otherwise return
+   NULL_RTL.  */
+
+static rtx
+expand_cond_expr_using_addcc (tree treeop0, tree treeop1, tree treeop2)
+{
+  tree diff;
+  rtx op1, op2, op00, op01, temp;
+  tree cmpop0, cmpop1, type;
+  enum tree_code cmpcode;
+  enum rtx_code comparison_code;
+  enum machine_mode comparison_mode, mode;
+  rtx seq;
+  int unsignedp;
+  gimple srcstmt;
+
+  type = TREE_TYPE (treeop1);
+  mode = TYPE_MODE (type);
+
+  if (!INTEGRAL_TYPE_P (type))
+    return NULL_RTX;
+
+  /* Handle 0/1 specially because boolean types and precision of one types,
+     will cause the diff to always be 1.  Note this really should have
+     simplified before reaching here.  */
+  /* A ? 1 : 0 is just (type)(A!=0). */
+  if (integer_zerop (treeop2) && integer_onep (treeop1))
+    {
+      tree t = fold_convert (TREE_TYPE (treeop0), integer_zero_node);
+      tree tmp = fold_build2 (NE_EXPR, TREE_TYPE (treeop0), treeop0, t);
+      tmp = fold_convert (type, tmp);
+      return expand_normal (tmp);
+    }
+
+  /* A ? 0 : 1 is just (type)(A==0). */
+  if (integer_zerop (treeop1) && integer_onep (treeop0))
+    {
+      tree t = fold_convert (TREE_TYPE (treeop0), integer_zero_node);
+      tree tmp = fold_build2 (EQ_EXPR, TREE_TYPE (treeop0), treeop0, t);
+      tmp = fold_convert (type, tmp);
+      return expand_normal (tmp);
+    }
+  /* A ? CST1 : CST2 can be expanded as CST2 + (!A)*(CST1 - CST2) */
+  if (TREE_CODE (treeop1) == INTEGER_CST
+      && TREE_CODE (treeop2) == INTEGER_CST)
+    diff = int_const_binop (MINUS_EXPR, treeop1, treeop2);
+  /* A ? b : b+c can be expanded as b + (!A)*(c) */
+  else if (TREE_CODE (treeop2) == SSA_NAME
+           && (srcstmt = get_def_noter_for_expr_with_code (treeop2, PLUS_EXPR)))
+    {
+      tree newop0;
+      if (gimple_assign_rhs1 (srcstmt) == treeop1)
+        diff = gimple_assign_rhs2 (srcstmt);
+      else if (gimple_assign_rhs2 (srcstmt) == treeop1)
+        diff = gimple_assign_rhs1 (srcstmt);
+      else
+        return NULL_RTX;
+      newop0 = fold_truth_not_expr (UNKNOWN_LOCATION, treeop0);
+      if (newop0 == NULL_TREE || !COMPARISON_CLASS_P (newop0))
+        diff = fold_build1 (NEGATE_EXPR, TREE_TYPE (treeop1), diff);
+      else
+        {
+          tree tmp = treeop2;
+
+          treeop0 = newop0;
+          treeop2 = treeop1;
+          treeop1 = tmp;
+        }
+    }
+  /* A ? b + c : b can be expanded as b + (!A)*(c) */
+  else if (TREE_CODE (treeop1) == SSA_NAME
+           && (srcstmt = get_def_noter_for_expr_with_code (treeop1, PLUS_EXPR)))
+    {
+      if (gimple_assign_rhs1 (srcstmt) == treeop2)
+        diff = gimple_assign_rhs2 (srcstmt);
+      else if (gimple_assign_rhs2 (srcstmt) == treeop2)
+        diff = gimple_assign_rhs1 (srcstmt);
+      else
+        return NULL_RTX;
+    }
+  else
+    return NULL_RTX;
+
+  start_sequence ();
+
+  get_condition_from_operand (treeop0, &cmpop0, &cmpop1, &cmpcode);
+
+  op00 = expand_normal (cmpop0);
+  op01 = expand_normal (cmpop1);
+  unsignedp = TYPE_UNSIGNED (TREE_TYPE (cmpop0));
+  comparison_mode = TYPE_MODE (TREE_TYPE (cmpop0));
+  comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp);
+
+  expand_operands (diff, treeop2,
+                   NULL_RTX, &op1, &op2, EXPAND_NORMAL);
+
+  temp = emit_conditional_add (NULL_RTX, comparison_code, op00, op01,
+                               comparison_mode, op2, op1, mode, unsignedp);
+
+  seq = get_insns ();
+  end_sequence ();
+
+  if (temp)
+    {
+      emit_insn (seq);
+      if (INTEGRAL_TYPE_P (type)
+          && GET_MODE_PRECISION (mode) > TYPE_PRECISION (type))
+        return reduce_to_bit_field_precision (temp, NULL_RTX, type);
+    }
+
+  return temp;
+}
+
+
+
 /* Try to expand the conditional expression which is represented by
    TREEOP0 ? TREEOP1 : TREEOP2 using conditonal moves.  If succeseds
    return the rtl reg which repsents the result.  Otherwise return
@@ -7882,11 +8057,12 @@  expand_cond_expr_using_cmove (tree treeo
   rtx op00, op01, op1, op2;
   enum rtx_code comparison_code;
   enum machine_mode comparison_mode;
-  gimple srcstmt;
   rtx temp;
   tree type = TREE_TYPE (treeop1);
   int unsignedp = TYPE_UNSIGNED (type);
   enum machine_mode mode = TYPE_MODE (type);
+  tree cmpop0, cmpop1;
+  enum tree_code cmpcode;
 
   temp = assign_temp (type, 0, 0, 1);
 
@@ -7902,34 +8078,13 @@  expand_cond_expr_using_cmove (tree treeo
   expand_operands (treeop1, treeop2,
 		   temp, &op1, &op2, EXPAND_NORMAL);
 
-  if (TREE_CODE (treeop0) == SSA_NAME
-      && (srcstmt = get_def_for_expr_class (treeop0, tcc_comparison)))
-    {
-      tree type = TREE_TYPE (gimple_assign_rhs1 (srcstmt));
-      enum tree_code cmpcode = gimple_assign_rhs_code (srcstmt);
-      op00 = expand_normal (gimple_assign_rhs1 (srcstmt));
-      op01 = expand_normal (gimple_assign_rhs2 (srcstmt));
-      comparison_mode = TYPE_MODE (type);
-      unsignedp = TYPE_UNSIGNED (type);
-      comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp);
-    }
-  else if (TREE_CODE_CLASS (TREE_CODE (treeop0)) == tcc_comparison)
-    {
-      tree type = TREE_TYPE (TREE_OPERAND (treeop0, 0));
-      enum tree_code cmpcode = TREE_CODE (treeop0);
-      op00 = expand_normal (TREE_OPERAND (treeop0, 0));
-      op01 = expand_normal (TREE_OPERAND (treeop0, 1));
-      unsignedp = TYPE_UNSIGNED (type);
-      comparison_mode = TYPE_MODE (type);
-      comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp);
-    }
-  else
-    {
-      op00 = expand_normal (treeop0);
-      op01 = const0_rtx;
-      comparison_code = NE;
-      comparison_mode = TYPE_MODE (TREE_TYPE (treeop0));
-    }
+  get_condition_from_operand (treeop0, &cmpop0, &cmpop1, &cmpcode);
+
+  op00 = expand_normal (cmpop0);
+  op01 = expand_normal (cmpop1);
+  unsignedp = TYPE_UNSIGNED (TREE_TYPE (cmpop0));
+  comparison_mode = TYPE_MODE (TREE_TYPE (cmpop0));
+  comparison_code = convert_tree_comp_to_rtx (cmpcode, unsignedp);
 
   if (GET_MODE (op1) != mode)
     op1 = gen_lowpart (mode, op1);
@@ -9017,6 +9172,10 @@  expand_expr_real_2 (sepops ops, rtx targ
 		  && TREE_TYPE (treeop1) != void_type_node
 		  && TREE_TYPE (treeop2) != void_type_node);
 
+      temp = expand_cond_expr_using_addcc (treeop0, treeop1, treeop2);
+      if (temp)
+	return temp;
+
       temp = expand_cond_expr_using_cmove (treeop0, treeop1, treeop2);
       if (temp)
 	return temp;
Index: doc/md.texi
===================================================================
--- doc/md.texi	(revision 187579)
+++ doc/md.texi	(working copy)
@@ -5236,7 +5236,7 @@  define these patterns.
 @item @samp{add@var{mode}cc}
 Similar to @samp{mov@var{mode}cc} but for conditional addition.  Conditionally
 move operand 2 or (operands 2 + operand 3) into operand 0 according to the
-comparison in operand 1.  If the comparison is true, operand 2 is moved into
+comparison in operand 1.  If the comparison is false, operand 2 is moved into
 operand 0, otherwise (operand 2 + operand 3) is moved.
 
 @cindex @code{cstore@var{mode}4} instruction pattern