Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add print(float) to save space (VS double). #2036

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

honnet
Copy link
Contributor

@honnet honnet commented Jun 6, 2023

Summary

This PR implements the following methods in Print.h and Print.cpp:

  • Feature 1: Print::print(float) - - - only double was available
  • Feature 2: Print::println(float) - - - only double was available
  • Feature 2: Print::printFloat(float) - - - only double was available => maybe it should be called printDouble(double)?

Motivation

Some of the new MCUs have a limited memory (ex: 32KB for STM32C011D6) and don't have floating point calculation unit, so a simple accelerometer driver (example) can take more than 100% of the flash if we don't optimize it a bit (the compilation fails at first).

This PR saves up to 10K by avoiding the compiler to use ARM libs for double calculation subroutines such as __aeabi_dsub, __aeabi_dadd, __aeabi_dmul, etc.


Validation

The following command allows listing the heaviest functions after compilation:
arm-none-eabi-nm --print-size --size-sort --radix=d -l file.elf | tail -n 3

134242369 00001416 T __aeabi_dmul
134240049 00001724 T __aeabi_dadd
134243785 00001796 T __aeabi_dsub

The problem can also be observed in the assembly code [with C code + comments using debug enabled (-g) if there's enough space]:
arm-none-eabi-objdump -S file.elf


Note
The printFloat(float) function repeats a lot of what printFloat(double) does and it should probably be factorized.
I expect that this PR could be rejected for this reason at least, let me know if you have any suggestion of how to implement it (using templates?).

@fpistm fpistm self-requested a review June 7, 2023 13:05
@fpistm fpistm added this to In progress in STM32 core based on ST HAL via automation Jun 7, 2023
@fpistm
Copy link
Member

fpistm commented Jun 7, 2023

HI @honnet
Thanks for this PR.
do you evaluate the Serial.printf("%f"); (enable Newlib + prinrtf float support in the menu) or the dtostrf()?

@honnet
Copy link
Contributor Author

honnet commented Jun 7, 2023

Thanks for the feedback!

Sadly, with the simple accelerometer example, enabling Newlib with prinrtf takes too much space (=> linker error).

...but it says how much memory is missing so I got the following values:

  • newlib + float + Serial.print() : 42K
  • newlib + float + Serial.printf(): 41K
  • dtostrf: 30K

As a comparison, with my PR, the space used is 27K (which is already tight for the 32K available flash).


Edit - other example:

For the C011 or C031 (32KB max), the following code will not compile without my PR:
https://github.com/adafruit/Adafruit_MMC56x3/tree/main/examples/compass

@fpistm
Copy link
Member

fpistm commented Jun 26, 2023

Hi @honnet
I've updated your PR with template to avoid code duplication.
I guess this enhancement could be shared with ArduinoCore-API in order to be aligned:
https://github.com/arduino/ArduinoCore-API/blob/master/api/Print.h

@fpistm fpistm added this to the 2.6.0 milestone Jun 26, 2023
Copy link
Member

@fpistm fpistm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

STM32 core based on ST HAL automation moved this from In progress to Reviewer approved Jun 26, 2023
@honnet
Copy link
Contributor Author

honnet commented Jun 26, 2023

Great!
Thanks a lot for the improvement!

@honnet honnet closed this Jun 26, 2023
STM32 core based on ST HAL automation moved this from Reviewer approved to Done Jun 26, 2023
@fpistm fpistm reopened this Jun 26, 2023
STM32 core based on ST HAL automation moved this from Done to In progress Jun 26, 2023
@fpistm
Copy link
Member

fpistm commented Jun 26, 2023

It is not yet merged 😉

@honnet honnet closed this Jun 26, 2023
STM32 core based on ST HAL automation moved this from In progress to Done Jun 26, 2023
@honnet
Copy link
Contributor Author

honnet commented Jun 26, 2023

My bad, sorry!

@fpistm
Copy link
Member

fpistm commented Jun 26, 2023

@honnet seems you rebase and removed all changes....

Signed-off-by: Cedric Honnet <honnet@telecom-paristech.org>
Co-Authored-By: Frederic Pillon <frederic.pillon@st.com>
@honnet
Copy link
Contributor Author

honnet commented Jun 26, 2023

Damn, I didn't pay attention and I thought I integrated your modifications!

I found a hash in my terminal history, cherry-picked it and pushed it:
main...honnet:Arduino_Core_STM32:main

Does it make the PR usable now, or should I do it again?

@fpistm
Copy link
Member

fpistm commented Jun 26, 2023

I will restore it tomorrow. Do nothing until I fix the issue.

@fpistm fpistm reopened this Jun 26, 2023
STM32 core based on ST HAL automation moved this from Done to In progress Jun 26, 2023
STM32 core based on ST HAL automation moved this from In progress to Reviewer approved Jun 27, 2023
@fpistm fpistm merged commit c6bc5b2 into stm32duino:main Jun 27, 2023
44 checks passed
STM32 core based on ST HAL automation moved this from Reviewer approved to Done Jun 27, 2023
@honnet
Copy link
Contributor Author

honnet commented Jun 29, 2023

I just tested a c/cpp compiler option to forbid doubles: -fsingle-precision-constant:
https://github.com/stm32duino/Arduino_Core_STM32/blob/main/platform.txt#L35-L37

It's even more systematic and future proof against libraries that hide implicit doubles (in constants for example).

...but I'm not sure how to make it user friendly:

  1. in the Optimize options?
    https://github.com/stm32duino/Arduino_Core_STM32/blob/main/boards.txt#L11490

  2. in the C Runtime Library options?
    https://github.com/stm32duino/Arduino_Core_STM32/blob/main/boards.txt#L12404

  3. something else? nowhere? (maybe it's just a bad idea)

Is it worth exploring something clean? (any advice would be appreciated)

@fpistm
Copy link
Member

fpistm commented Jun 29, 2023

You can use build_opt.h feature, it was added to support such kind of request:
https://github.com/stm32duino/Arduino_Core_STM32/wiki/Customize-build-options-using-build_opt.h

@honnet
Copy link
Contributor Author

honnet commented Jun 29, 2023

Perfect!
Thanks again so much!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants