Re: [PATCH] math: remove exp10 wrappers

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 10/01/24 18:35, Wilco Dijkstra wrote:
> Hi Adhemerval,
> 
>> As a side node, I am checking if it is possible to remove the i686 version
>> in favor of the generic one and it seems that the double_t usage on generic
>> implementation does not guarantee either a faster or conformant implementation
>> (i386 uses FLT_EVAL_METHOD=2 and the resulting exp10 shows a lot precision
>> issues with math tests).  Does it really need to use double_t?
> 
> Yes it would be great to remove target specific math implementations. With IA64
> removed I think the only ones left are x86 and m68k. Both would be better off using
> the generic math code.
> 
> We added double_t to use wider internal precision without storing to memory all the
> time. In principle it should be faster overall. Exp10 uses double_t like exp and exp2,
> so I'm surprised it doesn't work. I do notice both exp/exp2 do ki = asuint64 (kd);
> while exp10 just does ki = kd, but it's not clear that would make a difference on x86.
> 

The patch below fixes the numerical stability I found on i686:

diff --git a/sysdeps/ieee754/dbl-64/e_exp10.c b/sysdeps/ieee754/dbl-64/e_exp10.c
index 225fc74c4c..f961d58398 100644
--- a/sysdeps/ieee754/dbl-64/e_exp10.c
+++ b/sysdeps/ieee754/dbl-64/e_exp10.c
@@ -99,7 +99,7 @@ __exp10 (double x)

   /* Reduce x: z = x * N / log10(2), k = round(z).  */
   double_t z = __exp_data.invlog10_2N * x;
-  double_t kd;
+  double kd;
   int64_t ki;
 #if TOINT_INTRINSICS
   kd = roundtoint (z);

And I think we haven't see it because all ABIs with FLT_EVAL_METHOD=2 uses
assembly implementations for exp/exp2/exp10 (this is the case for i386
and m68k).

Furthermore, at least with i686 the use of double_t also does not show 
better performance either. The current sysdeps/i386/fpu/e_exp10.S
on a recent CPU (Ryzen 9 5900X) with gcc version 13.2.1:

$ ./benchtests/bench-exp10
  "exp10": {
   "": {
    "duration": 3.71309e+09,
    "iterations": 5.8534e+07,
    "max": 116.55,
    "min": 62.974,
    "mean": 63.4348
   }

The fixed version using double_t:

$ ./benchtests/bench-exp10
  "exp10": {
   "": {
    "duration": 3.71002e+09,
    "iterations": 6.5653e+07,
    "max": 130.499,
    "min": 50.282,
    "mean": 56.5096
   }

And just using double instead of double_t:

$ ./benchtests/bench-exp10
  "exp10": {
   "": {
    "duration": 3.71803e+09,
    "iterations": 7.3563e+07,
    "max": 123.062,
    "min": 43.549,
    "mean": 50.5421
   }
  }

So I am not sure if double_t is really helpful here.