-
Notifications
You must be signed in to change notification settings - Fork 1.6k
The Uncanny xmath Overhaul #5959
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
Open
StephanTLavavej
wants to merge
21
commits into
microsoft:main
Choose a base branch
from
StephanTLavavej:math-blaster-2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+318
−459
Conversation
This file contains hidden or 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
`_FDscale()` wasn't dllexported, so we can move it into an unnamed namespace.
`_FDnorm()` wasn't dllexported, so we can move it into an unnamed namespace.
It had no other effect. I'm adding `// underflow` versus `// overflow` comments to preserve clarity. `_Feraise()` wasn't dllexported. It was marked `__CLRCALL_PURE_OR_CDECL` but not `_CRTIMP2_PURE`. Remove now-unused macros.
They return the same values:
C:\Temp>type meow.cpp
#include <cmath>
#include <limits>
#include <print>
using namespace std;
static_assert(FP_SUBNORMAL == -2); // _DENORM
static_assert(FP_NORMAL == -1); // _FINITE
static_assert(FP_ZERO == 0);
static_assert(FP_INFINITE == 1); // _INFCODE
static_assert(FP_NAN == 2); // _NANCODE
extern "C" _CRTIMP2_PURE short __CLRCALL_PURE_OR_CDECL _Dtest(double*) noexcept;
void classify(double dbl) {
const short stl = _Dtest(&dbl);
const int crt = fpclassify(dbl);
println("dbl: {:6}; stl: {:2}; crt: {:2}; Equal: {}", dbl, stl, crt, stl == crt);
}
int main() {
classify(numeric_limits<double>::denorm_min());
classify(3.14);
classify(0.0);
classify(-0.0);
classify(numeric_limits<double>::infinity());
classify(numeric_limits<double>::quiet_NaN());
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp
meow.cpp
C:\Temp>meow
dbl: 5e-324; stl: -2; crt: -2; Equal: true
dbl: 3.14; stl: -1; crt: -1; Equal: true
dbl: 0; stl: 0; crt: 0; Equal: true
dbl: -0; stl: 0; crt: 0; Equal: true
dbl: inf; stl: 1; crt: 1; Equal: true
dbl: nan; stl: 2; crt: 2; Equal: true
Equivalence proof:
C:\Temp>type meow.cpp
#include <cmath>
#include <limits>
#include <print>
using namespace std;
union _Dval {
unsigned short _Sh[8];
double _Val;
};
union _Fval {
unsigned short _Sh[8];
float _Val;
};
#define _D0 3
#define _F0 1
#define DSIGN(x) (reinterpret_cast<_Dval*>(&(x))->_Sh[_D0] & _DSIGN)
#define FSIGN(x) (reinterpret_cast<_Fval*>(&(x))->_Sh[_F0] & _FSIGN)
void test_dbl(double dbl) {
const bool macro = static_cast<bool>(DSIGN(dbl));
println("dbl: {:4}; macro: {:5}; signbit: {:5}; Equal: {}", dbl, macro, signbit(dbl), macro == signbit(dbl));
}
void test_flt(float flt) {
const bool macro = static_cast<bool>(FSIGN(flt));
println("flt: {:4}; macro: {:5}; signbit: {:5}; Equal: {}", flt, macro, signbit(flt), macro == signbit(flt));
}
int main() {
test_dbl(numeric_limits<double>::infinity());
test_dbl(-numeric_limits<double>::infinity());
test_flt(numeric_limits<float>::infinity());
test_flt(-numeric_limits<float>::infinity());
}
C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp
meow.cpp
C:\Temp>meow
dbl: inf; macro: false; signbit: false; Equal: true
dbl: -inf; macro: true ; signbit: true ; Equal: true
flt: inf; macro: false; signbit: false; Equal: true
flt: -inf; macro: true ; signbit: true ; Equal: true
… namespace. It's okay to move these unions out of _EXTERN_C_UNLESS_PURE, because nothing else uses them.
…mespace. They obviously aren't dllexported.
They weren't dllexported. Although this results in a bit of code duplication, the result is more optimizer-friendly, and will allow us to eliminate xmath.hpp.
These are within an unnamed namespace, so we can alter their sizes. There was no actual harm here, since we never indexed into the nonexistent memory, but it was still risky, and likely to annoy static analysis someday.
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.
Followup to #5836.
I believe that there are no functional changes here. I've verified that the dllexport surface is unaffected.
Commits
_FDscale()wasn't dllexported, so we can move it into an unnamed namespace._FDnorm()wasn't dllexported, so we can move it into an unnamed namespace._Feraise()witherrno = ERANGE;.// underflowversus// overflowcomments to preserve clarity._Feraise()wasn't dllexported. It was marked__CLRCALL_PURE_OR_CDECLbut not_CRTIMP2_PURE. Remove now-unused macros.break;._Dtest()for bincompat, replaced withfpclassify()._FINITE=>FP_NORMAL_INFCODE=>FP_INFINITE_NANCODE=>FP_NAN0=>FP_ZERO, carefully._Dtest()macros.DSIGNandFSIGNwithsignbit()._Dval,_Fval, and their index macros into xexp.cpp's unnamed namespace._EXTERN_C_UNLESS_PURE, because nothing else uses them.constexpr int, cleanup comments._Xfe_overflow(),_Xfe_underflow()into xexp.cpp's unnamed namespace.extern const_Xbigand_FXbigwith replicatedconstexpr._Dval,_Fvalunion sizes.Equivalence Proofs
_DtestisfpclassifyDSIGNissignbit