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/macros: rewrite DIV_ROUND add DIV_ROUND_UINT #18856

Closed
wants to merge 1 commit into from

Conversation

kfessel
Copy link
Contributor

@kfessel kfessel commented Nov 8, 2022

with #18853 i had hope to remove SIGNOF from DIV_ROUND but tests showed that types and negative numbers had some false results, so this makes DIV_ROUND deliver what it promisses (math rounded division for integers that don't reach to high into the rage)

There are some variants to implement such thing at https://godbolt.org/z/TsKcnPoYx sadly most of them either fail in a specific range or if divisor or dividend is negative or unsigned are typical, so i choose the one that worked across the test range (DIV_ROUNDb) please feel free to find a better one.

Condition:
Working for either i or j or both being uint and for negative an positive values of i and j

Contribution description

This DIV_ROUND works for:

  • either a or b or both being uint and
  • for negative an positive values of a and b as expected

current upstream tells me -15 / -5 = rounded to nearest is 2 (that is wrong)
with this PR i get 3 (see the godbolt)

Oh and i shall not forget i added DIV_ROUND_UINT for the most likely usecases

this also circumvents the -Wtype-limits warning by having the same result for positive and 0 but 2 checks (thatat ar optimized away) an alternative would be to have 3 result 1,0,-1 or

#if defined(__GNUC__)
_Pragma("GCC diagnostic ignored \"-Wtype-limits\"")
#endif

Testing procedure

read, run tests, look at that linked compiler explorer

Issues/PRs references

this fixes the issue of #18849

#18853 was my first atempt on this

https://godbolt.org/z/TsKcnPoYx

@kfessel kfessel requested a review from kaspar030 as a code owner November 8, 2022 18:37
@github-actions github-actions bot added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Nov 8, 2022
@kfessel kfessel requested review from benpicco and maribu November 8, 2022 18:37
@kfessel kfessel added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 8, 2022
@riot-ci
Copy link

riot-ci commented Nov 8, 2022

Murdock results

✔️ PASSED

23bcc18 core/macros: rewrite DIV_ROUND add DIV_ROUND_UINT

Success Failures Total Runtime
1999 0 2000 06m:27s

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

#18858 is better

@kfessel kfessel closed this Nov 9, 2022
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! CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants