Patchwork [rs6000] permit the first operand of isel to be 0

login
register
mail settings
Submitter Nathan Froyd
Date July 15, 2010, 8:47 p.m.
Message ID <20100715204741.GU12333@codesourcery.com>
Download mbox | patch
Permalink /patch/59047/
State New
Headers show

Comments

Nathan Froyd - July 15, 2010, 8:47 p.m.
Like several other PowerPC insns, isel treats RA == 0 specially; the
value zero is used, instead of the contents of r0.  GCC doesn't take
advantage of this, preferring instead to constrain both operands to be
in registers 1-31.  This leads to unnecessary code pessimizations.

The patch below changes this by changing the constraints on the isel
patterns in two ways:

- Only comparisons that the actual isel insn can handle are permitted;

- Once we don't have to worry about swapping operands in output_isel, we
  can have proper constraints for both of isel's alternatives.

In addition, the patch centralizes the bulk of the above logic in
rs6000_emit_int_cmove, rather than duplicating it in rs6000_emit_sISEL
and rs6000_emit_int_cmove.  rs6000_emit_int_cmove now needs to handle
swapping arguments for combined conditions.

Tested with cross to powerpc-eabispe (ongoing, but looking good so far).
OK to commit?

-Nathan

	* config/rs6000/rs6000.c (rs6000_emit_sISEL): Let rs6000_emit_int_cmove
	do all the work.
	(rs6000_emit_int_cmove): Use function pointers for insn generation.
	Don't force values into registers unnecessarily.  Swap operands if
	the condition requires it.
	(output_isel): Assert that we're not given conditions we can't handle.
	Delete corresponding code.
	* config/rs6000/rs6000.md (isel_signed_<mode>): Use
	scc_comparison_operator constraint.  Permit 0 for the consequent
	operand.  Permit any GPR for the alternative operand.
	(isel_unsigned_<mode>): Likewise.
David Edelsohn - July 15, 2010, 10:37 p.m.
On Thu, Jul 15, 2010 at 4:47 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> Like several other PowerPC insns, isel treats RA == 0 specially; the
> value zero is used, instead of the contents of r0.  GCC doesn't take
> advantage of this, preferring instead to constrain both operands to be
> in registers 1-31.  This leads to unnecessary code pessimizations.
>
> The patch below changes this by changing the constraints on the isel
> patterns in two ways:
>
> - Only comparisons that the actual isel insn can handle are permitted;
>
> - Once we don't have to worry about swapping operands in output_isel, we
>  can have proper constraints for both of isel's alternatives.
>
> In addition, the patch centralizes the bulk of the above logic in
> rs6000_emit_int_cmove, rather than duplicating it in rs6000_emit_sISEL
> and rs6000_emit_int_cmove.  rs6000_emit_int_cmove now needs to handle
> swapping arguments for combined conditions.
>
> Tested with cross to powerpc-eabispe (ongoing, but looking good so far).
> OK to commit?
>
> -Nathan
>
>        * config/rs6000/rs6000.c (rs6000_emit_sISEL): Let rs6000_emit_int_cmove
>        do all the work.
>        (rs6000_emit_int_cmove): Use function pointers for insn generation.
>        Don't force values into registers unnecessarily.  Swap operands if
>        the condition requires it.
>        (output_isel): Assert that we're not given conditions we can't handle.
>        Delete corresponding code.
>        * config/rs6000/rs6000.md (isel_signed_<mode>): Use
>        scc_comparison_operator constraint.  Permit 0 for the consequent
>        operand.  Permit any GPR for the alternative operand.
>        (isel_unsigned_<mode>): Likewise.

Okay.

Thanks, David

Patch

Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c	(revision 162223)
+++ config/rs6000/rs6000.c	(working copy)
@@ -15997,53 +15997,12 @@  rs6000_generate_compare (rtx cmp, enum m
 }
 
 
-/* Emit the RTL for an sCOND pattern.  */
+/* Emit the RTL for an sISEL pattern.  */
 
 void
-rs6000_emit_sISEL (enum machine_mode mode, rtx operands[])
+rs6000_emit_sISEL (enum machine_mode mode ATTRIBUTE_UNUSED, rtx operands[])
 {
-  rtx condition_rtx;
-  enum machine_mode op_mode;
-  enum rtx_code cond_code;
-  rtx result = operands[0];
-
-  condition_rtx = rs6000_generate_compare (operands[1], mode);
-  cond_code = GET_CODE (condition_rtx);
-
-  op_mode = GET_MODE (XEXP (operands[1], 0));
-  if (op_mode == VOIDmode)
-    op_mode = GET_MODE (XEXP (operands[1], 1));
-
-  if (TARGET_POWERPC64 && GET_MODE (result) == DImode)
-    {
-      PUT_MODE (condition_rtx, DImode);
-      if (cond_code == GEU || cond_code == GTU || cond_code == LEU
-         || cond_code == LTU)
-       emit_insn (gen_isel_unsigned_di (result, condition_rtx,
-					force_reg (DImode, const1_rtx),
-					force_reg (DImode, const0_rtx),
-					XEXP (condition_rtx, 0)));
-      else
-       emit_insn (gen_isel_signed_di (result, condition_rtx,
-				      force_reg (DImode, const1_rtx),
-				      force_reg (DImode, const0_rtx),
-				      XEXP (condition_rtx, 0)));
-    }
-  else
-    {
-      PUT_MODE (condition_rtx, SImode);
-      if (cond_code == GEU || cond_code == GTU || cond_code == LEU
-	 || cond_code == LTU)
-       emit_insn (gen_isel_unsigned_si (result, condition_rtx,
-					force_reg (SImode, const1_rtx),
-					force_reg (SImode, const0_rtx),
-					XEXP (condition_rtx, 0)));
-      else
-       emit_insn (gen_isel_signed_si (result, condition_rtx,
-				      force_reg (SImode, const1_rtx),
-				      force_reg (SImode, const0_rtx),
-				      XEXP (condition_rtx, 0)));
-    }
+  rs6000_emit_int_cmove (operands[0], operands[1], const1_rtx, const0_rtx);
 }
 
 void
