On Mon, Jun 29, 2015 at 6:08 PM, Maciej W. Rozycki <ma...@linux-mips.org> wrote: > Richard, please have a look at my question below in a reference to your > previous statement. > > On Thu, 18 Jun 2015, Steve Ellcey wrote: > >> OK, I checked in the prequel patch and here is a new copy of the >> original patch based off of that (and with no HONOR_NAN checks in the >> fma/madd instructions). >> >> OK for checkin? > > Please see below for my notes. > >> 2015-06-18 Steve Ellcey <sell...@imgtec.com> >> >> * config.gcc (mips*-*-*): Add fused-madd.opt. > > Please use angle brackets as per > <https://www.gnu.org/prep/standards/html_node/Indicating-the-Part-Changed.html>, > i.e.: > > * config.gcc <mips*-*-*>: Add fused-madd.opt. > > There's no function or similar entity involved here and `mips*-*-*' is a > case value like with the C language's `switch' statement where you'd use > angle brackets too to refer to individual cases. > >> (*nmsub4<mode>_fastmath) Update condition. > > Extraneous space here. > >> diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md >> index f6912e1..4f5692c 100644 >> --- a/gcc/config/mips/mips.md >> +++ b/gcc/config/mips/mips.md > [...] >> +;; fnma is defined in GCC as (fma (neg op1) op2 op3) >> +;; (-op1 * op2) + op3 ==> -(op1 * op2) + op3 ==> -((op1 * op2) - op3) >> +;; The mips nmsub instructions implement -((op1 * op2) - op3) >> +;; This transformation means we may return the wrong signed zero >> +;; so we check HONOR_SIGNED_ZEROS. >> + >> +(define_expand "fnma<mode>4" >> + [(set (match_operand:ANYF 0 "register_operand") >> + (fma:ANYF (neg:ANYF (match_operand:ANYF 1 "register_operand")) >> + (match_operand:ANYF 2 "register_operand") >> + (match_operand:ANYF 3 "register_operand")))] >> + "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4) >> + && !HONOR_SIGNED_ZEROS (<MODE>mode)") > > Have you considered the alternative/complementary approach proposed by > Richard here: <http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00680.html>, > i.e. to introduce further expanders, e.g.: > > fmanM4: (neg:M (fma:M OP1 OP2 OP3)) (multiply-add, negated) > > fmsnM4: (neg:M (fma:M OP1 OP2 (neg:M OP3))) (multiply-subtract, negated) > > ? > > These patterns wouldn't need a check for !HONOR_SIGNED_ZEROS as they > match the respective hardware instructions in an exact manner. Therefore > I think they would be more useful as they would also suit software that > claims/requires full IEEE Std 754 compliance. > > Richard, do you maintain the introduction of these additional operations > would be a good idea and one you're willing to support for the purpose of > patch acceptance/approval if implemented?
Yes, esp. if there is now a second architecture that has such instructions. Thanks, Richard. >> +;; fnms is defined as: (fma (neg op1) op2 (neg op3)) >> +;; ((-op1) * op2) - op3 ==> -(op1 * op2) - op3 ==> -((op1 * op2) + op3) >> +;; The mips nmadd instructions implement -((op1 * op2) + op3) >> +;; This transformation means we may return the wrong signed zero >> +;; so we check HONOR_SIGNED_ZEROS. >> + >> +(define_expand "fnms<mode>4" >> + [(set (match_operand:ANYF 0 "register_operand") >> + (fma:ANYF >> + (neg:ANYF (match_operand:ANYF 1 "register_operand")) >> + (match_operand:ANYF 2 "register_operand") >> + (neg:ANYF (match_operand:ANYF 3 "register_operand"))))] >> + "(ISA_HAS_FUSED_MADD3 || ISA_HAS_FUSED_MADD4) >> + && !HONOR_SIGNED_ZEROS (<MODE>mode)") > > Same observation here. > > The change looks good to me otherwise. > > Maciej
