Skip to content

Fixes #84: Choice of float/double #116

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.

Sign up for GitHub

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

Jump to bottom
Merged
eyalroz merged 1 commit into from
Jan 21, 2023
Merged

Fixes #84: Choice of float/double #116

eyalroz merged 1 commit into from
Jan 21, 2023

Conversation

@eyalroz
Copy link
Owner

@eyalroz eyalroz commented Feb 7, 2022

Fixes #84.

@eyalroz eyalroz mentioned this pull request Feb 7, 2022
Closed
@eyalroz eyalroz force-pushed the develop branch 2 times, most recently from e91b7e5 to e0ef6ca Compare February 7, 2022 21:26
@jrahlf
Copy link

jrahlf commented Feb 7, 2022 •
edited
Loading

I think it looks good so far.

  • previously platforms where double is 32-bit (GCC-AVR) were supported implicitly (without documentation), this update breaks this unless PRINTF_USE_DOUBLE_INTERNALLY is set to 0. Either the code or the documentation should be updated accordingly.
  • As shown in the other PR, to get the full benefit, one must (try to) also get rid of the int64_t<>float conversions, but this can also be attempted in another PR

I can try it out on microcontroller on the weekend.

@eyalroz
Copy link
Owner Author

eyalroz commented Feb 7, 2022

this update breaks this unless PRINTF_USE_DOUBLE_INTERNALLY is set to 0

It shouldn't! If that happens, I have a bug. I tried to support double = 32-bits even when PRINTF_USE_DOUBLE_INTERNALLY is 1.

but this can also be attempted in another PR

One step at a time :-) ... also, I've created v6.0.0 with my recent changes. Would appreciate feedback there as well.

@jrahlf
Copy link

jrahlf commented Feb 8, 2022

this update breaks this unless PRINTF_USE_DOUBLE_INTERNALLY is set to 0

It shouldn't! If that happens, I have a bug. I tried to support double = 32-bits even when PRINTF_USE_DOUBLE_INTERNALLY is 1.

I misread the code, I think you are right.

@eyalroz
Copy link
Owner Author

eyalroz commented Feb 8, 2022

Just so you know, though, I'm not hurrying to merge this before I get some feedback of the v6.0.0 pre-release; and properly release it. Then this can go on the development branch.

@eyalroz eyalroz force-pushed the develop branch 2 times, most recently from 52e9999 to 2b85ce1 Compare February 11, 2022 09:30
@eyalroz eyalroz closed this Feb 21, 2022
@eyalroz eyalroz reopened this Feb 21, 2022
@eyalroz
Fixes #84: Choice of float/double
61921be 
* Can now choose between using `double` and `float` internally, for floating-point work, using the `PRINTF_USE_DOUBLE_INTERNALLY` definition and corresponding CMake option.
* Adjusted test suite to support the different choice of floating-point type:
	* Tests relevant both to `float` and `double`, but with different precisions, are adjusted with an `#if #else #endif`
	* Tests and test cases relevant only for `double` precision are not compiled at all when `float` is used.
* Lots of new explicit conversions :-(
@eyalroz eyalroz merged commit 61921be into develop Jan 21, 2023
@crazybolillo
Copy link

crazybolillo commented Oct 10, 2023

Hello, I see this was merged into develop. I personally tried this new option on a Cortex M4 with FP unit and it reduced code size by around 800 bytes, so that is good. Also my program still works fine. Thanks for the work, just thought I would give a testimony to the fact this works hehe.

@eyalroz
Copy link
Owner Author

eyalroz commented Oct 10, 2023

Thank you, @crazybolillo , it's nice to get positive feedback :-)

@eyalroz eyalroz mentioned this pull request Dec 10, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eyalroz @jrahlf @crazybolillo