@@ -16710,6 +16669,9 @@  rs6000_emit_int_cmove (rtx dest, rtx op,
 {
   rtx condition_rtx, cr;
   enum machine_mode mode = GET_MODE (dest);
+  enum rtx_code cond_code;
+  rtx (*isel_func) (rtx, rtx, rtx, rtx, rtx);
+  bool signedp;
 
   if (mode != SImode && (!TARGET_POWERPC64 || mode != DImode))
     return 0;
@@ -16718,27 +16680,37 @@  rs6000_emit_int_cmove (rtx dest, rtx op,
      compare, it just looks at the CRx bits set by a previous compare
      instruction.  */
   condition_rtx = rs6000_generate_compare (op, mode);
+  cond_code = GET_CODE (condition_rtx);
   cr = XEXP (condition_rtx, 0);
+  signedp = GET_MODE (cr) == CCmode;
 
-  if (mode == SImode)
-    {
-      if (GET_MODE (cr) == CCmode)
-	emit_insn (gen_isel_signed_si (dest, condition_rtx,
-				       true_cond, false_cond, cr));
-      else
-	emit_insn (gen_isel_unsigned_si (dest, condition_rtx,
-					 true_cond, false_cond, cr));
-    }
-  else
+  isel_func = (mode == SImode
+	       ? (signedp ? gen_isel_signed_si : gen_isel_unsigned_si)
+	       : (signedp ? gen_isel_signed_di : gen_isel_unsigned_di));
+
+  switch (cond_code)
     {
-      if (GET_MODE (cr) == CCmode)
-	emit_insn (gen_isel_signed_di (dest, condition_rtx,
-				       true_cond, false_cond, cr));
-      else
-	emit_insn (gen_isel_unsigned_di (dest, condition_rtx,
-					 true_cond, false_cond, cr));
+    case LT: case GT: case LTU: case GTU: case EQ:
+      /* isel handles these directly.  */
+      break;
+
+    default:
+      /* We need to swap the sense of the comparison.  */
+      {
+	rtx t = true_cond;
+	true_cond = false_cond;
+	false_cond = t;
+	PUT_CODE (condition_rtx, reverse_condition (cond_code));
+      }
+      break;
     }
 
+  false_cond = force_reg (mode, false_cond);
+  if (true_cond != const0_rtx)
+    true_cond = force_reg (mode, true_cond);
+
+  emit_insn (isel_func (dest, condition_rtx, true_cond, false_cond, cr));
+
   return 1;
 }
 
@@ -16748,13 +16720,10 @@  output_isel (rtx *operands)
   enum rtx_code code;
 
   code = GET_CODE (operands[1]);
-  if (code == GE || code == GEU || code == LE || code == LEU || code == NE)
-    {
-      PUT_CODE (operands[1], reverse_condition (code));
-      return "isel %0,%3,%2,%j1";
-    }
-  else
-    return "isel %0,%2,%3,%j1";
+
+  gcc_assert (!(code == GE || code == GEU || code == LE || code == LEU || code == NE));
+
+  return "isel %0,%2,%3,%j1";
 }
 
 void
Index: config/rs6000/rs6000.md
===================================================================
--- config/rs6000/rs6000.md	(revision 162223)
+++ config/rs6000/rs6000.md	(working copy)
@@ -6091,13 +6091,13 @@  (define_expand "mov<mode>cc"
 ;; change the mode underneath our feet and then gets confused trying
 ;; to reload the value.
 (define_insn "isel_signed_<mode>"
-  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
+  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r,r")
 	(if_then_else:GPR
-	 (match_operator 1 "comparison_operator"
-			 [(match_operand:CC 4 "cc_reg_operand" "y")
+	 (match_operator 1 "scc_comparison_operator"
+			 [(match_operand:CC 4 "cc_reg_operand" "y,y")
 			  (const_int 0)])
-	 (match_operand:GPR 2 "gpc_reg_operand" "b")
-	 (match_operand:GPR 3 "gpc_reg_operand" "b")))]
+	 (match_operand:GPR 2 "reg_or_cint_operand" "O,b")
+	 (match_operand:GPR 3 "gpc_reg_operand" "r,r")))]
   "TARGET_ISEL<sel>"
   "*
 { return output_isel (operands); }"
@@ -6105,13 +6105,13 @@  (define_insn "isel_signed_<mode>"
    (set_attr "length" "4")])
 
 (define_insn "isel_unsigned_<mode>"
-  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
+  [(set (match_operand:GPR 0 "gpc_reg_operand" "=r,r")
 	(if_then_else:GPR
-	 (match_operator 1 "comparison_operator"
-			 [(match_operand:CCUNS 4 "cc_reg_operand" "y")
+	 (match_operator 1 "scc_comparison_operator"
+			 [(match_operand:CCUNS 4 "cc_reg_operand" "y,y")
 			  (const_int 0)])
-	 (match_operand:GPR 2 "gpc_reg_operand" "b")
-	 (match_operand:GPR 3 "gpc_reg_operand" "b")))]
+	 (match_operand:GPR 2 "reg_or_cint_operand" "O,b")
+	 (match_operand:GPR 3 "gpc_reg_operand" "r,r")))]
   "TARGET_ISEL<sel>"
   "*
 { return output_isel (operands); }"