-
Notifications
You must be signed in to change notification settings - Fork 1.5k
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
<xutility>
: Use addition and multiplication overflow check MSVC intrinsics like _add_overflow_i8
, _mul_overflow_i16
, and _mul_full_overflow_i8
#4780
Comments
Another caveat is maybe DevCom-10668881 -- need a good coverage |
<xutility>
: Use addition and multiplication oveflow check MSVC intrinsics like _add_overflow_i8
, _mul_overflow_i16
, and _mul_full_overflow_i8
<xutility>
: Use addition and multiplication overflow check MSVC intrinsics like _add_overflow_i8
, _mul_overflow_i16
, and _mul_full_overflow_i8
Yeah, these were implemented on 2023-04-17 by internal MSVC-PR-456118 "Add overflow detection functions for addition, subtraction and multiplication on x86 and x64". We'd eventually want to move them to I'd like to understand the potential performance impact of this change before deciding whether to invest the contributor/maintainer effort in making/reviewing such a change, but if it's significant then this could be a reasonable thing to do. |
The changes of For For The usages of these are, as I mentioned All potentially-runtime The proposed runtime So it seems that currently improving We could simplify the implementation in |
After thinking about possible workaround I conclude that the best would be not to use the broken variant/platform combinations. Any workaround that doesn't too much interfere the optimization might be itself optimized away. It is is only x86 and not x64, this isn't too much impact. |
Inspired by #4776
There are Clang intrinsics used in
_Mul_overflow
and_Add_overflow
MSVC has its own
_mul_full_overflow_u8
and others.Using them would certainly result in a better codegen than the generic implementation.
Caveats:
constexpr
, unlike the Clang one<intrin.h>
These might warrant the need of another internal header for
_Mul_overflow
and_Add_overflow
, used by<ranges>
,<mdspan>
, and<numeric>
after #4776.The text was updated successfully, but these errors were encountered: