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

Peter Bergner bergner@vnet.ibm.com
Mon Jul 9 22:47:00 GMT 2007


On Mon, 2007-07-09 at 17:13 -0400, Jakub Jelinek wrote:
> 2007-07-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* simplify-rtx.c (simplify_plus_minus_op_data_cmp): If both operands
> 	are REGs, sort lower REGNOs first.
> 
> --- gcc/simplify-rtx.c.jj	2006-08-11 17:32:05.000000000 +0200
> +++ gcc/simplify-rtx.c	2007-07-09 22:53:26.000000000 +0200
> @@ -2608,6 +2608,12 @@ simplify_plus_minus_op_data_cmp (const v
>  	    - commutative_operand_precedence (d1->op));
>    if (result)
>      return result;
> +
> +  /* Group together equal REGs to do more simplification.  */
> +  if (TARGET_INDEX_OPERAND_FIRST && REG_P (d1->op) && REG_P (d2->op)
> +      && REGNO (d1->op) != REGNO (d2->op))
> +    return REGNO (d1->op) - REGNO (d2->op);
> +
>    return d1->ix - d2->ix;
>  }

I had forgot about this, but the code I added to sort by register number
in swap_commutative_operands_p was actually lifted from the mainline
version of simplify_plus_minus_op_data_cmp which was added to the trunk
by Paolo.

r116701 | bonzini | 2006-09-05 12:41:22 -0500 (Tue, 05 Sep 2006) | 11 lines

2006-09-05  Paolo Bonzini  <bonzini@gnu.org>

        PR rtl-optimization/26847
        * simplify-rtx.c (struct simplify_plus_minus_op_data): Remove ix.
        (simplify_plus_minus_op_data_cmp): For REGs, break ties on the regno.
        (simplify_plus_minus): Count n_constants while filling ops.  Replace
        qsort with insertion sort.  Before going through the array to simplify
        pairs, sort it.  Delay early exit until after the first sort, exiting
        only if no swaps occurred.  Simplify pairs in reversed order, without
        special-casing the first iteration.  Pack ops after simplifying pairs.

So mainline already has the code you're trying to add and hence doesn't
actually suffer from this bug. The problem was solely in the backport of
the patch to a compiler that didn't have Paolo's r116701 change.

Given that, I'll rebootstrap/regtest with latest sources and the current
indexed load/store patch since it's been a little while since the patch
was posted before committing it given Ian's earlier approval.

Jakub, I'll continue to test your simplify_associative_operation cleanup
patch on powerpc64-linux though and let you know how it goes.

Peter





More information about the Gcc-patches mailing list