Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756092AbdGXNhK convert rfc822-to-8bit (ORCPT ); Mon, 24 Jul 2017 09:37:10 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:54344 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932145AbdGXNgM (ORCPT ); Mon, 24 Jul 2017 09:36:12 -0400 From: Aleksandar Markovic To: James Hogan , Aleksandar Markovic CC: "linux-mips@linux-mips.org" , Miodrag Dinic , Goran Ferenc , "Douglas Leung" , "linux-kernel@vger.kernel.org" , Paul Burton , "Petar Jovanovic" , Raghu Gandham , Ralf Baechle Subject: RE: [PATCH v3 05/16] MIPS: math-emu: .: Fix quiet NaN propagation Thread-Topic: [PATCH v3 05/16] MIPS: math-emu: .: Fix quiet NaN propagation Thread-Index: AQHTAitmv0L4IhaR1UeYohnOc8UbGKJe0OIAgAQiJ3M= Date: Mon, 24 Jul 2017 13:36:05 +0000 Message-ID: References: <1500646206-2436-1-git-send-email-aleksandar.markovic@rt-rk.com> <1500646206-2436-6-git-send-email-aleksandar.markovic@rt-rk.com>,<20170721144504.GJ6973@jhogan-linux.le.imgtec.org> In-Reply-To: <20170721144504.GJ6973@jhogan-linux.le.imgtec.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [82.117.201.26] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3670 Lines: 106 Hi, James, I appreciate your thorough and expeditious review. > > ________________________________________ > From: James Hogan > Sent: Friday, July 21, 2017 7:45 AM > To: Aleksandar Markovic > Cc: linux-mips@linux-mips.org; Aleksandar Markovic; Miodrag Dinic; Goran Ferenc; Douglas Leung; linux-kernel@vger.kernel.org; Paul Burton; Petar Jovanovic; Raghu Gandham; Ralf Baechle > Subject: Re: [PATCH v3 05/16] MIPS: math-emu: .: Fix quiet NaN propagation > > On Fri, Jul 21, 2017 at 04:09:03PM +0200, Aleksandar Markovic wrote: > > From: Aleksandar Markovic > > > > Fix the value returned by ., if both inputs > > are quiet NaNs. The specifications of . state > > that the returned value in such cases should be the quiet NaN > > contained in register fs. > > > > The relevant example: > > > > MAX.S fd,fs,ft: > > If fs contains qNaN1, and ft contains qNaN2, fd is going to contain > > qNaN1 (without this patch, it used to contain qNaN2). > > > > Consider adding: > > Fixes: a79f5f9ba508 ("MIPS: math-emu: Add support for the MIPS R6 MAX{, A} FPU instruction") > Fixes: 4e9561b20e2f ("MIPS: math-emu: Add support for the MIPS R6 MIN{, A} FPU instruction") > Will add in v4 (for all MIN/MAX/MINA/MAXa patches). > > Signed-off-by: Miodrag Dinic > > Signed-off-by: Goran Ferenc > > Signed-off-by: Aleksandar Markovic > > Consider adding: > > Cc: # 4.3+ Will add on v4 (for all MIN/MAX/MINA/MAXA patches). > > --- > > arch/mips/math-emu/dp_fmax.c | 8 ++++++-- > > arch/mips/math-emu/dp_fmin.c | 8 ++++++-- > > arch/mips/math-emu/sp_fmax.c | 8 ++++++-- > > arch/mips/math-emu/sp_fmin.c | 8 ++++++-- > > 4 files changed, 24 insertions(+), 8 deletions(-) > > > > diff --git a/arch/mips/math-emu/dp_fmax.c b/arch/mips/math-emu/dp_fmax.c > > index fd71b8d..567fc33 100644 > > --- a/arch/mips/math-emu/dp_fmax.c > > +++ b/arch/mips/math-emu/dp_fmax.c > > @@ -47,6 +47,9 @@ union ieee754dp ieee754dp_fmax(union ieee754dp x, union ieee754dp y) > > case CLPAIR(IEEE754_CLASS_SNAN, IEEE754_CLASS_INF): > > return ieee754dp_nanxcpt(x); > > > > + case CLPAIR(IEEE754_CLASS_QNAN, IEEE754_CLASS_QNAN): > > + return x; > > couldn't the above... > > > + > > /* numbers are preferred to NaNs */ > > case CLPAIR(IEEE754_CLASS_ZERO, IEEE754_CLASS_QNAN): > > case CLPAIR(IEEE754_CLASS_NORM, IEEE754_CLASS_QNAN): > > @@ -54,7 +57,6 @@ union ieee754dp ieee754dp_fmax(union ieee754dp x, union ieee754dp y) > > ... go somewhere around here and fall through to the existing return x > case? > It could, but at the expense of code clarity and/or logical grouping of special cases, which after this patch looks like: . . . | case of both inputs qNaN | case of only x input qNaN | case of only y input qNaN | . . . If you agree, I suggest keeping the code the same as currently proposed in this patch, except that the following comments should be added in appropriate places: /* * Quiet NaN handling */ /* The case of both inputs quiet NaNs */ . . . /* The cases of exactly one input quiet NaN */ Unfortunately, the code segment for handling of sNaN and infinity inputs do not follow the organization that I proposed. However, I think that my proposal for case organization is the superior one - therefore I intend to keep it in v4, unless you tell me not to do so. Regards, Aleksandar