Skip to content
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

[libc++] Mark libc++ deallocation helpers as noexcept #110884

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Oct 2, 2024

They already can't throw exceptions and they are called from noexcept functions, but they were not marked as noexcept. Depending on compiler inlining, this might not make a difference or this might improve the codegen a bit by removing the implicit try-catch block that Clang generates around non-noexcept functions called from noexcept functions.

The original issue also mentioned that one occurrence of std::allocator::deallocate was missing noexcept, however it has since then been removed.

Fixes #66100

They already can't throw exceptions and they are called from noexcept
functions, but they were not marked as noexcept. Depending on compiler
inlining, this might not make a difference or this might improve the
codegen a bit by removing the implicit try-catch block that Clang
generates around non-noexcept functions called from noexcept functions.

The original issue also mentioned that one occurrence of
std::allocator::deallocate was missing noexcept, however
it has since then been removed.

Fixes llvm#66100
@ldionne ldionne requested a review from a team as a code owner October 2, 2024 16:49
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 2, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 2, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

They already can't throw exceptions and they are called from noexcept functions, but they were not marked as noexcept. Depending on compiler inlining, this might not make a difference or this might improve the codegen a bit by removing the implicit try-catch block that Clang generates around non-noexcept functions called from noexcept functions.

The original issue also mentioned that one occurrence of std::allocator::deallocate was missing noexcept, however it has since then been removed.

Fixes #66100


Full diff: https://github.com/llvm/llvm-project/pull/110884.diff

1 Files Affected:

  • (modified) libcxx/include/new (+4-4)
diff --git a/libcxx/include/new b/libcxx/include/new
index 3252b0bb1abcdb..8d7d1eb5fdc7b9 100644
--- a/libcxx/include/new
+++ b/libcxx/include/new
@@ -275,7 +275,7 @@ _LIBCPP_HIDE_FROM_ABI void* __libcpp_operator_new(_Args... __args) {
 }
 
 template <class... _Args>
-_LIBCPP_HIDE_FROM_ABI void __libcpp_operator_delete(_Args... __args) {
+_LIBCPP_HIDE_FROM_ABI void __libcpp_operator_delete(_Args... __args) _NOEXCEPT {
 #if __has_builtin(__builtin_operator_new) && __has_builtin(__builtin_operator_delete)
   __builtin_operator_delete(__args...);
 #else
@@ -296,7 +296,7 @@ inline _LIBCPP_HIDE_FROM_ABI void* __libcpp_allocate(size_t __size, size_t __ali
 }
 
 template <class... _Args>
-_LIBCPP_HIDE_FROM_ABI void __do_deallocate_handle_size(void* __ptr, size_t __size, _Args... __args) {
+_LIBCPP_HIDE_FROM_ABI void __do_deallocate_handle_size(void* __ptr, size_t __size, _Args... __args) _NOEXCEPT {
 #ifdef _LIBCPP_HAS_NO_SIZED_DEALLOCATION
   (void)__size;
   return std::__libcpp_operator_delete(__ptr, __args...);
@@ -305,7 +305,7 @@ _LIBCPP_HIDE_FROM_ABI void __do_deallocate_handle_size(void* __ptr, size_t __siz
 #endif
 }
 
-inline _LIBCPP_HIDE_FROM_ABI void __libcpp_deallocate(void* __ptr, size_t __size, size_t __align) {
+inline _LIBCPP_HIDE_FROM_ABI void __libcpp_deallocate(void* __ptr, size_t __size, size_t __align) _NOEXCEPT {
 #if defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION)
   (void)__align;
   return __do_deallocate_handle_size(__ptr, __size);
@@ -319,7 +319,7 @@ inline _LIBCPP_HIDE_FROM_ABI void __libcpp_deallocate(void* __ptr, size_t __size
 #endif
 }
 
-inline _LIBCPP_HIDE_FROM_ABI void __libcpp_deallocate_unsized(void* __ptr, size_t __align) {
+inline _LIBCPP_HIDE_FROM_ABI void __libcpp_deallocate_unsized(void* __ptr, size_t __align) _NOEXCEPT {
 #if defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION)
   (void)__align;
   return __libcpp_operator_delete(__ptr);

@EricWF
Copy link
Member

EricWF commented Oct 2, 2024

What effect does this have on codegen?

@ldionne
Copy link
Member Author

ldionne commented Oct 2, 2024

What effect does this have on codegen?

Depending on compiler inlining, this might not make a difference or this might improve the codegen a bit by removing the implicit try-catch block that Clang generates around non-noexcept functions called from noexcept functions.

Here's an example using Godbolt: https://godbolt.org/z/47d5P86Kz

Screenshot 2024-10-02 at 15 04 36

You'll have to zoom in, but on the left you have without my patch, and on the right with my patch. You can see that after my patch, we don't generate a try-catch block that calls __clang_call_terminate inside __libcpp_deallocate. The moment you enable optimizations (even -O1), there's no difference anymore.

So this is almost NFC, but the code is more correct this way anyway so we might as well do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libcxx header files: Missing _NOEXCEPT specifier on some allocator functions.
3 participants