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

core/marco: fix DIV_ROUND #18858

Merged
merged 2 commits into from
Nov 10, 2022
Merged

core/marco: fix DIV_ROUND #18858

merged 2 commits into from
Nov 10, 2022

Conversation

kfessel
Copy link
Contributor

@kfessel kfessel commented Nov 9, 2022

#18856 lead to huge implementations this does that better a more efficient DIV_ROUND_UINT is not required

Contribution description

this fixes DIV_ROUND
this fixes DIV_ROUND_UP and documents its now reliable behavior
this add DIV_ROUND_INF which round away from 0 (the original intended behavior of DIV_ROUND_UP)

this adds Tests that test the DIV_ROUND* macros with negative inputs

Testing procedure

read, test, run tests/unitest/ core

<RIOT>/tests/unittests$ make tests-core term

Issues/PRs references

have a test at
https://godbolt.org/z/bEvT6Wzr9

#18849
#18856
#18853

@kfessel kfessel requested review from benpicco and maribu November 9, 2022 16:57
@github-actions github-actions bot added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: tests Area: tests and testing framework labels Nov 9, 2022
@kfessel kfessel added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 9, 2022
@riot-ci
Copy link

riot-ci commented Nov 9, 2022

Murdock results

✔️ PASSED

aa31dd7 core/macros: rewrite DIV_ROUND, DIV_ROUND_UP; add DIV_ROUND_INF

Success Failures Total Runtime
2002 0 2002 06m:23s

Artifacts

This only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now.

@kfessel
Copy link
Contributor Author

kfessel commented Nov 9, 2022

alternative:

#define DIV_ROUND_UP(a, b) ((SIGNOF(a) == SIGNOF(b)) ? (((a) + (b) - SIGNOF(a)) / (b)): (a) / (b))

this one seems to produce less good code (needs more stack if put into its own function)

@kfessel kfessel merged commit 19021d6 into RIOT-OS:master Nov 10, 2022
@kaspar030 kaspar030 added this to the Release 2023.01 milestone Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants