Use jtckdint.h for checked integer operations #217
Merged
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 is a single header implementation of C11's `stdckdint.h` (using the GCC/Clang builtins where possible) which looks like it does a better job than our fallback functions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
==========================================
- Coverage 98.62% 98.60% -0.02%
==========================================
Files 69 69
Lines 2614 2578 -36
==========================================
- Hits 2578 2542 -36
Misses 36 36 ☔ View full report in Codecov by Sentry. |
* All functions are constexpr * #include <stdbool.h> to define the `_Bool` type to avoid an error with GCC in C++ mode * Use GNU builtins (__builtin_add_overflow etc) even when __STRICT_ANSI__ is defined (if upstream GCC and Clang are happy to do use the builtins in strict ANSI mode then so am I)
The jtckdint.h library uses * C11 `ckd_add()` etc when available * Otherwise, GNU compiler builtins (`__builtin_add_overflow()` etc) when available * Otherwise, a high-quality fallback implementation The fallback implementation looks to be rather better than our own fallback impls, and will hopefully lead to better codegen on MSVC where the builtins are not available.
This is "unreferenced formal parameter", which as of MSVC 19.40 is getting triggered even when the parameter *is* referenced by a different `if constexpr` path. https://developercommunity.visualstudio.com/t/Bogus-C4100-unreferenced-formal-paramet/10720820
After some experimentation on Compiler Explorer, it seems like we can use FLUX_ALWAYS_INLINE on the one hand and manually "flattening" calls on the other to (hopefully) improve the unoptimised codegen and the debugger experience
tcbrindle
force-pushed
the
pr/jtckdint
branch
from
68abce0 to
35081da
Compare
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.
The jtckdint.h library uses
ckd_add()etc when available__builtin_add_overflow()etc) when availableIn particular, the fallback implementations look to be rather better than our own fallback impls, and will hopefully lead to better codegen on MSVC where the builtins are not available.