From 3f2c423686e735ae4c8529b1df018d9884f48ce2 Mon Sep 17 00:00:00 2001 From: Damien George Date: Tue, 5 Dec 2023 16:38:44 +1100 Subject: [PATCH] rp2: Switch to locally provided math library. This commit fixes all known floating-point bugs with the pico-sdk. There are two things going on here: - Use a custom pico float component so that the pico-sdk doesn't include its math functions, and then provide our own from lib/libm. - Provide a wrapper for __aeabi_fadd to fix the infinity addition bug. Prior to this commit, the following tests failed on the rp2 port: cmath_fun float_parse math_domain math_domain_special math_fun_special. With this commit, all these tests pass. Thanks to @projectgus for how to approach this fix. Signed-off-by: Damien George --- ports/rp2/CMakeLists.txt | 67 ++++++++++++++++++++++++++++++++++++++ ports/rp2/libm_extra.c | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+) create mode 100644 ports/rp2/libm_extra.c diff --git a/ports/rp2/CMakeLists.txt b/ports/rp2/CMakeLists.txt index dcf1328724..c68c46cb69 100644 --- a/ports/rp2/CMakeLists.txt +++ b/ports/rp2/CMakeLists.txt @@ -196,6 +196,73 @@ set(PICO_SDK_COMPONENTS tinyusb_device ) +# Use our custom pico_float_micropython float implementation. This is needed for two reasons: +# - to fix inf handling in pico-sdk's __wrap___aeabi_fadd(); +# - so we can use our own libm functions, to fix inaccuracies in the pico-sdk versions. +pico_set_float_implementation(${MICROPY_TARGET} micropython) + +# Define our custom pico_float_micropython component. +pico_add_library(pico_float_micropython) + +# pico_float_micropython: add pico-sdk float and our libm source files. +target_sources(pico_float_micropython INTERFACE + ${PICO_SDK_PATH}/src/rp2_common/pico_float/float_aeabi.S + ${PICO_SDK_PATH}/src/rp2_common/pico_float/float_init_rom.c + ${PICO_SDK_PATH}/src/rp2_common/pico_float/float_v1_rom_shim.S + ${MICROPY_DIR}/lib/libm/math.c + ${MICROPY_DIR}/lib/libm/acoshf.c + ${MICROPY_DIR}/lib/libm/asinfacosf.c + ${MICROPY_DIR}/lib/libm/asinhf.c + ${MICROPY_DIR}/lib/libm/atan2f.c + ${MICROPY_DIR}/lib/libm/atanf.c + ${MICROPY_DIR}/lib/libm/atanhf.c + ${MICROPY_DIR}/lib/libm/ef_rem_pio2.c + ${MICROPY_DIR}/lib/libm/ef_sqrt.c + ${MICROPY_DIR}/lib/libm/erf_lgamma.c + ${MICROPY_DIR}/lib/libm/fmodf.c + ${MICROPY_DIR}/lib/libm/kf_cos.c + ${MICROPY_DIR}/lib/libm/kf_rem_pio2.c + ${MICROPY_DIR}/lib/libm/kf_sin.c + ${MICROPY_DIR}/lib/libm/kf_tan.c + ${MICROPY_DIR}/lib/libm/log1pf.c + ${MICROPY_DIR}/lib/libm/nearbyintf.c + ${MICROPY_DIR}/lib/libm/roundf.c + ${MICROPY_DIR}/lib/libm/sf_cos.c + ${MICROPY_DIR}/lib/libm/sf_erf.c + ${MICROPY_DIR}/lib/libm/sf_frexp.c + ${MICROPY_DIR}/lib/libm/sf_ldexp.c + ${MICROPY_DIR}/lib/libm/sf_modf.c + ${MICROPY_DIR}/lib/libm/sf_sin.c + ${MICROPY_DIR}/lib/libm/sf_tan.c + ${MICROPY_DIR}/lib/libm/wf_lgamma.c + ${MICROPY_DIR}/lib/libm/wf_tgamma.c + ${MICROPY_PORT_DIR}/libm_extra.c +) + +# pico_float_micropython: wrap low-level floating-point ops, to call the pico-sdk versions. +pico_wrap_function(pico_float_micropython __aeabi_fdiv) +pico_wrap_function(pico_float_micropython __aeabi_fmul) +pico_wrap_function(pico_float_micropython __aeabi_frsub) +pico_wrap_function(pico_float_micropython __aeabi_fsub) +pico_wrap_function(pico_float_micropython __aeabi_cfcmpeq) +pico_wrap_function(pico_float_micropython __aeabi_cfrcmple) +pico_wrap_function(pico_float_micropython __aeabi_cfcmple) +pico_wrap_function(pico_float_micropython __aeabi_fcmpeq) +pico_wrap_function(pico_float_micropython __aeabi_fcmplt) +pico_wrap_function(pico_float_micropython __aeabi_fcmple) +pico_wrap_function(pico_float_micropython __aeabi_fcmpge) +pico_wrap_function(pico_float_micropython __aeabi_fcmpgt) +pico_wrap_function(pico_float_micropython __aeabi_fcmpun) +pico_wrap_function(pico_float_micropython __aeabi_i2f) +pico_wrap_function(pico_float_micropython __aeabi_l2f) +pico_wrap_function(pico_float_micropython __aeabi_ui2f) +pico_wrap_function(pico_float_micropython __aeabi_ul2f) +pico_wrap_function(pico_float_micropython __aeabi_f2iz) +pico_wrap_function(pico_float_micropython __aeabi_f2lz) +pico_wrap_function(pico_float_micropython __aeabi_f2uiz) +pico_wrap_function(pico_float_micropython __aeabi_f2ulz) +pico_wrap_function(pico_float_micropython __aeabi_f2d) + if (MICROPY_PY_LWIP) target_link_libraries(${MICROPY_TARGET} micropy_lib_lwip) diff --git a/ports/rp2/libm_extra.c b/ports/rp2/libm_extra.c new file mode 100644 index 0000000000..4628fb0834 --- /dev/null +++ b/ports/rp2/libm_extra.c @@ -0,0 +1,69 @@ +/* + * This file is part of the MicroPython project, http://micropython.org/ + * + * The MIT License (MIT) + * + * Copyright (c) 2023 Damien P. George + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include +#include + +#define INF_POS (0x7f800000) +#define INF_NEG (0xff800000) + +union float_int_t { + float f; + uint32_t i; +}; + +float __wrap___aeabi_fadd(float x, float y); + +// The pico-sdk wraps __aeabi_fadd() with __wrap___aeabi_fadd() in order to use the ROM +// floating-point functions. But __wrap___aeabi_fadd() does not handle inf+(-inf) or +// (-inf)+inf correctly. To fix this we provide our own __aeabi_fadd() that fixes the +// inf calculation, and do not use the "--wrap=" linker command. The compiler then +// picks our __aeabi_fadd() instead of its built-in one. And the pico-sdk function +// still exists for us to call. +float __aeabi_fadd(float x, float y) { + // Handle addition of inf/-inf. This is optimised to reduce C stack usage, and + // only have one comparison/jump in the common case of x != inf. + union float_int_t xu = {.f = x}; + union float_int_t yu = {.f = y}; + if ((xu.i << 1) == (INF_POS << 1)) { + if (xu.i == INF_POS) { + if (yu.i == INF_POS) { + return INFINITY; + } else if (yu.i == INF_NEG) { + return NAN; + } + } else { + if (yu.i == INF_POS) { + return NAN; + } else if (yu.i == INF_NEG) { + return -INFINITY; + } + } + } + + // Use the pico-sdk function for all other calculations. + return __wrap___aeabi_fadd(x, y); +}