softfloat: Fix the incorrect computation in float32_exp2()

[PATCH] softfloat: Fix the incorrect computation in float32_exp2()

Posted by Shivaprasad G Bhat 1 year ago

The float32_exp2() is computing wrong exponent of 2.
For example, with the following set of values {0.1, 2.0, 2.0, -1.0},
the expected output would be {1.071773, 4.000000, 4.000000, 0.500000}.
Instead, the function is computing {1.119102, 3.382044, 3.382044, -0.191022}

Looking at the code, the float32_exp2() attempts to do this

                  2     3     4     5           n
  x        x     x     x     x     x           x
 e  = 1 + --- + --- + --- + --- + --- + ... + --- + ...
           1!    2!    3!    4!    5!          n!

But because of the 'typo'/bug it ends up doing

 x        x     x     x     x     x           x
e  = 1 + --- + --- + --- + --- + --- + ... + --- + ...
          1!    2!    3!    4!    5!          n!

This is because instead of the xnp which holds the numerator,
parts_muladd is using the xp which is just 'x'. The commit '572c4d862ff2'
refactored this function, and it seems mistakenly using xp instead of xnp.

The patches fixes this possible typo.

Fixes: 572c4d862ff2 "softfloat: Convert float32_exp2 to FloatParts"
Partially-Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1623
Reported-By: Luca Barbato (https://gitlab.com/lu-zero)
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhat@linux.ibm.com>
---
 fpu/softfloat.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index c7454c3eb1a..108f9cb224a 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5135,7 +5135,7 @@ float32 float32_exp2(float32 a, float_status *status)
     float64_unpack_canonical(&rp, float64_one, status);
     for (i = 0 ; i < 15 ; i++) {
         float64_unpack_canonical(&tp, float32_exp2_coefficients[i], status);
-        rp = *parts_muladd(&tp, &xp, &rp, 0, status);
+        rp = *parts_muladd(&tp, &xnp, &rp, 0, status);
         xnp = *parts_mul(&xnp, &xp, status);
     }

Re: [PATCH] softfloat: Fix the incorrect computation in float32_exp2()

Posted by Michael Tokarev 1 year ago

02.05.2023 18:25, Shivaprasad G Bhat wrote:
> The float32_exp2() is computing wrong exponent of 2.
> For example, with the following set of values {0.1, 2.0, 2.0, -1.0},
> the expected output would be {1.071773, 4.000000, 4.000000, 0.500000}.
> Instead, the function is computing {1.119102, 3.382044, 3.382044, -0.191022}
> 
> Looking at the code, the float32_exp2() attempts to do this
> 
>                    2     3     4     5           n
>    x        x     x     x     x     x           x
>   e  = 1 + --- + --- + --- + --- + --- + ... + --- + ...
>             1!    2!    3!    4!    5!          n!
> 
> But because of the 'typo'/bug it ends up doing
> 
>   x        x     x     x     x     x           x
> e  = 1 + --- + --- + --- + --- + --- + ... + --- + ...
>            1!    2!    3!    4!    5!          n!
> 
> This is because instead of the xnp which holds the numerator,
> parts_muladd is using the xp which is just 'x'. The commit '572c4d862ff2'
> refactored this function, and it seems mistakenly using xp instead of xnp.
> 
> The patches fixes this possible typo.

This smells like a -stable material.

/mjt

Re: [PATCH] softfloat: Fix the incorrect computation in float32_exp2()

Posted by Richard Henderson 1 year ago

On 5/2/23 16:25, Shivaprasad G Bhat wrote:
> The float32_exp2() is computing wrong exponent of 2.
> For example, with the following set of values {0.1, 2.0, 2.0, -1.0},
> the expected output would be {1.071773, 4.000000, 4.000000, 0.500000}.
> Instead, the function is computing {1.119102, 3.382044, 3.382044, -0.191022}
> 
> Looking at the code, the float32_exp2() attempts to do this
> 
>                    2     3     4     5           n
>    x        x     x     x     x     x           x
>   e  = 1 + --- + --- + --- + --- + --- + ... + --- + ...
>             1!    2!    3!    4!    5!          n!
> 
> But because of the 'typo'/bug it ends up doing
> 
>   x        x     x     x     x     x           x
> e  = 1 + --- + --- + --- + --- + --- + ... + --- + ...
>            1!    2!    3!    4!    5!          n!
> 
> This is because instead of the xnp which holds the numerator,
> parts_muladd is using the xp which is just 'x'. The commit '572c4d862ff2'
> refactored this function, and it seems mistakenly using xp instead of xnp.
> 
> The patches fixes this possible typo.
> 
> Fixes: 572c4d862ff2 "softfloat: Convert float32_exp2 to FloatParts"
> Partially-Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1623
> Reported-By: Luca Barbato (https://gitlab.com/lu-zero)
> Signed-off-by: Shivaprasad G Bhat<sbhat@linux.ibm.com>
> Signed-off-by: Vaibhav Jain<vaibhat@linux.ibm.com>
> ---
>   fpu/softfloat.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Whoops.  Good catch.

r~

Re: [PATCH] softfloat: Fix the incorrect computation in float32_exp2()

Posted by Shivaprasad G Bhat 1 year ago

Hi Richard,


On 5/3/23 01:11, Richard Henderson wrote:
> On 5/2/23 16:25, Shivaprasad G Bhat wrote:
>> The float32_exp2() is computing wrong exponent of 2.
>> For example, with the following set of values {0.1, 2.0, 2.0, -1.0},
>> the expected output would be {1.071773, 4.000000, 4.000000, 0.500000}.
>> Instead, the function is computing {1.119102, 3.382044, 3.382044, 
>> -0.191022}
>>
<snip>
>> his is because instead of the xnp which holds the numerator,
>> parts_muladd is using the xp which is just 'x'. The commit 
>> '572c4d862ff2'
>> refactored this function, and it seems mistakenly using xp instead of 
>> xnp.
>>
>> The patches fixes this possible typo.
>>
>> Fixes: 572c4d862ff2 "softfloat: Convert float32_exp2 to FloatParts"
>> Partially-Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1623
>> Reported-By: Luca Barbato (https://gitlab.com/lu-zero)
>> Signed-off-by: Shivaprasad G Bhat<sbhat@linux.ibm.com>
>> Signed-off-by: Vaibhav Jain<vaibhat@linux.ibm.com>
>> ---
>>   fpu/softfloat.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> Whoops.  Good catch.
>

If you are fine with the patch, could you fix the mail id for Vaibhav 
Jain as

  <vaibhav@linux.ibm.com> while pulling ?

If you have other comments, I will fix it in the next version otherwise.


Thanks,

Shivaprasad


H

> r~

Re: [PATCH] softfloat: Fix the incorrect computation in float32_exp2()

Posted by Richard Henderson 1 year ago

On 5/4/23 06:21, Shivaprasad G Bhat wrote:
> Hi Richard,
> 
> 
> On 5/3/23 01:11, Richard Henderson wrote:
>> On 5/2/23 16:25, Shivaprasad G Bhat wrote:
>>> The float32_exp2() is computing wrong exponent of 2.
>>> For example, with the following set of values {0.1, 2.0, 2.0, -1.0},
>>> the expected output would be {1.071773, 4.000000, 4.000000, 0.500000}.
>>> Instead, the function is computing {1.119102, 3.382044, 3.382044, -0.191022}
>>>
> <snip>
>>> his is because instead of the xnp which holds the numerator,
>>> parts_muladd is using the xp which is just 'x'. The commit '572c4d862ff2'
>>> refactored this function, and it seems mistakenly using xp instead of xnp.
>>>
>>> The patches fixes this possible typo.
>>>
>>> Fixes: 572c4d862ff2 "softfloat: Convert float32_exp2 to FloatParts"
>>> Partially-Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1623
>>> Reported-By: Luca Barbato (https://gitlab.com/lu-zero)
>>> Signed-off-by: Shivaprasad G Bhat<sbhat@linux.ibm.com>
>>> Signed-off-by: Vaibhav Jain<vaibhat@linux.ibm.com>
>>> ---
>>>   fpu/softfloat.c |    2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Whoops.  Good catch.
>>
> 
> If you are fine with the patch, could you fix the mail id for Vaibhav Jain as
> 
>   <vaibhav@linux.ibm.com> while pulling ?

Done.  Queued to tcg-next.

r~