Keep the intermediate results in FloatParts instead of
converting back and forth between float64.  Use muladd
instead of separate mul+add.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 53 +++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 30 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index c32b1c7113..27306d6a93 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5210,47 +5210,40 @@ static const float64 float32_exp2_coefficients[15] =
 
 float32 float32_exp2(float32 a, float_status *status)
 {
-    bool aSign;
-    int aExp;
-    uint32_t aSig;
-    float64 r, x, xn;
+    FloatParts64 xp, xnp, tp, rp;
     int i;
-    a = float32_squash_input_denormal(a, status);
 
-    aSig = extractFloat32Frac( a );
-    aExp = extractFloat32Exp( a );
-    aSign = extractFloat32Sign( a );
-
-    if ( aExp == 0xFF) {
-        if (aSig) {
-            return propagateFloat32NaN(a, float32_zero, status);
+    float32_unpack_canonical(&xp, a, status);
+    if (unlikely(xp.cls != float_class_normal)) {
+        switch (xp.cls) {
+        case float_class_snan:
+        case float_class_qnan:
+            parts_return_nan(&xp, status);
+            return float32_round_pack_canonical(&xp, status);
+        case float_class_inf:
+            return xp.sign ? float32_zero : a;
+        case float_class_zero:
+            return float32_one;
+        default:
+            break;
         }
-        return (aSign) ? float32_zero : a;
-    }
-    if (aExp == 0) {
-        if (aSig == 0) return float32_one;
+        g_assert_not_reached();
     }
 
     float_raise(float_flag_inexact, status);
 
-    /* ******************************* */
-    /* using float64 for approximation */
-    /* ******************************* */
-    x = float32_to_float64(a, status);
-    x = float64_mul(x, float64_ln2, status);
+    float64_unpack_canonical(&xnp, float64_ln2, status);
+    xp = *parts_mul(&xp, &tp, status);
+    xnp = xp;
 
-    xn = x;
-    r = float64_one;
+    float64_unpack_canonical(&rp, float64_one, status);
     for (i = 0 ; i < 15 ; i++) {
-        float64 f;
-
-        f = float64_mul(xn, float32_exp2_coefficients[i], status);
-        r = float64_add(r, f, status);
-
-        xn = float64_mul(xn, x, status);
+        float64_unpack_canonical(&tp, float32_exp2_coefficients[i], status);
+        rp = *parts_muladd(&tp, &xp, &rp, 0, status);
+        xnp = *parts_mul(&xnp, &xp, status);
     }
 
-    return float64_to_float32(r, status);
+    return float32_round_pack_canonical(&rp, status);
 }
 
 /*----------------------------------------------------------------------------
-- 
2.25.1

On Thu, 3 Jun 2021 at 22:58, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Keep the intermediate results in FloatParts instead of
> converting back and forth between float64.  Use muladd
> instead of separate mul+add.
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 53 +++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index c32b1c7113..27306d6a93 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -5210,47 +5210,40 @@ static const float64 float32_exp2_coefficients[15] =
>
>  float32 float32_exp2(float32 a, float_status *status)
>  {
> -    bool aSign;
> -    int aExp;
> -    uint32_t aSig;
> -    float64 r, x, xn;
> +    FloatParts64 xp, xnp, tp, rp;

Coverity points out that we declare tp here without initializing it...

>      int i;
> -    a = float32_squash_input_denormal(a, status);
>
> -    aSig = extractFloat32Frac( a );
> -    aExp = extractFloat32Exp( a );
> -    aSign = extractFloat32Sign( a );
> -
> -    if ( aExp == 0xFF) {
> -        if (aSig) {
> -            return propagateFloat32NaN(a, float32_zero, status);
> +    float32_unpack_canonical(&xp, a, status);
> +    if (unlikely(xp.cls != float_class_normal)) {
> +        switch (xp.cls) {
> +        case float_class_snan:
> +        case float_class_qnan:
> +            parts_return_nan(&xp, status);
> +            return float32_round_pack_canonical(&xp, status);
> +        case float_class_inf:
> +            return xp.sign ? float32_zero : a;
> +        case float_class_zero:
> +            return float32_one;
> +        default:
> +            break;
>          }
> -        return (aSign) ? float32_zero : a;
> -    }
> -    if (aExp == 0) {
> -        if (aSig == 0) return float32_one;
> +        g_assert_not_reached();
>      }
>
>      float_raise(float_flag_inexact, status);
>
> -    /* ******************************* */
> -    /* using float64 for approximation */
> -    /* ******************************* */
> -    x = float32_to_float64(a, status);
> -    x = float64_mul(x, float64_ln2, status);
> +    float64_unpack_canonical(&xnp, float64_ln2, status);
> +    xp = *parts_mul(&xp, &tp, status);

...and then here we pass &tp to parts_mul() which looks at
its various fields. Missing initializer of some sort ?

(CID 1457457)

thanks
-- PMM

On 6/7/21 2:07 PM, Peter Maydell wrote:
>> +    FloatParts64 xp, xnp, tp, rp;
> 
> Coverity points out that we declare tp here without initializing it...
...
>> +    float64_unpack_canonical(&xnp, float64_ln2, status);
>> +    xp = *parts_mul(&xp, &tp, status);
> 
> ...and then here we pass &tp to parts_mul() which looks at
> its various fields. Missing initializer of some sort ?

Typo in the unpack; should have been tp.


r~