[Core] Simplify audio_duration_to_ms() and audio_ms_to_duration(), reduce firmware size by a few bytes. #21427
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit simplifies `audio_duration_to_ms()` and `audio_ms_to_duration()` to reduce firmware size. The revised implementation is a shared integer implementation for both ARM and AVR. It has the same accuracy and supported numerical range as before, verified by new unit tests added with this commit. It costs a few bytes less in firware size: Checked with `qmk compile` before vs. after: keyboard before after delta ARV rhino 23374 23370 4 bytes smaller Moonlander (ARM) 69106 69098 8 bytes smaller Checked with godbolt.org Compiler Explorer (AVR results compiled with AVR gcc 13.1.0 -Os, ARM results with ARM 32-bit gcc 13.1 -Os): function before after delta AVR audio_duration_to_ms 24 19 5 instructions fewer AVR audio_ms_to_duration 24 21 3 instructions fewer ARM audio_duration_to_ms 16 9 7 instructions fewer ARM audio_ms_to_duration 17 8 9 instructions fewer
drashna
approved these changes
Jul 3, 2023
tzarc
approved these changes
Jul 7, 2023
jesperhellberg
pushed a commit
to jesperhellberg/qmk_firmware
that referenced
this pull request
Sep 9, 2023
…duce firmware size by a few bytes. (qmk#21427)
thismarvin
pushed a commit
to thismarvin/qmk_firmware
that referenced
this pull request
Sep 27, 2023
…duce firmware size by a few bytes. (qmk#21427)
akeep
pushed a commit
to akeep/qmk_firmware
that referenced
this pull request
Oct 2, 2023
…duce firmware size by a few bytes. (qmk#21427)
csolje
pushed a commit
to csolje/qmk_firmware
that referenced
this pull request
Oct 21, 2023
…duce firmware size by a few bytes. (qmk#21427)
autoferrit
pushed a commit
to SpaceRockMedia/bastardkb-qmk
that referenced
this pull request
Dec 8, 2023
…duce firmware size by a few bytes. (qmk#21427)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit simplifies
audio_duration_to_ms()andaudio_ms_to_duration()and reduces firmware size by a few bytes.Description
This commit simplifies the audio duration unit conversion functions
audio_duration_to_ms()andaudio_ms_to_duration()to reduce firmware size. While the existing code has separate implementations for AVR vs. ARM, this revision is a single shared integer implementation for both.This revision has the same accuracy and supported numerical range as before. This is verified by new unit tests added with this commit. I've also added comments with a brief math analysis to clarify when these conversions are sure to work without overflow.
This revision costs a few bytes less in firmware size.
Checked firmware size with
qmk compilebefore vs. after:Checked number of instructions with godbolt (AVR results compiled with AVR gcc 13.1.0 -Os, ARM results with ARM 32-bit gcc 13.1 -Os):
Types of Changes
Checklist