Re: [PATCH] fix non-canonical RTL produced by expand

[Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>]

> > >     * optabs.c (expand_binop): Use best_order_for_commutative_binop_p
> > >     to order operands.
> > >     (best_order_for_commutative_binop_p): New.   
> >
> > You should have gone with the name I gave you.  This name is exactly
> > wrong -- it returns true when the order given is in fact *not* best.
>
> I see.  I'll switch it.
Sorry for hitting send too early.  Here is the patch.

Paolo
2005-09-15  Paolo Bonzini  <bonzini@gnu.org>

	* optabs.c (expand_binop): Use swap_commutative_operands_with_target
	to order operands.
	(swap_commutative_operands_with_target): New.

Index: optabs.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/optabs.c,v
retrieving revision 1.290
diff -p -u -u -r1.290 optabs.c
--- optabs.c	19 Aug 2005 21:20:02 -0000	1.290
+++ optabs.c	15 Sep 2005 18:52:45 -0000
@@ -998,6 +998,28 @@ expand_simple_binop (enum machine_mode m
   return expand_binop (mode, binop, op0, op1, target, unsignedp, methods);
 }
 
+
+/* Return whether OP0 and OP1 should be swapped when expanding a commutative
+   binop.  Order them according to commutative_operand_precedence and, if
+   possible, try to put TARGET first.  */
+static bool
+swap_commutative_operands_with_target (rtx target, rtx op0, rtx op1)
+{
+  int op0_prec = commutative_operand_precedence (op0);
+  int op1_prec = commutative_operand_precedence (op1);
+
+  if (op0_prec < op1_prec)
+    return true;
+
+  if (op0_prec > op1_prec)
+    return false;
+
+  /* With equal precedence, both orders are ok, but try to put the
+     target first.  */
+  return target && rtx_equal_p (op1, target);
+}
+
+
 /* Generate code to perform an operation specified by BINOPTAB
    on operands OP0 and OP1, with result having machine-mode MODE.
 
@@ -1060,10 +1082,7 @@ expand_binop (enum machine_mode mode, op
   /* Record where to delete back to if we backtrack.  */
   last = get_last_insn ();
 
-  /* If operation is commutative,
-     try to make the first operand a register.
-     Even better, try to make it the same as the target.
-     Also try to make the last operand a constant.  */
+  /* If operation is commutative, canonicalize the order of the operands.  */
   if (GET_RTX_CLASS (binoptab->code) == RTX_COMM_ARITH
       || binoptab == smul_widen_optab
       || binoptab == umul_widen_optab
@@ -1071,13 +1090,7 @@ expand_binop (enum machine_mode mode, op
       || binoptab == umul_highpart_optab)
     {
       commutative_op = 1;
-
-      if (((target == 0 || REG_P (target))
-	   ? ((REG_P (op1)
-	       && !REG_P (op0))
-	      || target == op1)
-	   : rtx_equal_p (op1, target))
-	  || GET_CODE (op0) == CONST_INT)
+      if (swap_commutative_operands_with_target (Target, op0, op1))
 	{
 	  temp = op1;
 	  op1 = op0;