[Reland][TLI] Add support for hypot libcall. #114343
Merged
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
llvmbot
added
llvm:analysis
Member
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Kenji Mouri / 毛利 研二 (MouriNaruto) ChangesThis patch adds basic support for Related issue: #113711 Note: It's my first time contributing to the LLVM with encouragement from one of my friends, @fawdlstty. I learned a lot from #99611, and thanks for that. Note: I had created the same PR and merged (#113724), but reverted caused by the merging issue. (The CI issue happened in 3 A.M. at my timezone.) So, I rebased to the latest main branch and recreate the PR and hope I won't have the third time to create the same PR. I hope @arsenm can help me review the code again. I’m sorry for that. Kenji Mouri Full diff: https://github.com/llvm/llvm-project/pull/114343.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index 3e23e398f6a797..fd53a26ef8fc11 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -1671,6 +1671,21 @@ TLI_DEFINE_ENUM_INTERNAL(htons)
TLI_DEFINE_STRING_INTERNAL("htons")
TLI_DEFINE_SIG_INTERNAL(Int16, Int16)
+/// double hypot(double x, double y);
+TLI_DEFINE_ENUM_INTERNAL(hypot)
+TLI_DEFINE_STRING_INTERNAL("hypot")
+TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl, Dbl)
+
+/// float hypotf(float x, float y);
+TLI_DEFINE_ENUM_INTERNAL(hypotf)
+TLI_DEFINE_STRING_INTERNAL("hypotf")
+TLI_DEFINE_SIG_INTERNAL(Flt, Flt, Flt)
+
+/// long double hypotl(long double x, long double y);
+TLI_DEFINE_ENUM_INTERNAL(hypotl)
+TLI_DEFINE_STRING_INTERNAL("hypotl")
+TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, LDbl)
+
/// int iprintf(const char *format, ...);
TLI_DEFINE_ENUM_INTERNAL(iprintf)
TLI_DEFINE_STRING_INTERNAL("iprintf")
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index 0ee83d217a5001..7f0b98ab3c1514 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -300,6 +300,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_expf);
TLI.setUnavailable(LibFunc_floorf);
TLI.setUnavailable(LibFunc_fmodf);
+ TLI.setUnavailable(LibFunc_hypotf);
TLI.setUnavailable(LibFunc_log10f);
TLI.setUnavailable(LibFunc_logf);
TLI.setUnavailable(LibFunc_modff);
@@ -331,6 +332,7 @@ static void initializeLibCalls(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_floorl);
TLI.setUnavailable(LibFunc_fmodl);
TLI.setUnavailable(LibFunc_frexpl);
+ TLI.setUnavailable(LibFunc_hypotl);
TLI.setUnavailable(LibFunc_ldexpl);
TLI.setUnavailable(LibFunc_log10l);
TLI.setUnavailable(LibFunc_logl);
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index 5fd4fd78c28a95..e039457f313b29 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -1215,6 +1215,9 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
case LibFunc_fmod:
case LibFunc_fmodf:
case LibFunc_fmodl:
+ case LibFunc_hypot:
+ case LibFunc_hypotf:
+ case LibFunc_hypotl:
case LibFunc_isascii:
case LibFunc_isdigit:
case LibFunc_labs:
diff --git a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
index d8266f4c6703dd..452d90aa98d88d 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
@@ -589,6 +589,15 @@ declare ptr @gets(ptr)
; CHECK: declare noundef i32 @gettimeofday(ptr nocapture noundef, ptr nocapture noundef) [[NOFREE_NOUNWIND]]
declare i32 @gettimeofday(ptr, ptr)
+; CHECK: declare double @hypot(double, double) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare double @hypot(double, double)
+
+; CHECK: declare float @hypotf(float, float) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare float @hypotf(float, float)
+
+; CHECK: declare x86_fp80 @hypotl(x86_fp80, x86_fp80) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
+declare x86_fp80 @hypotl(x86_fp80, x86_fp80)
+
; CHECK: declare i32 @isascii(i32) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
declare i32 @isascii(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 408b9c39934286..6702725e4fc8a4 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: 268
+# CHECK: == Total TLI yes SDK yes: 271
#
# 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: 267
+# WRONG_SUMMARY: == Total TLI yes SDK yes: 270
#
## 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 519 symbols, 286 available
-# AVAIL-COUNT-286: {{^}} available
+# AVAIL: TLI knows 522 symbols, 289 available
+# AVAIL-COUNT-289: {{^}} available
# AVAIL-NOT: {{^}} available
# UNAVAIL-COUNT-233: not available
# UNAVAIL-NOT: not available
@@ -602,6 +602,18 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
+ - Name: hypot
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: hypotf
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
+ - Name: hypotl
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
- Name: isdigit
Type: STT_FUNC
Section: .text
diff --git a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
index 98f8989d4e6e9e..982d00c5d33593 100644
--- a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
+++ b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
@@ -249,6 +249,9 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare %struct* @getpwnam(i8*)\n"
"declare i8* @gets(i8*)\n"
"declare i32 @gettimeofday(%struct*, i8*)\n"
+ "declare double @hypot(double, double)\n"
+ "declare float @hypotf(float, float)\n"
+ "declare x86_fp80 @hypotl(x86_fp80, x86_fp80)\n"
"declare i32 @_Z7isasciii(i32)\n"
"declare i32 @_Z7isdigiti(i32)\n"
"declare i64 @labs(i64)\n"
|
dtcxzyw
changed the title
c8ef
approved these changes
Oct 31, 2024
Contributor
c8ef
left a comment
c8ef
left a comment
There was a problem hiding this comment.
Generally looks good, thanks!
| TLI.setUnavailable(LibFunc_expf); | ||
| TLI.setUnavailable(LibFunc_floorf); | ||
| TLI.setUnavailable(LibFunc_fmodf); | ||
| TLI.setUnavailable(LibFunc_hypotf); |
Contributor
There was a problem hiding this comment.
I'm uncertain whether we should add this here because, according to the C standard, hypot was introduced in C99, not C89.
Contributor
Author
There was a problem hiding this comment.
I also have the same question because I also confused for that. I finally chose that place because I followed #99611 (I'm sorry. It's my first time contributing to that. So I chose to follow other people's way.) According to https://en.cppreference.com/w/c/numeric/math/remquo, the remquo series functions seems introduced in C99 but this commit adds remquo to the C89 code block I added the hypot to.
llvm-project/llvm/lib/Analysis/TargetLibraryInfo.cpp
Lines 291 to 319 in 6effab9
According to https://en.cppreference.com/w/c/numeric/math, that code block may should fix the comment because C math functions with ’f‘ and 'l' suffixes seem introduced in C99.
Contributor
There was a problem hiding this comment.
These functions are treated as a part of the system, not the language
Contributor
Author
There was a problem hiding this comment.
According to page 191 of https://web.archive.org/web/20200909074736if_/https://www.pdf-archive.com/2014/10/02/ansi-iso-9899-1990-1/ansi-iso-9899-1990-1.pdf, it seems the C89 doesn't have the C math functions with ’f‘ and 'l' suffixes.
Contributor
Author
There was a problem hiding this comment.
These functions are treated as a part of the system, not the language
I see. It solves my confusion.
arsenm
approved these changes
Oct 31, 2024
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
This patch adds basic support for `hypot`. Constant folding support will be submitted in a subsequent patch. Related issue: llvm#113711 Note: It's my first time contributing to the LLVM with encouragement from one of my friends, @fawdlstty. I learned a lot from llvm#99611, and thanks for that. Note: I had created the same PR and merged (llvm#113724), but reverted caused by the merging issue. (The CI issue happened in 3 A.M. at my timezone. So, I need to fall asleep again after I replied about why issue happened.) So, I rebased to the latest main branch and recreate the PR and hope I won't have the third time to create the same PR. I hope @arsenm can help me review the code again. I’m sorry for that. Kenji Mouri
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
This patch adds basic support for `hypot`. Constant folding support will be submitted in a subsequent patch. Related issue: llvm#113711 Note: It's my first time contributing to the LLVM with encouragement from one of my friends, @fawdlstty. I learned a lot from llvm#99611, and thanks for that. Note: I had created the same PR and merged (llvm#113724), but reverted caused by the merging issue. (The CI issue happened in 3 A.M. at my timezone. So, I need to fall asleep again after I replied about why issue happened.) So, I rebased to the latest main branch and recreate the PR and hope I won't have the third time to create the same PR. I hope @arsenm can help me review the code again. I’m sorry for that. Kenji Mouri
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This patch adds basic support for
hypot. Constant folding support will be submitted in a subsequent patch.Related issue: #113711
Note: It's my first time contributing to the LLVM with encouragement from one of my friends, @fawdlstty. I learned a lot from #99611, and thanks for that.
Note: I had created the same PR and merged (#113724), but reverted caused by the merging issue. (The CI issue happened in 3 A.M. at my timezone. So, I need to fall asleep again after I replied about why issue happened.) So, I rebased to the latest main branch and recreate the PR and hope I won't have the third time to create the same PR.
I hope @arsenm can help me review the code again. I’m sorry for that.
Kenji Mouri