-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[APFloat] Add exp function for APFloat::IEEESsingle using expf implementation from LLVM libc. #143959
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
base: main
Are you sure you want to change the base?
[APFloat] Add exp function for APFloat::IEEESsingle using expf implementation from LLVM libc. #143959
Changes from 11 commits
86174cc
98d27b2
e7c6d12
c09dce9
2c9c56a
884e4d1
e1a65ad
b6afbb0
1fb5a69
57ff4aa
aa44b1f
51a2fc8
9c87a7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,9 @@ | |
#include <cstring> | ||
#include <limits.h> | ||
|
||
// Shared headers from LLVM libc | ||
#include "shared/math.h" | ||
|
||
#define APFLOAT_DISPATCH_ON_SEMANTICS(METHOD_CALL) \ | ||
do { \ | ||
if (usesLayout<IEEEFloat>(getSemantics())) \ | ||
|
@@ -5601,6 +5604,29 @@ float APFloat::convertToFloat() const { | |
return Temp.getIEEE().convertToFloat(); | ||
} | ||
|
||
static constexpr int getFEnvRoundingMode(llvm::RoundingMode rm) { | ||
switch (rm) { | ||
case APFloat::rmTowardPositive: | ||
return FE_UPWARD; | ||
case APFloat::rmTowardNegative: | ||
return FE_DOWNWARD; | ||
case APFloat::rmTowardZero: | ||
return FE_TOWARDZERO; | ||
default: | ||
// TODO: fix rmNearestTiesToAway for platform without FE_TONEARESTFROMZERO. | ||
return FE_TONEAREST; | ||
}; | ||
} | ||
|
||
APFloat exp(const APFloat &X, RoundingMode rounding_mode) { | ||
if (&X.getSemantics() == &semIEEEsingle) { | ||
float result = LIBC_NAMESPACE::shared::expf( | ||
X.convertToFloat(), getFEnvRoundingMode(rounding_mode)); | ||
return APFloat(result); | ||
Comment on lines
+6175
to
+6177
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this provide different results depending on whether or not the platform we are running against uses DAZ or FTZ? It seems a bit confusing to rely on host numerics in a function which goes from APFloat -> APFloat. I would recommend that APFloat just have its own copy of the implementation, that way the result of the computation won't depend on the host's ambient conditions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If someone compiles or runs clang/llvm with FTZ/DAZ, do they really expect the results related to floating point denormals would be consistent with clang/llvm without FTZ/DAZ? Even with a perfect implementation of these functions using entirely integer arithmetics, every time a The implementations we have for these math functions are as consistent as we could, and the unit tests added in this PRs also guard against being compiled or run with FTZ/DAZ mode. If some users really want to build or run clang/llvm with FTZ/DAZ for specific targets, we could in theory perform a clear-and-restore those flags in the directed rounding functions. But there will be a lot more places that they need to worry about in that case. I don't think even MPFR could guarantee correct rounding when building/running under FTZ/DAZ flags. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm somewhat concerned about this too. But I've already manually verified that the proposed tests for this method will fail if DAZ/FTZ are enabled, and given that the current state of affairs is that we just blindly call host Personally, I'd think it best if the implementation just worked off of |
||
} | ||
assert(false && "Unsupported floating-point semantics"); | ||
} | ||
|
||
} // namespace llvm | ||
|
||
#undef APFLOAT_DISPATCH_ON_SEMANTICS |
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.
Use the pragma to enable fenv_access?
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.
Done.
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.
I would still be more comfortable if you just add an exp implementation with a template argument for the rounding mode, and didn't use fenv access anywhere in the stack. This has just merely shuffled the problem into the libc code directly
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.
At some point I'll implement a version of directed rounding math functions without relying on adjusting the floating point environment.
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.
I'm kind of surprised that setting the rounding mode for a foo() implementation results in a foo() result with the correct rounding mode. Are the transcendental functions in libc specifically implemented in a way that satisfies that property?
Can you point me to where libc is testing these functions in different rounding modes?
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.
@nikic: Yes, at the moment, all LLVM libc transcendental math functions are rounded toward the current (dynamic) floating point environment rounding mode (https://libc.llvm.org/headers/math/index.html#implementation-requirements-goals), and we test them for all 4 rounding modes (FE_NEAREST, FE_UPWARD, FE_DOWNWARD, and FE_TOWARDZERO) against MPFR outputs.
Some examples in our test suites:
using the test template: https://github.com/llvm/llvm-project/blob/main/libc/test/src/math/exhaustive/exhaustive_test.h#L213
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.
Awesome, thanks for the references!