Re: [PULL 08/13] softfloat: Fix BAD_SHIFT from normalizeFloatx80Subnormal

[Aleksandar Markovic]

17:55 Uto, 07.04.2020. Alex Bennée <address@hidden> је написао/ла:
>
> From: Richard Henderson <address@hidden>
>
> All other calls to normalize*Subnormal detect zero input before
> the call -- this is the only outlier.  This case can happen with
> +0.0 + +0.0 = +0.0 or -0.0 + -0.0 = -0.0, so return a zero of
> the correct sign.
>
> Reported-by: Coverity (CID 1421991)
> Signed-off-by: Richard Henderson <address@hidden>
> Signed-off-by: Alex Bennée <address@hidden>
> Message-Id: <address@hidden>
> Message-Id: <address@hidden>
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 301ce3b537b..ae6ba718540 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -5856,6 +5856,9 @@ static floatx80 addFloatx80Sigs(floatx80 a, floatx80 b, flag zSign,
>          zSig1 = 0;
>          zSig0 = aSig + bSig;
>          if ( aExp == 0 ) {
> +            if (zSig0 == 0) {
> +                return packFloatx80(zSign, 0, 0);
> +            }
>              normalizeFloatx80Subnormal( zSig0, &zExp, &zSig0 );
>              goto roundAndPack;
>          }
> --
> 2.20.1
>
>

We in MIPS have extensive FP tests, that certainly include many cases of operations with +0 and -0. And they are all correct even before this patch.

Unfortunately, because of my current remote work, I don't havecthese tests with me, and can't confirm if they work correctly, or perhaps are unaffected at all.

Alex, from the commit message, it not clear if this is a fix of a bug (in which case a test example would be useful to have, and the assesment on what scenarios could be affected), or just a correction for some rare condition that practically for all intents and purposes was never triggered, or perhaps something third.

Alex, please explain this in more detail to me.

Secondly, and not related to this patch only, I see more and more patches integrated into the main tree without "Reviewed-by:" tag. I don't think this is the best way an open source community works. In my personal opinion, this must stop.

Regards,
Aleksandar