Skip to content

[TLI] Add support for nan libfunc #101356

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

Merged
merged 1 commit into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions llvm/include/llvm/Analysis/TargetLibraryInfo.def
Original file line number Diff line number Diff line change
Expand Up @@ -1807,6 +1807,21 @@ TLI_DEFINE_ENUM_INTERNAL(modfl)
TLI_DEFINE_STRING_INTERNAL("modfl")
TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl, Ptr)

/// double nan(const char *arg);
TLI_DEFINE_ENUM_INTERNAL(nan)
TLI_DEFINE_STRING_INTERNAL("nan")
TLI_DEFINE_SIG_INTERNAL(Dbl, Ptr)

/// float nanf(const char *arg);
TLI_DEFINE_ENUM_INTERNAL(nanf)
TLI_DEFINE_STRING_INTERNAL("nanf")
TLI_DEFINE_SIG_INTERNAL(Flt, Ptr)

/// long double nanl(const char *arg);
TLI_DEFINE_ENUM_INTERNAL(nanl)
TLI_DEFINE_STRING_INTERNAL("nanl")
TLI_DEFINE_SIG_INTERNAL(LDbl, Ptr)

/// double nearbyint(double x);
TLI_DEFINE_ENUM_INTERNAL(nearbyint)
TLI_DEFINE_STRING_INTERNAL("nearbyint")
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Transforms/Utils/BuildLibCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
Changed |= setNonLazyBind(F);

switch (TheLibFunc) {
case LibFunc_nan:
case LibFunc_nanf:
case LibFunc_nanl:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? These are spec'd to call strtod and friends, which set errno.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling
This function is not subject to any of the error conditions specified in math_errhandling.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LLVM's libc implementation sets errno: https://github.com/llvm/llvm-project/pull/76690/files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glibc also sets errno.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? These are spec'd to call strtod and friends, which set errno.

In this case, strtoX never sets ERANGE. So it looks ok.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#include <math.h>
#include <errno.h>
#include <stdio.h>

int main() {
	errno = 0;
	double d = nan("99999999999999999999999999999999999999999");
	int err = errno;
	printf("%d %f\n", err, d);
	return 0;
}

$ clang -lm test.c && ./a.out 
34 nan

With glibc. Is this a glibc bug?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conditions where strtod would hit an error shouldn't apply for nan though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strotod says for NaN sequences:

A character sequence NAN or NAN(n-char-sequenceopt) is interpreted as a quiet NaN, if supported in the return type, else like a subject sequence part that does not have the expected form; the meaning of the n-char sequence
is implementation-defined.

It gives explicit rules for overflow and underflow, which can set errno values, but is silent on what happens with the n-char sequence being out of range of the payload, outside of the above quote. But... there's not really any latitude I see in the specification for errno to be set (unless you think 7.12.1 applies to nan and sNaN is a domain error).

glibc fails because it's using strtoull here, and strtoull does ERANGE on overflow. But I think this is a glibc bug, it shouldn't be setting errno.

(I'm not fully conversant in llvm-libc implementation, but I think the llvm-libc implementation doesn't ever set errno in nan--the interface allows it to, but nothing appears to set an error).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case LibFunc_strlen:
case LibFunc_strnlen:
case LibFunc_wcslen:
Expand Down
11 changes: 10 additions & 1 deletion llvm/test/Transforms/InferFunctionAttrs/annotate.ll
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,15 @@ declare float @modff(float, ptr)
; CHECK: declare x86_fp80 @modfl(x86_fp80, ptr nocapture) [[ARGMEMONLY_NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
declare x86_fp80 @modfl(x86_fp80, ptr)

; CHECK: declare double @nan(ptr nocapture) [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN:#[0-9]+]]
declare double @nan(ptr)

; CHECK: declare float @nanf(ptr nocapture) [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]]
declare float @nanf(ptr)

; CHECK: declare x86_fp80 @nanl(ptr nocapture) [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]]
declare x86_fp80 @nanl(ptr)

; CHECK: declare double @nearbyint(double) [[NOFREE_NOUNWIND_WILLRETURN_WRITEONLY]]
declare double @nearbyint(double)

Expand Down Expand Up @@ -956,7 +965,7 @@ declare ptr @strncpy(ptr, ptr, i64)
; CHECK: declare noalias ptr @strndup(ptr nocapture readonly, i64 noundef) [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN_FAMILY_MALLOC]]
declare ptr @strndup(ptr, i64)

; CHECK: declare i64 @strnlen(ptr nocapture, i64) [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN:#[0-9]+]]
; CHECK: declare i64 @strnlen(ptr nocapture, i64) [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]]
declare i64 @strnlen(ptr, i64)

; CHECK: declare ptr @strpbrk(ptr, ptr nocapture) [[ARGMEMONLY_NOFREE_NOUNWIND_READONLY_WILLRETURN]]
Expand Down
20 changes: 16 additions & 4 deletions llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,21 @@
#
# CHECK: << Total TLI yes SDK no: 8
# CHECK: >> Total TLI no SDK yes: 0
# CHECK: == Total TLI yes SDK yes: 245
# CHECK: == Total TLI yes SDK yes: 248
#
# WRONG_DETAIL: << TLI yes SDK no : '_ZdaPv' aka operator delete[](void*)
# WRONG_DETAIL: >> TLI no SDK yes: '_ZdaPvj' aka operator delete[](void*, unsigned int)
# WRONG_DETAIL-COUNT-8: << TLI yes SDK no : {{.*}}__hot_cold_t
# WRONG_SUMMARY: << Total TLI yes SDK no: 9{{$}}
# WRONG_SUMMARY: >> Total TLI no SDK yes: 1{{$}}
# WRONG_SUMMARY: == Total TLI yes SDK yes: 244
# WRONG_SUMMARY: == Total TLI yes SDK yes: 247
#
## 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 486 symbols, 253 available
# AVAIL-COUNT-253: {{^}} available
# AVAIL: TLI knows 489 symbols, 256 available
# AVAIL-COUNT-256: {{^}} available
# AVAIL-NOT: {{^}} available
# UNAVAIL-COUNT-233: not available
# UNAVAIL-NOT: not available
Expand Down Expand Up @@ -703,6 +703,18 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
- Name: nan
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
- Name: nanf
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
- Name: nanl
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
- Name: nearbyint
Type: STT_FUNC
Section: .text
Expand Down
3 changes: 3 additions & 0 deletions llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare double @modf(double, double*)\n"
"declare float @modff(float, float*)\n"
"declare x86_fp80 @modfl(x86_fp80, x86_fp80*)\n"
"declare double @nan(ptr)\n"
"declare float @nanf(ptr)\n"
"declare x86_fp80 @nanl(ptr)\n"
"declare double @nearbyint(double)\n"
"declare float @nearbyintf(float)\n"
"declare x86_fp80 @nearbyintl(x86_fp80)\n"
Expand Down
Loading