This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Sun, Jul 15, 2007 at 10:24:03PM -0500, Peter Bergner wrote:
> On Sat, 2007-07-14 at 09:56 -0700, H.J. Lu wrote:
> > So the order Power 6 prefers is
> >
> > 1. REG_POINTER
> > 2. MEM_POINTER
> > 3. REG_P
> > 4. MEM_P
> 
> That is how the code has it now, although 4. is really everything that
> isn't a REG_POINTER, MEM_POINTER or REG_P.  Originally, I had the code
> setup for:
>   1. REG_POINTER
>   2. REG
>   3. Everything else.
> However, I hit a few test cases were we were passed a MEM_POINTER and
> a REG which would cause the REG to be put before the MEM_POINTER.
> Now the POWER ISA only has REG+CONST and REG+REG addressing modes, so
> the MEM_POINTER was eventually forced into a register, but by then, we'd
> already made the decision that it was the second operand which isn't
> the way we want it.  That's why MEM_POINTER's are now sorted before
> REGs.
> 
>  
> > Will
> > 
> > 1. REG_POINTER
> > 2. MEM_POINTER
> > 3. REG_P/MEM_P
> > 
> > work for you. 
> > 
> > Also will
> > 
> > 1. REG_POINTER/MEM_POINTER
> > 2. REG_P/MEM_P
> > 
> > work for you.
> 
> Without having actually tried it, I'm guessing they would work from the
> standpoint that we probably would get the order of the indexed load or
> store operands in the correct order.  However, by mixing REGs and MEMs,
> we'll possibly reduce the chances of some canonicalizations.  An example
> would be: r1 + *p - r1.  Although, those aren't being caught now with
> current mainline either.

I'd like to change the current addressing mode as little as possible
unless we can show there is a benifit like you saw on Power 6 on other
processors.

> 
> Did you try removing the REGno sorting from swap_commutative_operands_p
> and see if that helps like I suggested?  Did you try Paolo's suggestion

Your REGNO sorting change may be a real bug fix for:

http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=247256

I proposed a modified patch:

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

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?

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.

> 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.


H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]