[TLI] Add support for inferring attr cold/noreturn on std::terminate and __cxa_throw
#101622
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
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: None (goldsteinn) ChangesThese functions are both inherently on the error path so Full diff: https://github.com/llvm/llvm-project/pull/101622.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index 754f09c19fb35..1ad5277c6a4d9 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -491,6 +491,16 @@ TLI_DEFINE_ENUM_INTERNAL(Exit)
TLI_DEFINE_STRING_INTERNAL("_Exit")
TLI_DEFINE_SIG_INTERNAL(Void, Int)
+/// void std::terminate(void);
+TLI_DEFINE_ENUM_INTERNAL(terminate)
+TLI_DEFINE_STRING_INTERNAL("_ZSt9terminatev")
+TLI_DEFINE_SIG_INTERNAL(Void)
+
+/// void __cxa_throw(void *, void *, void (*)(void *));
+TLI_DEFINE_ENUM_INTERNAL(cxa_throw)
+TLI_DEFINE_STRING_INTERNAL("__cxa_throw")
+TLI_DEFINE_SIG_INTERNAL(Void, Ptr, Ptr, Ptr)
+
/// void __cxa_guard_abort(guard_t *guard);
/// guard_t is int64_t in Itanium ABI or int32_t on ARM eabi.
TLI_DEFINE_ENUM_INTERNAL(cxa_guard_abort)
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index 30a343b2c564e..c17a3461d7b46 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -49,6 +49,7 @@ STATISTIC(NumNoUndef, "Number of function returns inferred as noundef returns");
STATISTIC(NumReturnedArg, "Number of arguments inferred as returned");
STATISTIC(NumWillReturn, "Number of functions inferred as willreturn");
STATISTIC(NumCold, "Number of functions inferred as cold");
+STATISTIC(NumNoReturn, "Number of functions inferred as no return");
static bool setDoesNotAccessMemory(Function &F) {
if (F.doesNotAccessMemory())
@@ -66,6 +67,14 @@ static bool setIsCold(Function &F) {
return true;
}
+static bool setNoReturn(Function &F) {
+ if (F.hasFnAttribute(Attribute::NoReturn))
+ return false;
+ F.addFnAttr(Attribute::NoReturn);
+ ++NumNoReturn;
+ return true;
+}
+
static bool setOnlyAccessesInaccessibleMemory(Function &F) {
if (F.onlyAccessesInaccessibleMemory())
return false;
@@ -1102,6 +1111,15 @@ bool llvm::inferNonMandatoryLibFuncAttrs(Function &F,
case LibFunc_abort:
Changed |= setIsCold(F);
break;
+ case LibFunc_terminate:
+ Changed |= setIsCold(F);
+ Changed |= setNoReturn(F);
+ break;
+ case LibFunc_cxa_throw:
+ Changed |= setIsCold(F);
+ Changed |= setNoReturn(F);
+ // Don't add `nofree` on `__cxa_throw`
+ return Changed;
// int __nvvm_reflect(const char *)
case LibFunc_nvvm_reflect:
Changed |= setRetAndArgsNoUndef(F);
diff --git a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
index 3b914dc29ca41..7e03861bf1ad7 100644
--- a/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
+++ b/llvm/test/Transforms/InferFunctionAttrs/annotate.ll
@@ -1100,6 +1100,12 @@ declare i64 @write(i32, ptr, i64)
; CHECK: declare void @abort() [[NOFREE_COLD:#[0-9]+]]
declare void @abort()
+; CHECK: declare void @__cxa_throw() [[NOFREE_COLD_NORETURN:#[0-9]+]]
+declare void @__cxa_throw(ptr, ptr, ptr)
+
+; CHECK: declare void @_ZSt9terminatev() [[NOFREE_COLD_NORETURN:#[0-9]+]]
+declare void @_ZSt9terminatev()
+
; memset_pattern{4,8,16} aren't available everywhere.
; CHECK-DARWIN: declare void @memset_pattern4(ptr nocapture writeonly, ptr nocapture readonly, i64) [[ARGMEMONLY_NOFREE_NOUNWIND_WILLRETURN]]
declare void @memset_pattern4(ptr, ptr, i64)
@@ -1126,6 +1132,8 @@ declare void @memset_pattern16(ptr, ptr, i64)
; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGMEMONLY_NOUNWIND_WILLRETURN_ALLOCKIND_REALLOC_ALLOCSIZE1_FAMILY_MALLOC]] = { mustprogress nounwind willreturn allockind("realloc") allocsize(1) memory(argmem: readwrite, inaccessiblemem: readwrite) "alloc-family"="malloc" }
; CHECK-DAG: attributes [[INACCESSIBLEMEMORARGONLY_NOFREE_NOUNWIND_WILLRETURN_FAMILY_MALLOC]] = { mustprogress nofree nounwind willreturn memory(argmem: readwrite, inaccessiblemem: readwrite) "alloc-family"="malloc" }
; CHECK-DAG: attributes [[NOFREE_COLD]] = { cold nofree }
+; CHECK-DAG: attributes [[NOFREE_COLD_NORETURN]] = { cold nofree noreturn }
+; CHECK-DAG: attributes [[COLD_NORETURN]] = { cold noreturn }
; CHECK-NVPTX-DAG: attributes [[NOFREE_NOUNWIND_READNONE]] = { nofree nosync nounwind memory(none) }
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 81f2c9c55b54d..1f24561c58d3f 100644
--- a/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
+++ b/llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml
@@ -34,21 +34,21 @@
#
# CHECK: << Total TLI yes SDK no: 8
# CHECK: >> Total TLI no SDK yes: 0
-# CHECK: == Total TLI yes SDK yes: 248
+# CHECK: == Total TLI yes SDK yes: 250
#
# 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: 247
+# WRONG_SUMMARY: == Total TLI yes SDK yes: 249
#
## 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 489 symbols, 256 available
-# AVAIL-COUNT-256: {{^}} available
+# AVAIL: TLI knows 491 symbols, 258 available
+# AVAIL-COUNT-258: {{^}} available
# AVAIL-NOT: {{^}} available
# UNAVAIL-COUNT-233: not available
# UNAVAIL-NOT: not available
@@ -167,6 +167,10 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
+ - Name: __cxa_throw
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
- Name: __cxa_guard_release
Type: STT_FUNC
Section: .text
@@ -279,6 +283,10 @@ DynamicSymbols:
Type: STT_FUNC
Section: .text
Binding: STB_GLOBAL
+ - Name: _ZSt9terminatev
+ Type: STT_FUNC
+ Section: .text
+ Binding: STB_GLOBAL
- Name: atof
Type: STT_FUNC
Section: .text
diff --git a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
index d344ebe676799..0ba26466a4520 100644
--- a/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
+++ b/llvm/unittests/Analysis/TargetLibraryInfoTest.cpp
@@ -500,12 +500,14 @@ TEST_F(TargetLibraryInfoTest, ValidProto) {
"declare void @__cxa_guard_abort(%struct*)\n"
"declare i32 @__cxa_guard_acquire(%struct*)\n"
"declare void @__cxa_guard_release(%struct*)\n"
+ "declare void @__cxa_throw(ptr, ptr, ptr)\n"
"declare i32 @atexit(void ()*)\n"
"declare void @abort()\n"
"declare void @exit(i32)\n"
"declare void @_Exit(i32)\n"
+ "declare void @_ZSt9terminatev()\n"
"declare i32 @__nvvm_reflect(i8*)\n"
|
dtcxzyw
reviewed
Aug 2, 2024
dtcxzyw
reviewed
Aug 2, 2024
|
ping |
goldsteinn
force-pushed
the
goldsteinn/recognize-cxx-error-handling
branch
from
7ca7df8 to
cce0805
Compare
e-kud
reviewed
Sep 3, 2024
| @@ -517,6 +517,16 @@ TLI_DEFINE_ENUM_INTERNAL(Exit) | |||
| TLI_DEFINE_STRING_INTERNAL("_Exit") | |||
| TLI_DEFINE_SIG_INTERNAL(Void, Int) | |||
|
|
|||
| /// void std::terminate(); | |||
| TLI_DEFINE_ENUM_INTERNAL(terminate) | |||
There was a problem hiding this comment.
Shouldn't be here ZSt9terminatev similarly to other cpp functions? E.g.
TLI_DEFINE_ENUM_INTERNAL(ZdaPv)
TLI_DEFINE_STRING_INTERNAL("_ZdaPv")
| @@ -66,6 +67,14 @@ static bool setIsCold(Function &F) { | |||
| return true; | |||
| } | |||
|
|
|||
| static bool setNoReturn(Function &F) { | |||
| if (F.hasFnAttribute(Attribute::NoReturn)) | |||
There was a problem hiding this comment.
Wouldn't it better to use F.doesNotReturn() and setDoesNotReturn as it is in setDoesNotAccessMemory instead of checking attributes explicitly? And rename to setDoesNotReturn probably...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
These functions are both inherently on the error path so
coldseemsappropriate.
noreturnis definitional.