-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[TLI] Add basic support for nextafter/nexttoward libcalls #166250
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
[TLI] Add basic support for nextafter/nexttoward libcalls #166250
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
9fc5d57 to
f6a9fab
Compare
|
Even though it wasn't linked in the issue post, I suspect I'll need to follow the Instruction combining contributor guidelines and precommit the tests. I'll try to do that and clean up the PR later today. As an aside, I was somewhat unsure how to write the lit Edit: Regarding the CI failures, the implicit conversion is easy to fix (although I'm surprised my editor didn't catch that, I'll need to look into it). It seems like TargetLibraryInfo is complaining, hopefully I can figure out how to run specifically that test and fix it today. |
|
I'm going to restructure this PR to follow the example of #99611. This PR will first purely add |
Yes, it's easiest to split up these cases into separate tests |
f6a9fab to
93e4a83
Compare
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: Sayan Sivakumaran (sivakusayan) ChangesFirst patch for #74368. Constant folding will be added in a follow-up patch. Full diff: https://github.com/llvm/llvm-project/pull/166250.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index 014988299d37f..76b89dcb3f25d 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -1951,6 +1951,36 @@ TLI_DEFINE_ENUM_INTERNAL(nearbyintl)
TLI_DEFINE_STRING_INTERNAL("nearbyintl")
TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl)
+/// double nextafter(double x, double y);
+TLI_DEFINE_ENUM_INTERNAL(nextafter)
+TLI_DEFINE_STRING_INTERNAL("nextafter")
+TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl, Dbl)
+
+/// float nextafterf(float x, float y);
+TLI_DEFINE_ENUM_INTERNAL(nextafterf)
+TLI_DEFINE_STRING_INTERNAL("nextafterf")
+TLI_DEFINE_SIG_INTERNAL(Flt, Flt, Flt)
+
+/// long double nextafterl(long double x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(nextafterl)
+TLI_DEFINE_STRING_INTERNAL("nextafterl")
+TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl)
+
+/// double nexttoward(double x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(nexttoward)
+TLI_DEFINE_STRING_INTERNAL("nexttoward")
+TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl, LDbl)
+
+/// float nexttowardf(float x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(nexttowardf)
+TLI_DEFINE_STRING_INTERNAL("nexttowardf")
+TLI_DEFINE_SIG_INTERNAL(Flt, Flt, LDbl)
+
+/// long double nexttowardl(long double x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(nexttowardl)
+TLI_DEFINE_STRING_INTERNAL("nexttowardl")
+TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl)
+
/// uint32_t ntohl(uint32_t netlong);
TLI_DEFINE_ENUM_INTERNAL(ntohl)
TLI_DEFINE_STRING_INTERNAL("ntohl")
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index 813632c375308..74f3a7d131c35 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -388,6 +388,10 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setAvailableWithName(LibFunc_logbf, "_logbf");
else
TLI.setUnavailable(LibFunc_logbf);
+ TLI.setUnavailable(LibFunc_nextafter);
+ TLI.setUnavailable(LibFunc_nextafterf);
+ TLI.setUnavailable(LibFunc_nexttoward);
+ TLI.setUnavailable(LibFunc_nexttowardf);
TLI.setUnavailable(LibFunc_rint);
TLI.setUnavailable(LibFunc_rintf);
TLI.setUnavailable(LibFunc_round);
@@ -418,6 +422,8 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_logbl);
TLI.setUnavailable(LibFunc_ilogbl);
TLI.setUnavailable(LibFunc_nearbyintl);
+ TLI.setUnavailable(LibFunc_nextafterl);
+ TLI.setUnavailable(LibFunc_nexttowardl);
TLI.setUnavailable(LibFunc_rintl);
TLI.setUnavailable(LibFunc_roundl);
TLI.setUnavailable(LibFunc_scalblnl);
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index 573a78150ff3d..02b73e85d783f 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -1283,6 +1283,12 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
case LibFunc_ilogbl:
case LibFunc_logf:
case LibFunc_logl:
+ case LibFunc_nextafter:
+ case LibFunc_nextafterf:
+ case LibFunc_nextafterl:
+ case LibFunc_nexttoward:
+ case LibFunc_nexttowardf:
+ case LibFunc_nexttowardl:
case LibFunc_pow:
case LibFunc_powf:
case LibFunc_powl:
diff --git a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
index 51e22bb86f331..25a70a026a0b7 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
@@ -762,6 +762,24 @@ declare float @nearbyintf(float)
; CHECK: declare x86_fp80 @nearbyintl(x86_fp80) [[MEMNONE_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
declare x86_fp80 @nearbyintl(x86_fp80)
+; CHECK: declare double @nextafter(double, double) [[ERRNOMEMONLY_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
+declare double @nextafter(double, double)
+
+; CHECK: declare float @nextafterf(float, float) [[ERRNOMEMONLY_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
+declare float @nextafterf(float, float)
+
+; CHECK: declare x86_fp80 @nextafterl(x86_fp80, x86_fp80) [[ERRNOMEMONLY_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
+declare x86_fp80 @nextafterl(x86_fp80, x86_fp80)
+
+; CHECK: declare double @nexttoward(double, x86_fp80) [[ERRNOMEMONLY_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
+declare double @nexttoward(double, x86_fp80)
+
+; CHECK: declare float @nexttowardf(float, x86_fp80) [[ERRNOMEMONLY_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
+declare float @nexttowardf(float, x86_fp80)
+
+; CHECK: declare x86_fp80 @nexttowardl(x86_fp80, x86_fp80) [[ERRNOMEMONLY_NOFREE_NOUNWIND_WILLRETURN:#[0-9]+]]
+declare x86_fp80 @nexttowardl(x86_fp80, x86_fp80)
+
; CHECK-LINUX: declare noundef i32 @open(ptr noundef readonly captures(none), i32 noundef, ...) [[NOFREE]]
; CHECK-OPEN: declare noundef i32 @open(ptr noundef readonly captures(none), i32 noundef, ...) [[NOFREE:#[0-9]+]]
declare i32 @open(ptr, i32, ...)
diff --git a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
index 51a5a63ba370c..ff2c9ae00bdb9 100644
--- a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
+++ b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
@@ -34,7 +34,7 @@
#
# CHECK: << Total TLI yes SDK no: 18
# CHECK: >> Total TLI no SDK yes: 0
-# CHECK: == Total TLI yes SDK yes: 271
+# CHECK: == Total TLI yes SDK yes: 277
#
# WRONG_DETAIL: << TLI yes SDK no : '_ZdaPv' aka operator delete[](void*)
# WRONG_DETAIL: >> TLI no SDK yes: '_ZdaPvj' aka operator delete[](void*, unsigned int)
@@ -48,14 +48,14 @@
# WRONG_DETAIL: << TLI yes SDK no : 'fminimum_numl'
# WRONG_SUMMARY: << Total TLI yes SDK no: 19{{$}}
# WRONG_SUMMARY: >> Total TLI no SDK yes: 1{{$}}
-# WRONG_SUMMARY: == Total TLI yes SDK yes: 270
+# WRONG_SUMMARY: == Total TLI yes SDK yes: 276
#
## The -COUNT suffix doesn't care if there are too many matches, so check
## the exact count first; the two directives should add up to that.
## Yes, this means additions to TLI will fail this test, but the argument
## to -COUNT can't be an expression.
-# AVAIL: TLI knows 524 symbols, 289 available
-# AVAIL-COUNT-289: {{^}} available
+# AVAIL: TLI knows 530 symbols, 295 available
+# AVAIL-COUNT-295: {{^}} available
# AVAIL-NOT: {{^}} available
# UNAVAIL-COUNT-235: not available
# UNAVAIL-NOT: not available
@@ -778,6 +778,30 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
+ - Name: nextafter
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: nextafterf
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: nextafterl
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: nexttoward
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: nexttowardf
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: nexttowardl
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
- Name: perror
Type: STT_FUNC
Section: .text
diff --git a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
index b33419545efa8..787a32407ad95 100644
--- a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
+++ b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
@@ -277,6 +277,12 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare x86_fp80 @logbl(x86_fp80)\n"
"declare float @logf(float)\n"
"declare x86_fp80 @logl(x86_fp80)\n"
+ "declare double @nextafter(double, double)\n"
+ "declare float @nextafterf(float, float)\n"
+ "declare x86_fp80 @nextafterl(x86_fp80, x86_fp80)\n"
+ "declare double @nexttoward(double, x86_fp80)\n"
+ "declare float @nexttowardf(float, x86_fp80)\n"
+ "declare x86_fp80 @nexttowardl(x86_fp80, x86_fp80)\n"
"declare i8* @malloc(i64)\n"
"declare i8* @memccpy(i8*, i8*, i32, i64)\n"
"declare i8* @memchr(i8*, i32, i64)\n"
|
| - Name: nexttowardl | ||
| Type: STT_FUNC | ||
| Section: .text | ||
| Binding: STB_GLOBAL |
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 see that other similar patches added entries here, but I wasn't sure where to find authoritative documentation on what math functions are supported for PS4. Is there some obvious documentation I'm missing? The closest I was able to find is this wiki, which doesn't seem the most authoritative.
| TLI.setUnavailable(LibFunc_ilogbl); | ||
| TLI.setUnavailable(LibFunc_nearbyintl); | ||
| TLI.setUnavailable(LibFunc_nextafterl); | ||
| TLI.setUnavailable(LibFunc_nexttowardl); |
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 was bit confused by the comment:
// Win32 does not support long double C99 math functions.
as I believe this documentation implies long double support for many C99 functions goes back as early as MSVC 140. I haven't actually tried it myself though and might be misunderstanding something, so I'll just follow the pattern for now.
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.
This quote in the documentation linked in the code comments seems interesting:
Previous 16-bit versions of Microsoft C/C++ and Microsoft Visual C++ supported the long double type as an 80-bit precision floating-point data type. In later versions of Visual C++, the long double data type is a 64-bit precision floating-point data type identical to the double type. The compiler treats long double and double as distinct types, but the long double functions are identical to their double counterparts. The CRT provides long double versions of the math functions for ISO C99 source code compatibility, but note that the binary representation may differ from other compilers.
It's unfortunate that paragraph isn't more specific about the MSVC version, it's probably not worth worrying in this PR in any case.
|
@sivakusayan Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
First patch for #74368. Constant folding will be added in a follow-up patch.