Re: [PATCH, MIPS] 74k madd scheduler tweaks

[Sandra Loosemore <sandra at codesourcery dot com>]

On 08/04/2012 07:48 AM, Richard Sandiford wrote:
> Sandra Loosemore<sandra@codesourcery.com>  writes:
> > The existing scheduler bypass information for madd on the 74k uses some
> > bits copied from the 24k, and is not quite correct.  This patch is based
> > on one originally sent to us by MIPS and has been present in our local
> > source base for years.  I've confirmed that we are legally allowed to
> > contribute this to the FSF; ok for mainline?
>
> Sorry to ask, but do you have a record of why?  Reason I ask is that...
>
> > Index: gcc/config/mips/74k.md
> > ===================================================================
> > --- gcc/config/mips/74k.md	(revision 189988)
> > +++ gcc/config/mips/74k.md	(working copy)
> > @@ -168,10 +168,11 @@
> >   ;; mult/madd/msub->int_mfhilo  : 4 cycles (default)
> >   ;; mult->madd/msub             : 1 cycles
> >   ;; madd/msub->madd/msub        : 1 cycles
> > -(define_bypass 1 "r74k_int_mult,r74k_int_mul3" "r74k_int_madd"
> > -  "mips_linked_madd_p")
> > -(define_bypass 1 "r74k_int_madd" "r74k_int_madd"
> > -  "mips_linked_madd_p")
> > +(define_bypass 1 "r74k_int_mult" "r74k_int_madd")
> > +(define_bypass 1 "r74k_int_madd" "r74k_int_madd")
> > +
> > +(define_bypass 1 "r74k_int_mul3" "r74k_int_madd"
> > +  "mips_mult_madd_chain_bypass_p")
> >
> >   ;; --------------------------------------------------------------
> >   ;; Floating Point Instructions
>
> ...this looks like a step backwards.
> [long explanation trimmed]

Sigh, I wasn't able to find any detailed rationale for this patch. 
However, the "DSP ALU scheduling" patch I posted separately gives a clue 
what the intent was as it also uses mips_mult_madd_chain_bypass_p to 
give different behavior pre- and post-reload:

> +;; Before reload, all multiplier is registered as imul3 (which has a long
> +;;  latency).  We temporary jig the latency such that the macc groups
> +;;  are scheduled closely together during the first scheduler pass.
> +(define_bypass 1 "r74k_int_mul3" "r74k_dsp_mac"
> +  "mips_mult_madd_chain_bypass_p")
> +(define_bypass 1 "r74k_int_mul3" "r74k_dsp_mac_sat"
> +  "mips_mult_madd_chain_bypass_p")

If this is incorrect or looks like a hack to paper over some other 
problem, I'd be happy to drop the predicate on these bits too.  WDYT?

-Sandra