-
Notifications
You must be signed in to change notification settings - Fork 31
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
Cleanup umul() #195
Cleanup umul() #195
Conversation
Codecov Report
@@ Coverage Diff @@
## master #195 +/- ##
===========================================
- Coverage 100.00% 99.10% -0.90%
===========================================
Files 9 9
Lines 2332 2335 +3
===========================================
- Hits 2332 2314 -18
- Misses 0 21 +21 |
18eda72
to
8d4d5c0
Compare
@chfast Not sure what to do with coverage here, any ideas? |
#if defined(__SIZEOF_INT128__) | ||
#pragma GCC diagnostic push | ||
#pragma GCC diagnostic ignored "-Wpedantic" | ||
if (!is_constant_evaluated()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using builtin uint128 is fine in constexpr too. The is_constant_evaluated()
is only actually needed for MSVC usage of intrinsic. We can keep it if not too big problem.
Yes, the 32-bit will not have |
This has landed, you can rebase now. |
e1658b3
to
d297c13
Compare
Co-authored-by: Andrei Maiboroda <andrei@ethereum.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Pulled out of #180 with a test fixed in a boring way.