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

[Peter Bergner]

On Sun, 2007-07-15 at 21:25 -0700, H.J. Lu wrote:
> Your REGNO sorting change may be a real bug fix for:
> 
> http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=247256

This is a redhat/gcc-4_1-branch bug and it fails because it _has_
my REGNO sorting change to swap_commutative_operands_p, but doesn't
have the simplify_plus_minus_op_data_cmp REGNO sorting code which
was added to mainline by:

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.

The fix as Jakub discovered and posted, was to add the REGNO sorting code to
simplify_plus_minus_op_data_cmp.


> to unite swap_commutative_operands_p and
> simplify_plus_minus_op_data_cmp. But sort REGNO in
> swap_commutative_operands_p introduces a regression on x86-64:
> 
> FAIL: gcc.target/i386/387-11.c scan-assembler-not fchs
> 
> Did you run "make check" on x86-64 with your patch?

I'm positive I did, but cannot verify that now, as the system was
reclaimed and reinstalled.  I have the system again, so I'll do
another run of bootstrap/regtesting to verify.

I'm sure you've told me before, but I've probably lost track, can you
tell me exactly what GCC configure options you use to build GCC with?



> If Power needs the REGNO sorting to fix a real bug and the same patch
> causes the performance regression on x86, we get a problem here. I
> think REGNO sorting should be disable/enabled at both places,
> swap_commutative_operands_p and simplify_plus_minus_op_data_cmp, at
> the same time.

As I said previously, it was suggested to me to add REGNO sorting so
as to make it similar to simplify_plus_minus_op_data_cmp to allow more
chances for canonicalizations.  I also said then that it was not
required for getting the order correct for POWER6 indexed load/store
operands.



> > of using -frename-registers?
> 
> Are you suggesting we should enable -frename-registers for -O2? I am
> tracking -O2 performance. Add -frename-registers won't help me.

I'm not suggesting anything.  I just repeating Paolo's suggestion he
made here:

    http://gcc.gnu.org/ml/gcc-patches/2007-07/msg01233.html

of trying to use -frename-registers to see whether that helped your
performance regression.

Peter