Re: [PATCH] Fix PR37053: Move tweaks of commutative precedence to target hook

[Richard Sandiford <rdsandiford at googlemail dot com>]

Maxim Kuvyrkov <maxim@codesourcery.com> writes:
> Paolo Bonzini wrote:
>> I cannot say I like Maxim's patch and he knows, but unfortunately, m68k 
>> apparently implements 2-address arithmetic in a different way than x86 
>> (or with other constraint) and we're talking about a lot of ICEs there. :-(

FWIW, I don't like the patch either.  Operator precedence seems too
fundemantal to be deferred to a target hook.  But...

>> Another possibility, which you would have to try on PowerPC, could be to 
>> limit the change of precedence to before reload.
>
> Here is the other patch.  It has a downside that *during* and after 
> reload the optimal for PowerPC commutativeness is not available, so that 
> may degrade performance.
>
> Peter, if you find out that this patch doesn't cause any performance 
> problems, I'm happy to go with it.
>
> Index: gcc-mainline/gcc/rtlanal.c
> ===================================================================
> --- gcc-mainline/gcc/rtlanal.c  (revision 148831)
> +++ gcc-mainline/gcc/rtlanal.c  (working copy)
> @@ -2936,8 +2936,9 @@ commutative_operand_precedence (rtx op)
>       case RTX_OBJ:
>         /* Complex expressions should be the first, so decrease priority
>            of objects.  Prefer pointer objects over non pointer objects.  */
> -      if ((REG_P (op) && REG_POINTER (op))
> -         || (MEM_P (op) && MEM_POINTER (op)))
> +      if ((!reload_completed && !reload_in_progress)
> +         && ((REG_P (op) && REG_POINTER (op))
> +             || (MEM_P (op) && MEM_POINTER (op))))
>          return -1;
>         return -2;
>
>> That would be a 
>> simpler patch and one that does not risk slowing down the compiler that 
>> tiny 0.1% that sums up quickly.  But it would also be pretty magical...

...I don't like this any better.  We can process constraints before reload too,
and having different rules for "%" at different stages seems like a bad idea.

I always assumed that one of the main uses of "%" was precisely to allow
regs and mems to be swapped in order to satisfy constraints (which if I've
understood correctly is all that m68k seems to be trying to do).  Declaring
a non-pointer reg + a pointer mem to be noncanonical rtl seems far too strong
at any stage of compilation.  I know 28690 was a serious performance problem,
but I think we need to consider handling this in another way, separating out
"try to achieve this" from "non-canonical".

Which isn't very constructive, sorry.

Richard