Re: [PATCH v3 07/16] MIPS: math-emu: <MAX|MIN>.<D|S>: Fix cases of both inputs negative

From: James Hogan
Date: Fri Jul 21 2017 - 11:15:21 EST

On Fri, Jul 21, 2017 at 04:09:05PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <aleksandar.markovic@xxxxxxxxxx>
>
> Fix the value returned by <MAX|MIN>.<D|S>, if both inputs are negative
> normal fp numbers. The previous logic did not take into account that
> if both inputs have the same sign, there should be separate treatment
> of the cases when both inputs are negative and when both inputs are
> positive.
>
> The relevant example:
>
> MAX.S fd,fs,ft:
> If fs contains -5, and ft contains -7, fd is going to contain -5
> (without this patch, it used to contain -7).

ouch!

>

same fixes/stable comment as for previous min/max patches

> Signed-off-by: Miodrag Dinic <miodrag.dinic@xxxxxxxxxx>
> Signed-off-by: Goran Ferenc <goran.ferenc@xxxxxxxxxx>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@xxxxxxxxxx>

Reviewed-by: James Hogan <james.hogan@xxxxxxxxxx>

Cheers
James

> ---
> arch/mips/math-emu/dp_fmax.c | 33 +++++++++++++++++++++++++--------
> arch/mips/math-emu/dp_fmin.c | 33 +++++++++++++++++++++++++--------
> arch/mips/math-emu/sp_fmax.c | 33 +++++++++++++++++++++++++--------
> arch/mips/math-emu/sp_fmin.c | 32 +++++++++++++++++++++++++-------
> 4 files changed, 100 insertions(+), 31 deletions(-)
>
> diff --git a/arch/mips/math-emu/dp_fmax.c b/arch/mips/math-emu/dp_fmax.c
> index 9517572..a0175cc 100644
> --- a/arch/mips/math-emu/dp_fmax.c
> +++ b/arch/mips/math-emu/dp_fmax.c
> @@ -106,16 +106,33 @@ union ieee754dp ieee754dp_fmax(union ieee754dp x, union ieee754dp y)
> else if (xs < ys)
> return x;
>
> - /* Compare exponent */
> - if (xe > ye)
> - return x;
> - else if (xe < ye)
> - return y;
> + /* Signs of inputs are the same, let's compare exponents */
> + if (xs == 0) {
> + /* Inputs are both positive */
> + if (xe > ye)
> + return x;
> + else if (xe < ye)
> + return y;
> + } else {
> + /* Inputs are both negative */
> + if (xe > ye)
> + return y;
> + else if (xe < ye)
> + return x;
> + }
>
> - /* Compare mantissa */
> - if (xm <= ym)
> + /* Signs and exponents of inputs are the same, let's compare mantissas */
> + if (xs == 0) {
> + /* Inputs are both positive, with equal exponents */
> + if (xm <= ym)
> + return y;
> + return x;
> + } else {
> + /* Inputs are both negative, with equal exponents */
> + if (xm <= ym)
> + return x;
> return y;
> - return x;
> + }
> }
>
> union ieee754dp ieee754dp_fmaxa(union ieee754dp x, union ieee754dp y)
> diff --git a/arch/mips/math-emu/dp_fmin.c b/arch/mips/math-emu/dp_fmin.c
> index 7069320..074a858 100644
> --- a/arch/mips/math-emu/dp_fmin.c
> +++ b/arch/mips/math-emu/dp_fmin.c
> @@ -106,16 +106,33 @@ union ieee754dp ieee754dp_fmin(union ieee754dp x, union ieee754dp y)
> else if (xs < ys)
> return y;
>
> - /* Compare exponent */
> - if (xe > ye)
> - return y;
> - else if (xe < ye)
> - return x;
> + /* Signs of inputs are the same, let's compare exponents */
> + if (xs == 0) {
> + /* Inputs are both positive */
> + if (xe > ye)
> + return y;
> + else if (xe < ye)
> + return x;
> + } else {
> + /* Inputs are both negative */
> + if (xe > ye)
> + return x;
> + else if (xe < ye)
> + return y;
> + }
>
> - /* Compare mantissa */
> - if (xm <= ym)
> + /* Signs and exponents of inputs are the same, let's compare mantissas */
> + if (xs == 0) {
> + /* Inputs are both positive, with equal exponents */
> + if (xm <= ym)
> + return x;
> + return y;
> + } else {
> + /* Inputs are both negative, with equal exponents */
> + if (xm <= ym)
> + return y;
> return x;
> - return y;
> + }
> }
>
> union ieee754dp ieee754dp_fmina(union ieee754dp x, union ieee754dp y)
> diff --git a/arch/mips/math-emu/sp_fmax.c b/arch/mips/math-emu/sp_fmax.c
> index d72111a..15825db 100644
> --- a/arch/mips/math-emu/sp_fmax.c
> +++ b/arch/mips/math-emu/sp_fmax.c
> @@ -106,16 +106,33 @@ union ieee754sp ieee754sp_fmax(union ieee754sp x, union ieee754sp y)
> else if (xs < ys)
> return x;
>
> - /* Compare exponent */
> - if (xe > ye)
> - return x;
> - else if (xe < ye)
> - return y;
> + /* Signs of inputs are the same, let's compare exponents */
> + if (xs == 0) {
> + /* Inputs are both positive */
> + if (xe > ye)
> + return x;
> + else if (xe < ye)
> + return y;
> + } else {
> + /* Inputs are both negative */
> + if (xe > ye)
> + return y;
> + else if (xe < ye)
> + return x;
> + }
>
> - /* Compare mantissa */
> - if (xm <= ym)
> + /* Signs and exponents of inputs are the same, let's compare mantissas */
> + if (xs == 0) {
> + /* Inputs are both positive, with equal exponents */
> + if (xm <= ym)
> + return y;
> + return x;
> + } else {
> + /* Inputs are both negative, with equal exponents */
> + if (xm <= ym)
> + return x;
> return y;
> - return x;
> + }
> }
>
> union ieee754sp ieee754sp_fmaxa(union ieee754sp x, union ieee754sp y)
> diff --git a/arch/mips/math-emu/sp_fmin.c b/arch/mips/math-emu/sp_fmin.c
> index 61ff9c6..f1418f7 100644
> --- a/arch/mips/math-emu/sp_fmin.c
> +++ b/arch/mips/math-emu/sp_fmin.c
> @@ -106,16 +106,34 @@ union ieee754sp ieee754sp_fmin(union ieee754sp x, union ieee754sp y)
> else if (xs < ys)
> return y;
>
> - /* Compare exponent */
> - if (xe > ye)
> + /* Signs of inputs are the same, let's compare exponents */
> + if (xs == 0) {
> + /* Inputs are both positive */
> + if (xe > ye)
> + return y;
> + else if (xe < ye)
> + return x;
> + } else {
> + /* Inputs are both negative */
> + if (xe > ye)
> + return x;
> + else if (xe < ye)
> + return y;
> + }
> +
> + /* Signs and exponents of inputs are the same, let's compare mantissas */
> + if (xs == 0) {
> + /* Inputs are both positive, with equal exponents */
> + if (xm <= ym)
> + return x;
> return y;
> - else if (xe < ye)
> + } else {
> + /* Inputs are both negative, with equal exponents */
> + if (xm <= ym)
> + return y;
> return x;
> + }
>
> - /* Compare mantissa */
> - if (xm <= ym)
> - return x;
> - return y;
> }
>
> union ieee754sp ieee754sp_fmina(union ieee754sp x, union ieee754sp y)
> --
> 2.7.4
>

Attachment: signature.asc
Description: Digital signature