Re: [PING][PATCH][REVISED] Fix PR middle-end/PR28690, modify swap_commutative_operands_p

["H.J. Lu" <hjl at lucon dot org>]

On Sat, Jul 14, 2007 at 10:38:34AM -0500, Peter Bergner wrote:
> On Sat, 2007-07-14 at 07:25 -0700, H.J. Lu wrote:
> > Peter, will gcc-PR28690-5.patch solve your problem?
> 
> I'm not sure you actually looked at my previous post or the patch.
> By leaving out the changes that adjust all of the precedence values
> by -3 but leaving in this part:
> 
> -      return -1;
> +      if (REG_P (op))
> +       return (REG_POINTER (op)) ? -1 : -3;
> +      else
> +       return (MEM_P (op) && MEM_POINTER (op)) ? -2 : -4;
> 
> You have now just changed the precedence sorting in an unwanted way.
> For example, RTX_EXTRA which has a precedence of -2 (-5 with my patch)
> and previously always had a lower precedence than all RTX_OBJs, now has
> the same precedence or higher precedence than REG's and MEM's.
> We don't want that!
> 
> As I mentioned in the previous post, the adjustment of the precedence
> values by -3 is totally cosmetic and will not change the behavior of
> the compiler in any way, so I'm curious why you even attempted to leave
> that part out.
> 
> Peter
> 

Something like this? I have a question on this part


+      if (REG_P (op))
+	return (REG_POINTER (op)) ? -1 : -3;
+      else
+	return (MEM_P (op) && MEM_POINTER (op)) ? -2 : -4;


So the order Power 6 prefers is

1. REG_POINTER
2. MEM_POINTER
3. REG_P
4. MEM_P

Is that correct? Will

1. REG_POINTER
2. MEM_POINTER
3. REG_P/MEM_P

work for you.


H.J.
----
2007-06-20  Peter Bergner  <bergner@vnet.ibm.com>

	PR middle-end/PR28690
	* optabs.c (emit_cmp_and_jump_insns): Allow EQ compares.
	* rtlanal.c (commutative_operand_precedence): Prefer both
	REG_POINTER and MEM_POINTER operands over REG and MEM operands.
	* tree-ssa-address.c (gen_addr_rtx): Use simplify_gen_binary
	instead of gen_rtx_PLUS.

--- gcc/optabs.c.PR28690	2007-07-13 22:11:26.000000000 -0700
+++ gcc/optabs.c	2007-07-14 06:38:57.000000000 -0700
@@ -4054,9 +4054,11 @@ emit_cmp_and_jump_insns (rtx x, rtx y, e
   /* Swap operands and condition to ensure canonical RTL.  */
   if (swap_commutative_operands_p (x, y))
     {
-      /* If we're not emitting a branch, this means some caller
-         is out of sync.  */
-      gcc_assert (label);
+      /* If we're not emitting a branch, callers are required to pass
+	 operands in an order conforming to canonical RTL.  We relax this
+	 for commutative comparsions so callers using EQ don't need to do
+	 swapping by hand.  */
+      gcc_assert (label || (comparison == swap_condition (comparison)));
 
       op0 = y, op1 = x;
       comparison = swap_condition (comparison);
--- gcc/rtlanal.c.PR28690	2007-07-13 22:11:26.000000000 -0700
+++ gcc/rtlanal.c	2007-07-14 09:38:48.000000000 -0700
@@ -2877,9 +2877,9 @@ commutative_operand_precedence (rtx op)
   
   /* Constants always come the second operand.  Prefer "nice" constants.  */
   if (code == CONST_INT)
-    return -7;
+    return -10;
   if (code == CONST_DOUBLE)
-    return -6;
+    return -9;
   op = avoid_constant_pool_reference (op);
   code = GET_CODE (op);
 
@@ -2887,22 +2887,24 @@ commutative_operand_precedence (rtx op)
     {
     case RTX_CONST_OBJ:
       if (code == CONST_INT)
-        return -5;
+        return -8;
       if (code == CONST_DOUBLE)
-        return -4;
-      return -3;
+        return -7;
+      return -6;
 
     case RTX_EXTRA:
       /* SUBREGs of objects should come second.  */
       if (code == SUBREG && OBJECT_P (SUBREG_REG (op)))
-        return -2;
-
+        return -5;
       return 0;
 
     case RTX_OBJ:
       /* Complex expressions should be the first, so decrease priority
          of objects.  */
-      return -1;
+      if (REG_P (op))
+	return (REG_POINTER (op)) ? -1 : -3;
+      else
+	return (MEM_P (op) && MEM_POINTER (op)) ? -2 : -4;
 
     case RTX_COMM_ARITH:
       /* Prefer operands that are themselves commutative to be first.
--- gcc/tree-ssa-address.c.PR28690	2007-07-13 22:11:26.000000000 -0700
+++ gcc/tree-ssa-address.c	2007-07-14 06:38:57.000000000 -0700
@@ -125,7 +125,7 @@ gen_addr_rtx (rtx symbol, rtx base, rtx 
   if (base)
     {
       if (*addr)
-	*addr = gen_rtx_PLUS (Pmode, *addr, base);
+	*addr = simplify_gen_binary (PLUS, Pmode, base, *addr);
       else
 	*addr = base;
     }