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

Remove the usage of floats #108

Closed
bjoernQ opened this issue Jul 20, 2022 · 3 comments · Fixed by #173
Closed

Remove the usage of floats #108

bjoernQ opened this issue Jul 20, 2022 · 3 comments · Fixed by #173
Assignees

Comments

@bjoernQ
Copy link
Contributor

bjoernQ commented Jul 20, 2022

Currently we use floats in one place:

// TODO can we get this to not use doubles/floats
let period = 1_000_000f64 / (clock.to_Hz() as f64 / divider as f64); // micros
(micros as f64 / period) as u64

If possible we should remove that since it adds 2k of code (optimized) for ESP32-C3:

File  .text   Size                       Crate Name
0.0%  12.3% 1.2KiB           compiler_builtins compiler_builtins::float::div::__divdf3
0.0%   3.0%   292B           compiler_builtins __floatundidf
0.0%   1.6%   156B           compiler_builtins __floatunsidf
0.0%   1.4%   140B           compiler_builtins __gedf2
0.0%   1.4%   140B           compiler_builtins __gtdf2
0.0%   1.3%   130B           compiler_builtins __fixunsdfdi
@bjoernQ
Copy link
Contributor Author

bjoernQ commented Jul 26, 2022

note: the upcoming LEDC implementation is currently also using floats

@jessebraham
Copy link
Member

I did say to @JurajSadel that we can not worry about the floats right now, just for the sake of getting things completed and merged. If you have a problem with this please feel free to say so though, doesn't really make a difference to me.

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Jul 26, 2022

No problem with that - we can replace the floats in one go later

@jessebraham jessebraham added this to the v0.2.0 milestone Aug 22, 2022
@JurajSadel JurajSadel self-assigned this Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants