Add float benchmark, and some minor float fixes/cleanup #2864

Merged
kilograham merged 29 commits from float-benchmark into develop 2026-03-24 17:49:55 -05:00
kilograham commented 2026-03-11 13:36:12 -05:00 (Migrated from github.com)
No description provided.
kilograham commented 2026-03-11 13:36:57 -05:00 (Migrated from github.com)

Actually converting to draft, as we should document the new defines (though they aren't really PICO_CONFIG as they are provided not options)

Actually converting to draft, as we should document the new defines (though they aren't really PICO_CONFIG as they are provided not options)
lurch (Migrated from github.com) reviewed 2026-03-12 02:57:37 -05:00
@ -198,0 +81,4 @@
*
* On Arm, the following additional optimized functions are also provided when using `pico_double_pico`, all of which
* saturate to the nearest representable value for too large input when converting from floating point types:
*
lurch (Migrated from github.com) commented 2026-03-12 02:57:36 -05:00

Huh? Line 54 says "On Arm, ..." and then this line says "Additional functions on Arm:" ??

Huh? Line 54 says "On Arm, ..." and then this line says "Additional functions on Arm:" ??
lurch (Migrated from github.com) reviewed 2026-03-12 03:00:40 -05:00
@ -198,0 +129,4 @@
#include "pico.h"
// PICO_CONFIG: PICO_DOUBLE_IN_RAM, Force placement of SDK provided double-precision floating point code into RAM, type=bool, default=0, group=pico_double
lurch (Migrated from github.com) commented 2026-03-12 03:00:40 -05:00

Should this be group=pico_double ? (defined on line 18 in this file)

Should this be `group=pico_double` ? (defined on line 18 in this file)
lurch (Migrated from github.com) reviewed 2026-03-12 03:15:44 -05:00
@ -0,0 +1,1342 @@
#include <stdio.h>
lurch (Migrated from github.com) commented 2026-03-12 03:15:44 -05:00

I guess the "dcmpge" name implies that this was supposed to be

    volatile bool v = a >= b;

?

I guess the "dcmpge" name implies that this was supposed to be ```suggestion volatile bool v = a >= b; ``` ?
lurch (Migrated from github.com) reviewed 2026-03-12 03:24:44 -05:00
@ -0,0 +309,4 @@
return cycle_diff(t0, t1) - INT_INPUT_COST - DOUBLE_OUTPUT_COST;
}
timer_func_def(ui2d)(volatile int32_t i) {
lurch (Migrated from github.com) commented 2026-03-12 03:24:44 -05:00

This might be a stupid question, but why does timer_func_def(ui2d), timer_func_def(uint2double), etc. take a volatile int32_t i argument instead of a volatile uint32_t i argument? (Same question also for the uint64_t functions.)

This might be a stupid question, but why does `timer_func_def(ui2d)`, `timer_func_def(uint2double)`, etc. take a `volatile int32_t i` argument instead of a `volatile uint32_t i` argument? (Same question also for the `uint64_t` functions.)
lurch (Migrated from github.com) reviewed 2026-03-12 03:35:45 -05:00
@ -0,0 +1,1342 @@
#include <stdio.h>
lurch (Migrated from github.com) commented 2026-03-12 03:35:45 -05:00

??

??
lurch (Migrated from github.com) reviewed 2026-03-12 03:41:15 -05:00
@ -0,0 +757,4 @@
#endif
}
timer_func_def(fix642double_c)(volatile int64_t i) {
lurch (Migrated from github.com) commented 2026-03-12 03:41:15 -05:00

This appears to be almost identical to timer_func_def(fix642double) ? (With I guess the only difference being that here the n parameter is const)

This appears to be almost identical to `timer_func_def(fix642double)` ? (With I guess the only difference being that here the `n` parameter is `const`)
lurch (Migrated from github.com) reviewed 2026-03-12 03:54:44 -05:00
@ -0,0 +1,1286 @@
#include <stdio.h>
lurch (Migrated from github.com) commented 2026-03-12 03:54:43 -05:00
    volatile bool v = a >= b;
```suggestion volatile bool v = a >= b; ```
lurch (Migrated from github.com) reviewed 2026-03-12 03:57:29 -05:00
@ -198,0 +129,4 @@
#include "pico.h"
// PICO_CONFIG: PICO_DOUBLE_IN_RAM, Force placement of SDK provided double-precision floating point code into RAM, type=bool, default=0, group=pico_double
lurch (Migrated from github.com) commented 2026-03-12 03:57:29 -05:00

If we've got all these shiny new PICO_DOUBLE_HAS_BLAH_CONVERSIONS defines, maybe it makes sense to also use them in custom_double_funcs_test.c ? (and similarly for float, of course)

If we've got all these shiny new `PICO_DOUBLE_HAS_BLAH_CONVERSIONS` defines, maybe it makes sense to also use them in `custom_double_funcs_test.c` ? (and similarly for float, of course)
lurch commented 2026-03-12 04:11:12 -05:00 (Migrated from github.com)

Bazel is complaining that it hasn't been told about test/pico_float_test/double_benchmark.c and test/pico_float_test/float_benchmark.c yet 😉

check-configs is complaining because there's no #define PICO_DOUBLE_IN_RAM 0. In #2858 I added it to src/rp2_common/pico_double/double_v1_rom_shim_rp2040.S, copying what https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_float/float_v1_rom_shim_rp2040.S#L11 does.

Bazel is complaining that it hasn't been told about `test/pico_float_test/double_benchmark.c` and `test/pico_float_test/float_benchmark.c` yet :wink: check-configs is complaining because there's no `#define PICO_DOUBLE_IN_RAM 0`. In #2858 I added it to `src/rp2_common/pico_double/double_v1_rom_shim_rp2040.S`, copying what https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_float/float_v1_rom_shim_rp2040.S#L11 does.
kilograham (Migrated from github.com) reviewed 2026-03-16 15:12:00 -05:00
@ -0,0 +309,4 @@
return cycle_diff(t0, t1) - INT_INPUT_COST - DOUBLE_OUTPUT_COST;
}
timer_func_def(ui2d)(volatile int32_t i) {
kilograham (Migrated from github.com) commented 2026-03-16 15:11:59 -05:00

because i didn't feel like adding even more variants for signed integers - the argument type is known so i just cast

because i didn't feel like adding even more variants for signed integers - the argument type is known so i just cast
kilograham (Migrated from github.com) reviewed 2026-03-16 15:13:47 -05:00
@ -0,0 +757,4 @@
#endif
}
timer_func_def(fix642double_c)(volatile int64_t i) {
kilograham (Migrated from github.com) commented 2026-03-16 15:13:47 -05:00

yup - we have optimized implementations for fixed exponents since there is a processor instruction

yup - we have optimized implementations for fixed exponents since there is a processor instruction
kilograham (Migrated from github.com) reviewed 2026-03-16 15:15:31 -05:00
@ -0,0 +1,1286 @@
#include <stdio.h>
kilograham (Migrated from github.com) commented 2026-03-16 15:15:31 -05:00

goot spot

goot spot
kilograham (Migrated from github.com) reviewed 2026-03-16 15:15:50 -05:00
@ -198,0 +129,4 @@
#include "pico.h"
// PICO_CONFIG: PICO_DOUBLE_IN_RAM, Force placement of SDK provided double-precision floating point code into RAM, type=bool, default=0, group=pico_double
kilograham (Migrated from github.com) commented 2026-03-16 15:15:50 -05:00

yup

yup
kilograham (Migrated from github.com) reviewed 2026-03-16 15:19:14 -05:00
@ -198,0 +81,4 @@
*
* On Arm, the following additional optimized functions are also provided when using `pico_double_pico`, all of which
* saturate to the nearest representable value for too large input when converting from floating point types:
*
kilograham (Migrated from github.com) commented 2026-03-16 15:19:14 -05:00

oops

oops
lurch commented 2026-03-17 04:03:31 -05:00 (Migrated from github.com)

check-configs is complaining because there's no #define PICO_DOUBLE_IN_RAM 0. In #2858 I added it to src/rp2_common/pico_double/double_v1_rom_shim_rp2040.S, copying what https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_float/float_v1_rom_shim_rp2040.S#L11 does.

This is still the case 😉

> check-configs is complaining because there's no `#define PICO_DOUBLE_IN_RAM 0`. In #2858 I added it to `src/rp2_common/pico_double/double_v1_rom_shim_rp2040.S`, copying what https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_float/float_v1_rom_shim_rp2040.S#L11 does. This is still the case :wink:
lurch (Migrated from github.com) reviewed 2026-03-17 04:12:48 -05:00
@ -18,22 +18,23 @@
#define test_checki64(x, expected, msg) ({ if ((x) != (expected)) { printf(" %s: %lld != %lld\n", msg, (int64_t)(x), (int64_t)(expected)); stop(); } })
lurch (Migrated from github.com) commented 2026-03-17 04:12:48 -05:00

Maybe it makes sense to also use the PICO_DOUBLE_HAS_ macros here and on line 37?

Maybe it makes sense to also use the `PICO_DOUBLE_HAS_` macros here and on line 37?
lurch (Migrated from github.com) reviewed 2026-03-17 04:14:14 -05:00
lurch (Migrated from github.com) commented 2026-03-17 04:14:14 -05:00

Lovely ❤️

Lovely :heart:
kilograham (Migrated from github.com) reviewed 2026-03-17 09:34:01 -05:00
@ -18,22 +18,23 @@
#define test_checki64(x, expected, msg) ({ if ((x) != (expected)) { printf(" %s: %lld != %lld\n", msg, (int64_t)(x), (int64_t)(expected)); stop(); } })
kilograham (Migrated from github.com) commented 2026-03-17 09:34:01 -05:00

no the comment was meant to imply that is not the case, maybe it could be clearer

no the comment was meant to imply that is not the case, maybe it could be clearer
kilograham commented 2026-03-17 09:37:40 -05:00 (Migrated from github.com)

check-configs is complaining because there's no #define PICO_DOUBLE_IN_RAM 0. In #2858 I added it to src/rp2_common/pico_double/double_v1_rom_shim_rp2040.S, copying what https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_float/float_v1_rom_shim_rp2040.S#L11 does.

This is still the case 😉

Yeah - i was fixing that but my change ended up ballooning

  1. I decided to move it into the headers
  2. Which meant they had to be included by the asm
  3. I then thought - hey maybe assembly code would like the PICO_FLOAT_HAS_ macros too
  4. which then led me to re-arrange the header and ended up guarding all the function declarations with the #if PICO_FLOAT_HAS_XXX

3 & 4 made things really messy after all, so I think I'll just stick with 1/2 for now

> > check-configs is complaining because there's no `#define PICO_DOUBLE_IN_RAM 0`. In #2858 I added it to `src/rp2_common/pico_double/double_v1_rom_shim_rp2040.S`, copying what https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2_common/pico_float/float_v1_rom_shim_rp2040.S#L11 does. > > This is still the case 😉 Yeah - i was fixing that but my change ended up ballooning 1. I decided to move it into the headers 2. Which meant they had to be included by the asm 3. I then thought - hey maybe assembly code would like the PICO_FLOAT_HAS_ macros too 4. which then led me to re-arrange the header and ended up guarding all the function declarations with the #if PICO_FLOAT_HAS_XXX 3 & 4 made things really messy after all, so I think I'll just stick with 1/2 for now
lurch (Migrated from github.com) reviewed 2026-03-18 10:24:40 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-18 10:24:39 -05:00
// funcs except for __PICO_DOUBLE_ARM_OPTIMIZED so wouldn't
```suggestion // funcs except for __PICO_DOUBLE_ARM_OPTIMIZED so wouldn't ```
lurch (Migrated from github.com) reviewed 2026-03-18 10:26:38 -05:00
@ -135,75 +127,238 @@ extern "C" {
*
lurch (Migrated from github.com) commented 2026-03-18 10:26:37 -05:00
    // for VFP the C cast is an assembly instruction anyway, so we prefer that over a function call
```suggestion // for VFP the C cast is an assembly instruction anyway, so we prefer that over a function call ```
lurch (Migrated from github.com) reviewed 2026-03-18 10:26:55 -05:00
@ -135,75 +127,238 @@ extern "C" {
*
lurch (Migrated from github.com) commented 2026-03-18 10:26:55 -05:00
// funcs except for __PICO_FLOAT_ARM_OPTIMIZED so wouldn't
```suggestion // funcs except for __PICO_FLOAT_ARM_OPTIMIZED so wouldn't ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:11:14 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:11:14 -05:00

I wonder if these descriptions in double.h should say "...to the nearest double" ?

I wonder if these descriptions in `double.h` should say "...to the nearest double" ?
lurch (Migrated from github.com) reviewed 2026-03-19 03:13:57 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:13:57 -05:00
//! \brief Convert an unsigned 32-bit integer with the given number of fractional bits to the nearest double

?

```suggestion //! \brief Convert an unsigned 32-bit integer with the given number of fractional bits to the nearest double ``` ?
lurch (Migrated from github.com) reviewed 2026-03-19 03:14:39 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:14:38 -05:00
//! \brief Convert an unsigned 64-bit integer with the given number of fractional bits to the nearest double
```suggestion //! \brief Convert an unsigned 64-bit integer with the given number of fractional bits to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:18:42 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:18:42 -05:00
//! \brief Convert a double to an usigned 64-bit integer, rounding towards -Infinity.
//! This conversion is saturating (to UINT64_MAX/UINT64_MIN) for out of range input
```suggestion //! \brief Convert a double to an usigned 64-bit integer, rounding towards -Infinity. //! This conversion is saturating (to UINT64_MAX/UINT64_MIN) for out of range input ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:23:24 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:23:24 -05:00

I'm gonna assume yes, based on other descriptions in this header file!

I'm gonna assume yes, based on other descriptions in this header file!
lurch (Migrated from github.com) reviewed 2026-03-19 03:23:49 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:23:49 -05:00
//! \brief Convert a signed 32-bit integer with the given number of fractional bits to the nearest double
```suggestion //! \brief Convert a signed 32-bit integer with the given number of fractional bits to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:24:10 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:24:10 -05:00
//! \brief Convert a signed 64-bit integer with the given number of fractional bits to the nearest double
```suggestion //! \brief Convert a signed 64-bit integer with the given number of fractional bits to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:30:36 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:30:35 -05:00

Probably a stupid question, but is this needed? https://en.cppreference.com/w/c/header/float.html says that it just defines a bunch of constants starting with either FLT_, DBL_ or LDBL_; but I can't see any of those being used here?

Probably a stupid question, but is this needed? https://en.cppreference.com/w/c/header/float.html says that it just defines a bunch of constants starting with either `FLT_`, `DBL_` or `LDBL_`; but I can't see any of those being used here?
lurch (Migrated from github.com) reviewed 2026-03-19 03:31:01 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:31:00 -05:00
    //! Convert a signed 32-bit integer to the nearest double
```suggestion //! Convert a signed 32-bit integer to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:31:23 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:31:22 -05:00
    //! Convert an unsigned 32-bit integer to the nearest double
```suggestion //! Convert an unsigned 32-bit integer to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:31:54 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:31:54 -05:00
    //! Convert a signed 64-bit integer to the nearest double
```suggestion //! Convert a signed 64-bit integer to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:32:14 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:32:14 -05:00
    //! Convert an unsigned 64-bit integer to the nearest double
```suggestion //! Convert an unsigned 64-bit integer to the nearest double ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:37:37 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:37:37 -05:00

Does it matter that one of these is double powint(double, int32_t) but the other is double powint(double, int) ?

Does it matter that one of these is `double powint(double, int32_t)` but the other is `double powint(double, int)` ?
lurch (Migrated from github.com) reviewed 2026-03-19 03:43:57 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-19 03:43:56 -05:00

I had to look up https://en.cppreference.com/w/c/numeric/math/fma to check the arguments here - is it worth adding a small comment to clarify that this function effectively returns (x * y) + z ?

I had to look up https://en.cppreference.com/w/c/numeric/math/fma to check the arguments here - is it worth adding a small comment to clarify that this function effectively returns `(x * y) + z` ?
lurch (Migrated from github.com) reviewed 2026-03-19 03:48:11 -05:00
@ -135,75 +127,238 @@ extern "C" {
*
lurch (Migrated from github.com) commented 2026-03-19 03:48:11 -05:00
//! \brief Convert an unsigned 32-bit integer with the given number of fractional bits to the nearest float
```suggestion //! \brief Convert an unsigned 32-bit integer with the given number of fractional bits to the nearest float ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:48:33 -05:00
@ -135,75 +127,238 @@ extern "C" {
*
lurch (Migrated from github.com) commented 2026-03-19 03:48:33 -05:00
//! \brief Convert an unsigned 64-bit integer with the given number of fractional bits to the nearest float
```suggestion //! \brief Convert an unsigned 64-bit integer with the given number of fractional bits to the nearest float ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:50:13 -05:00
@ -135,75 +127,238 @@ extern "C" {
*
lurch (Migrated from github.com) commented 2026-03-19 03:50:13 -05:00
//! \brief Convert a float to an unsigned 64-bit integer, rounding towards -Infinity.
//! This conversion is saturating (to UINT64_MAX/UINT64_MIN) for out of range input
```suggestion //! \brief Convert a float to an unsigned 64-bit integer, rounding towards -Infinity. //! This conversion is saturating (to UINT64_MAX/UINT64_MIN) for out of range input ```
lurch (Migrated from github.com) reviewed 2026-03-19 03:55:23 -05:00
@ -442,10 +442,9 @@ int main() {
for (double a = -100.0; a < 100.0; a += 53.103) {
lurch (Migrated from github.com) commented 2026-03-19 03:55:23 -05:00

Is this right? The "FMA" and "FMAFAST" tests seem to be calling the same function?

Is this right? The "FMA" and "FMAFAST" tests seem to be calling the same function?
kilograham (Migrated from github.com) reviewed 2026-03-19 12:54:48 -05:00
@ -18,22 +18,23 @@
#define test_checki64(x, expected, msg) ({ if ((x) != (expected)) { printf(" %s: %lld != %lld\n", msg, (int64_t)(x), (int64_t)(expected)); stop(); } })
kilograham (Migrated from github.com) commented 2026-03-19 12:54:02 -05:00
#if PICO_DOUBLE_HAS_FIX32_TO_DOUBLE_CONVERSIONS

actually you were right

```suggestion #if PICO_DOUBLE_HAS_FIX32_TO_DOUBLE_CONVERSIONS ``` actually you were right
@ -442,10 +442,9 @@ int main() {
for (double a = -100.0; a < 100.0; a += 53.103) {
kilograham (Migrated from github.com) commented 2026-03-19 12:51:20 -05:00
                printf("FMAFAST %f\n", check_close3(fma_fast, a, b, c));

yes thankyou

```suggestion printf("FMAFAST %f\n", check_close3(fma_fast, a, b, c)); ``` yes thankyou
kilograham (Migrated from github.com) reviewed 2026-03-19 12:56:44 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
kilograham (Migrated from github.com) commented 2026-03-19 12:56:43 -05:00

it is a good question; i'm not sure why it was there but seems mean to remove it now in case someone is relying on it

it is a good question; i'm not sure why it was there but seems mean to remove it now in case someone is relying on it
kilograham (Migrated from github.com) reviewed 2026-03-19 13:01:57 -05:00
@ -195,3 +1,1 @@
#endif
#endif
/*
kilograham (Migrated from github.com) commented 2026-03-19 12:58:09 -05:00
//! Perform a fast multiply-add (x * y + z) with reduced accuracy (not fused multiply-add). This is another name for \ref fma_fast
```suggestion //! Perform a fast multiply-add (x * y + z) with reduced accuracy (not fused multiply-add). This is another name for \ref fma_fast ```
@ -289,36 +445,53 @@ uint64_t float2ufix64(float f, int e);
})
kilograham (Migrated from github.com) commented 2026-03-19 13:01:29 -05:00
    float powintf(float x, int32_t y);
```suggestion float powintf(float x, int32_t y); ```
lurch (Migrated from github.com) reviewed 2026-03-20 06:41:17 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
lurch (Migrated from github.com) commented 2026-03-20 06:41:17 -05:00

Before this PR, src/rp2_common/pico_float/include/pico/float.h did #include <float.h>.
This PR also adds #include <float.h> to src/rp2_common/pico_double/include/pico/double.h, which is what I was (indirectly) querying.

Before this PR, `src/rp2_common/pico_float/include/pico/float.h` did `#include <float.h>`. This PR also adds `#include <float.h>` to `src/rp2_common/pico_double/include/pico/double.h`, which is what I was (indirectly) querying.
lurch commented 2026-03-20 06:43:57 -05:00 (Migrated from github.com)

Yikes, CI builds are all failing with

/opt/actions-runner/_work/pico-sdk/pico-sdk/src/rp2_common/pico_double/include/pico/double.h:7: error: unterminated #ifndef
    7 | #ifndef _PICO_DOUBLE_H
Yikes, CI builds are all failing with ``` /opt/actions-runner/_work/pico-sdk/pico-sdk/src/rp2_common/pico_double/include/pico/double.h:7: error: unterminated #ifndef 7 | #ifndef _PICO_DOUBLE_H ```
lurch (Migrated from github.com) reviewed 2026-03-20 06:49:48 -05:00
@ -195,3 +1,1 @@
#endif
#endif
/*
lurch (Migrated from github.com) commented 2026-03-20 06:49:48 -05:00
*   mla/fma_fast
```suggestion * mla/fma_fast ```
lurch (Migrated from github.com) reviewed 2026-03-20 06:52:21 -05:00
@ -198,0 +205,4 @@
#if !__PICO_DOUBLE_ARM_OPTIMIZED
// for non Arm-optimized we may as well provide the function and let the compiler handle it
static inline double int2double(int32_t i) { return (double)i; }
static inline double uint2double(uint32_t i) { return (double)i; }
lurch (Migrated from github.com) commented 2026-03-20 06:52:21 -05:00

GitHub seems to have had a brain-fart and added a load of unwanted blank lines all throughout some of the recent commits in this PR 😢

GitHub seems to have had a brain-fart and added a load of unwanted blank lines all throughout some of the recent commits in this PR :cry:
lurch (Migrated from github.com) reviewed 2026-03-20 06:57:39 -05:00
lurch (Migrated from github.com) commented 2026-03-20 06:57:39 -05:00

IMHO ((x * y) + z) would be slightly clearer here, but I guess it doesn't matter as long as you assume that it's using the "normal" operator-precedence rules 😉

IMHO `((x * y) + z)` would be _slightly_ clearer here, but I guess it doesn't matter as long as you assume that it's using the "normal" operator-precedence rules :wink:
lurch (Migrated from github.com) reviewed 2026-03-20 07:00:05 -05:00
@ -195,3 +1,1 @@
#endif
#endif
/*
lurch (Migrated from github.com) commented 2026-03-20 07:00:04 -05:00
    double powint(double x, int32_t y);

(to match the change in float.h)

```suggestion double powint(double x, int32_t y); ``` (to match the change in `float.h`)
lurch (Migrated from github.com) reviewed 2026-03-20 07:12:10 -05:00
@ -0,0 +223,4 @@
#if PICO_FLOAT_HAS_SQRTF_FAST
register io_ro_32 *systick_ptr = systick_value_ptr();
uint32_t t0 = *systick_ptr;
volatile float x = sqrtf_fast(a);
lurch (Migrated from github.com) commented 2026-03-20 07:12:09 -05:00

Is it intentional that this uses both fsqrt_fast and sqrtf_fast, or should the timer_func_def also be using sqrtf_fast ?

Is it intentional that this uses both `fsqrt_fast` and `sqrtf_fast`, or should the `timer_func_def` also be using `sqrtf_fast` ?
lurch commented 2026-03-20 07:21:39 -05:00 (Migrated from github.com)
  • test/pico_float_test/pico_double_test.c is testing sqrt_fast - should test/pico_float_test/pico_float_test.c be testing sqrtf_fast?
  • Should test/pico_float_test/pico_double_test.c also be testing ddiv_fast ?
  • Should test/pico_float_test/pico_float_test.c also be testing fdiv_fast ?

P.S. furious_fast(7) 🏎️ 😆

- `test/pico_float_test/pico_double_test.c` is testing `sqrt_fast` - should `test/pico_float_test/pico_float_test.c` be testing `sqrtf_fast`? - Should `test/pico_float_test/pico_double_test.c` also be testing `ddiv_fast` ? - Should `test/pico_float_test/pico_float_test.c` also be testing `fdiv_fast` ? P.S. `furious_fast(7)` :racing_car: :laughing:
lurch (Migrated from github.com) reviewed 2026-03-20 07:26:59 -05:00
@ -195,3 +1,1 @@
#endif
#endif
/*
lurch (Migrated from github.com) commented 2026-03-20 07:26:59 -05:00
double ddiv_fast(double n, double d);
#endif

This should fix the build error 🤞
(undoing the whoopise that happened in 382b3e3535 )

```suggestion double ddiv_fast(double n, double d); #endif ``` This should fix the build error :crossed_fingers: (undoing the whoopise that happened in 382b3e3535304207eacae2980b7fefa0175c19ac )
kilograham (Migrated from github.com) reviewed 2026-03-24 11:00:31 -05:00
@ -198,0 +123,4 @@
*
* mla/fma_fast
*
* On RISC-V there is no custom double-precision floating point support, so `pico_double_pico` is equivalent to `pico_double_compiler`
kilograham (Migrated from github.com) commented 2026-03-24 11:00:31 -05:00

yeah probably don't want that

yeah probably don't want that
kilograham (Migrated from github.com) reviewed 2026-03-24 11:02:04 -05:00
@ -198,0 +205,4 @@
#if !__PICO_DOUBLE_ARM_OPTIMIZED
// for non Arm-optimized we may as well provide the function and let the compiler handle it
static inline double int2double(int32_t i) { return (double)i; }
static inline double uint2double(uint32_t i) { return (double)i; }
kilograham (Migrated from github.com) commented 2026-03-24 11:02:04 -05:00

grr

grr
kilograham (Migrated from github.com) reviewed 2026-03-24 11:02:21 -05:00
kilograham (Migrated from github.com) commented 2026-03-24 11:02:21 -05:00

yeah i think that is sane for any programmer

yeah i think that is sane for any programmer
kilograham (Migrated from github.com) reviewed 2026-03-24 11:03:09 -05:00
@ -0,0 +223,4 @@
#if PICO_FLOAT_HAS_SQRTF_FAST
register io_ro_32 *systick_ptr = systick_value_ptr();
uint32_t t0 = *systick_ptr;
volatile float x = sqrtf_fast(a);
kilograham (Migrated from github.com) commented 2026-03-24 11:03:09 -05:00

this is intentional

this is intentional
kilograham commented 2026-03-24 17:33:51 -05:00 (Migrated from github.com)
  • test/pico_float_test/pico_double_test.c is testing sqrt_fast - should test/pico_float_test/pico_float_test.c be testing sqrtf_fast?

sure

* Should `test/pico_float_test/pico_double_test.c` also be testing `ddiv_fast` ?

already was

* Should `test/pico_float_test/pico_float_test.c` also be testing `fdiv_fast` ?

sure

P.S. furious_fast(7) 🏎️ 😆

> * `test/pico_float_test/pico_double_test.c` is testing `sqrt_fast` - should `test/pico_float_test/pico_float_test.c` be testing `sqrtf_fast`? > sure > * Should `test/pico_float_test/pico_double_test.c` also be testing `ddiv_fast` ? > already was > * Should `test/pico_float_test/pico_float_test.c` also be testing `fdiv_fast` ? > > sure > P.S. `furious_fast(7)` 🏎️ 😆
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk!2864
No description provided.