Re: [PATCH] Fix PR middle-end/28690, indexed load/store performance + reload bug

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

> -  return (commutative_operand_precedence (x)
> -	  < commutative_operand_precedence (y));
> +  int precedence_x = commutative_operand_precedence (x);
> +  int precedence_y = commutative_operand_precedence (y);
> +
> +  /* Everything else being equal, preference registers with pointers
> +     over registers without pointers.  This is desirable for performance
> +     reasons on indexed load/store instructions for some processors.  */
> +  if ((precedence_x == precedence_y)
> +      && REG_P (x) && !REG_POINTER (x)
> +      && REG_P (y) && REG_POINTER (y))
> +    return 1;
> +
> +  return precedence_x < precedence_y;
>  }

This patch does not necessarily work.  You should change other users of 
commutative_operand_precedence to agree with the new definition of 
swap_commutative_operands_p.

The issue of the relative order of objects is quite thorny.  The problem 
is that GCC prefers to have (set X (op X Y)) rather than (set X (op Y 
X)).  This blocks the possibility to give a different precedence to MEMs 
 and REGs (and in turn, this would allow to have a different precedence 
for REGs depending on REG_POINTER).  I think that this patch is not 
going to be safe unless the underlying problems with (set X (op Y X)) 
are fixed.

To see an example of the issue, try changing these instructions in 
swap_commutative_operands_with_target:

  /* With equal precedence, both orders are ok, but it is better if the
     first operand is TARGET, or if both TARGET and OP0 are pseudos.  */
  if (target == 0 || REG_P (target))
    return (REG_P (op1) && !REG_P (op0)) || target == op1;
  else
    return rtx_equal_p (op1, target);

to just "return false".  crafty from SPEC2000 should degrade a lot on x86.

Paolo