libm/libm: Do not force floating point type size evaluation.

[agatti]

MicroPython's vendored libm and libm_dbl definition of FLT_EVAL_METHOD break compilation on embedded targets that use picolibc as their libc provider (see discussion: #12853 (comment)).

One case where this is needed is the RISC-V toolchain available on the CI image. Going forward, other toolchains may prefer to use picolibc instead of newlib, and users may want to be able to use said library with MicroPython.

Starting from C99 FLT_EVAL_METHOD should be left to the compiler/libc to define, according to the appropriate floating point types' representation for the platform. Choosing floating point types representations should be done via compiler options rather than with #defines.

Tests pass on both qemu-arm and qemu-riscv with this change applied. Though I saw no build breakages for ports/boards that used either libm or libm_dbl, I cannot tell whether this PR breaks code on hardware platforms I have no access to.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (3af006e) to head (d8b4bf4).

❗ Current head d8b4bf4 differs from pull request most recent head 3613ad9

Please upload reports for the commit 3613ad9 to get more accurate results.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #14456   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21204    21204           
=======================================
  Hits        20864    20864           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[projectgus]

Starting from C99 FLT_EVAL_METHOD should be left to the compiler/libc to define, according to the appropriate floating point types' representation for the platform. Choosing floating point types representations should be done via compiler options rather than with #defines.

As I understand it, the maybe-tricky part here is that MicroPython libm provides a bunch of optimised math routines that rely on a particular binary representation of floats. So if FLT_EVAL_METHOD != 0 then probably float_t != float and possibly double_t != double and therefore MicroPython's libm may produce garbage results on that compiler.

To keep a check for this, I think it might be enough to insert this into both libm headers where the define previously was:

// These lines verify that FLT_EVAL_METHOD==0, MicroPython's libm requires this.
// If compilation fails here then check the host compiler's FLT_EVAL_METHOD.
typedef float float_t;
typedef double double_t;

Redundant (compatible) typedef declarations are allowed, so provided math.h makes matching definitions then this should compile. If a particular platform is configured differently, math.h should have defined float_t or double_t to something else so a compiler error should appear at this point. I think it's preferable to (maybe) compiling and producing garbage results

That said, I don't know if MicroPython supports any build configurations where this would fail.

[agatti]

@projectgus, I've looked a bit into it as well.

On RISC-V I believe FLT_EVAL_METHOD may change depending on the combination of architecture and abi being passed to GCC via -march and -mabi. If my understanding is correct (and please do not quote me :)), if GCC is asked to build for a CPU implementing a combination of "D", "Q", or "Zfh"/"Zfhmin" extensions (double, quad, half precision) but not the "F" extension (single precision) then MicroPython's libm would not work as either floats or doubles will be aliased to something else. Same goes for libm_dbl on a CPU that only implements "F", "Q", or "Zfh"/"Zfhmin" but not "D", doubles will be aliased to floats.

That alone makes your point valid, I guess.

I haven't looked much into how Arm handles this, but for example what happens if you use libm_dbl targetting a Cortex-M4F (whose FPU only supports single-precision floats) passing -mfloat-abi=hard? No idea whether the compiler will still use soft-float code when using doubles or will just alias doubles to floats.

That said, I'll update the patch following your suggestion, thanks!

[projectgus]

@agatti These are good questions that I don't know the answers to! At minimum, I think these changes won't make anything not work that previously would have worked...