Re: [patch, mips] Patch to control the use of integer madd/msub instructions

[Richard Sandiford <rdsandiford at googlemail dot com>]

"Steve Ellcey " <sellcey@imgtec.com> writes:
> While testing GCC on a 74k MIPS chip I noticed that by default the -mtune=74k*
> flags cause GCC to not use the integer madd/msub instructions.  According to
> the checkin comments these were found to cause a performance hit over using
> individual mult and add/sub instructions.  I think there are some programs
> though where using madd/msub would be a win on the 74k and I would like to
> have a flag to allow users to override the default behaviour (either turning
> it on for 74k or turning it off for other achitectures).  This patch allows
> -mimadd or -mno-imadd to override the default behaviour but does not change
> that default behaviour.

This is similar in spirit to -mbranch-likely.  It'd be good for consistency
if they were defined in a similar style.  I think that means removing
!TARGET_MIPS16 from ISA_HAS_MADD_MSUB and instead having:

#define GENERATE_MADD_MSUB      (TARGET_IMADD && !TARGET_MIPS16)

There would also be:

#define PTF_AVOID_IMADD 0x2

which should be included in the 74k description, and a block similar to
the MASK_BRANCHLIKELY one in mips_option_override.  There needs to be
documentation in invoke.texi.

But -- sorry for the soapbox speech -- it would be better to retune
so that new options aren't needed.  I'm assuming you're testing against
the same microarchitecture that the original 74k authors were.  If so,
it seems like -mimadd is just an option for choosing between two bad
implementations.  One uses MADD and MSUB unconditionally (contrary to
the experience of the original authors) and the other never uses it
at all (contrary to your experience).

That's not enough reason to reject the patch, just saying :-)

Thanks,
Richard