-
Notifications
You must be signed in to change notification settings - Fork 69
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
Use hardware FMA #3007
Use hardware FMA #3007
Conversation
#endif | ||
|
||
// ⟦ab + c⟧. | ||
inline double FusedMultiplyAdd(double a, double b, double c); |
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.
Could we have tests for these functions to verify that we are calling the intrinsics correctly?
@@ -127,6 +135,7 @@ using internal_double_precision::Mod2π; | |||
using internal_double_precision::TwoDifference; | |||
using internal_double_precision::TwoProduct; | |||
using internal_double_precision::TwoSum; | |||
using internal_double_precision::VeltkampDekkerProduct; |
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.
Why would any client ever want to use this function? Shouldn't it be purely internal?
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.
See the comments:
Principia/numerics/double_precision.hpp
Lines 54 to 57 in 19073a6
// Returns the exact product of its arguments. Note that this function checks | |
// whether |UseHardwareFMA| is true. If the value of that flag is already known | |
// from context, it may be preferable to either: | |
// — use VeltkampDekkerProduct(a, b) below; |
You've added references to a test file in the projects, but not the test file itself. |
Also, why don't we change |
It doesn’t really make sense to inline that ; we don’t want to do CPUID calls every time we use something, instead we do them once and refer to the static result (here UseHardwareFMA). |
numerics/fma_test.cpp
Outdated
|
||
using base::CPUFeatureFlags; | ||
using base::HasCPUFeatures; | ||
using ::testing::Test; |
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.
We generally don't do a using
of this class, we use the full name when declaring the fixture. Also, we put the using
of symbols that start with ::
at the end of the list.
…or Voigt-Kampff/Dekker if we don’t have that. Avoid
std::fma
.