rationalize pico_float/pico_double libraries #2208

kilograham · 2025-01-23T22:45:46Z

on RP2350 _dcp variant now enables -msoft-float, since if you're using this at all it is likely because you don't want to use the VFP unit at all (to save stack space)
implement all float_ and double_ conversion functions in all pico_float_pico_ variants and pico_double_pico on RP2040 and RP2350 (many were missing in some combinations)
provide better granularity of what functions are wrapped in each case

* on RP2350 _dcp variant now enables -msoft-float, since if you're using this at all it is likely because you don't want to use the VFP unit at all (to save stack space) * implement all float_ and double_ conversion functions in all pico_float_pico_ variants and pico_double_pico on RP2040 and RP2350 (many were missing in some combinations) * provide better granularity of what functions are wrapped in each case

kilograham · 2025-01-23T22:46:07Z

fixes #2160

kilograham · 2025-01-23T22:49:46Z

cc @armandomontanez - i fixed up the main bazel build, but did not attempt to add the new tests

lurch · 2025-01-24T11:32:25Z

src/rp2_common/pico_double/include/pico/double.h

+* On Arm, (replacement) optimized implementations are provided for the following compiler built-ins when
+* and math library functions when using `pico_dobule_pico`:


Some kind of copy-paste typo? "compiler built-ins when and math library functions"

lurch · 2025-01-24T11:36:00Z

src/rp2_common/pico_double/include/pico/double.h

+*     note: on `pico_double_vfp` the 32-bit functions are curretly _only_ provided as C macros and must use a compile
+*     time constant between 1 and 32 for the fixed point position
+*
+* - Even faster methods versions of divide and square-root that do not round correctly:


"methods versions" doesn't sound right?

lurch · 2025-01-24T11:38:54Z

src/rp2_common/pico_double/include/pico/double.h

-double mla(double x, double y, double z); // note this is not fused
+double sqrt_fast(double f);
+double fma_fast(double x, double y, double z); // this is not fused
+double mla(double x, double y, double z); // another name for fma_flast


Was the fma_flast here supposed to say fma_fast ?

lurch · 2025-01-24T11:47:27Z

src/rp2_common/pico_float/include/pico/float.h

-* - __addsf3, __subsf3, __mulsf3
+* 1. `pico_float_pico_vfp` - this library leaves basic C single-precision floating point operations to the compiler
+* which can use inlined VFP (Arm FPU) code. Custom optimized versions of trigonometric and scientific functions are provided.
+* no DCP (RP2350 Double co-processor) instructions are used.


nit: capital N on no here?

lurch · 2025-01-24T11:48:38Z

src/rp2_common/pico_float/include/pico/float.h

+* no DCP (RP2350 Double co-processor) instructions are used.
+* 2. `pico_float_pico_dcp` - this library prevents the compiler injecting inlined VFP code, and also implements
+* all single-precision floating point operations in optimized DCP or M33 code. This option is not as fast
+* as pico_float_pico_vfp, however allows floating point operations without enabling the floating point co-processor


"however allows" -> "however it allows" ?

lurch · 2025-01-24T11:49:06Z

src/rp2_common/pico_float/include/pico/float.h

+* on the CPU; this can be beneficial in certain circumstances, e.g. where leaving stack in tasks or interrupts
+* for the floating point state is undesirable.
+*
+* Note: `pico_float_pico` is equivalent ot `pico_flot_pico_vfp` on RP2350, as this is the most sensible default


"ot" -> "to"

Also flot -> float ? 😉

lurch · 2025-01-24T11:51:35Z

src/rp2_common/pico_float/include/pico/float.h

+* Note: `pico_float_pico` is equivalent ot `pico_flot_pico_vfp` on RP2350, as this is the most sensible default
+* \endif
+*
+* On Arm, (replacement) optimized implementations are provided for the following compiler built-ins when


Same "when and" typo as earlier.

lurch · 2025-01-24T11:53:43Z

src/rp2_common/pico_float/include/pico/float.h

+*     note: on `pico_float_vfp` the 32-bit functions are also provided as C macros since they can map to inline VFP code
+*     when the number of fractional bits is a compile time constant between 1 and 32
+*
+* - Even faster methods versions of divide and square-root that do not round correctly: (`pico_float_pico_dcp` only)


"methods versions" again

lurch · 2025-01-24T11:56:13Z

src/rp2_common/pico_float/include/pico/float.h

+#define _float2ufix_inline(f, e) _float2ufix_z_inline((f), (e))
+#endif
+
+#if LIB_PIC_FLOAT_PICO_VFP


Was this supposed to be LIB_PICO_FLOAT_PICO_VFP ?

lurch · 2025-01-24T11:59:11Z

test/pico_float_test/CMakeLists.txt

+
+set(FLOAT_TYPES compiler)
+set(DOUBLE_TYPES compiler)
+#if (PICO_RP2040)


Why the commented-out if() here? Would it make sense to remove it?

lurch · 2025-01-24T12:02:43Z

src/rp2_common/pico_float/include/pico/float.h

+*
+*     int2float, uint2float, int642float, uint642float
+*
+*     note: on `pico_float_vfp` the 32-bit functions are also provided as C macros since they map to inline VFP code


Are the mentions of pico_float_vfp here (and elsewhere) supposed to be pico_float_pico_vfp ? 🤷

lurch · 2025-01-24T12:05:41Z

src/rp2_common/pico_double/include/pico/double.h

+*
+*       fix2double, ufix2double, fix642double, ufix642double
+*
+*     note: on `pico_double_vfp` the 32-bit functions are curretly _only_ provided as C macros and must use a compile


"curretly" -> "currently"
(and also on lines 123 and 130)

lurch · 2025-01-24T12:08:49Z

src/rp2_common/pico_double/include/pico/double.h

+*
+*   __aeabi_dadd, __aeabi_ddiv, __aeabi_dmul, __aeabi_drsub, __aeabi_dsub
+*
+* - comparison: (except `pico_double_pico_vfp`)


This is the only mention of pico_double_pico_vfp in this PR, and pico_double_vfp also appears 5 times. I've no idea which version is correct! (Perhaps they both are?)

lurch · 2025-01-24T12:10:01Z

src/rp2_common/pico_double/include/pico/double.h

-* (Replacement) optimized implementations are provided of the following compiler built-ins
-* and math library functions:
+* An application can take control of the floating point routines used in the application over and above what is provided by the compiler,
+* by depending on the pico_double library. A user might want to do this


nit: Should probably be a colon at the end of this line? (and same in float.h)

kilograham added this to the 2.1.1 milestone Jan 23, 2025

lurch reviewed Jan 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rationalize pico_float/pico_double libraries #2208

rationalize pico_float/pico_double libraries #2208

kilograham commented Jan 23, 2025

kilograham commented Jan 23, 2025

kilograham commented Jan 23, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

lurch Jan 24, 2025

		* On Arm, (replacement) optimized implementations are provided for the following compiler built-ins when
		* and math library functions when using `pico_dobule_pico`:

rationalize pico_float/pico_double libraries #2208

Are you sure you want to change the base?

rationalize pico_float/pico_double libraries #2208

Conversation

kilograham commented Jan 23, 2025

kilograham commented Jan 23, 2025

kilograham commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment