[libc-commits] [libc] [llvm] [libc][OSUtil] refactor quick_exit to be an object library everywhere (PR #85955)
Nick Desaulniers via libc-commits
libc-commits at lists.llvm.org
Thu Mar 21 14:38:28 PDT 2024
https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/85955
>From 139c49518cc8c68c4e3ff2584252fb18872b9487 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Wed, 20 Mar 2024 09:12:27 -0700
Subject: [PATCH 1/4] [libc][OSUtil] refactor quick_exit to be an object
library everywhere
The usage of __builtin_unreachable after calls to quick_exit were distressing.
If a function is properly marked [[noreturn]] then __builtin_unreachable is not
necessary.
Looking into this further, we seem to have header only implementations for CPU
targets. The inline nature of these functions is curious; we're going to exit,
it doesn't matter if we need to pay the call of a function or not. If we just
make these functions have distinct TUs rather than be header only, we can clean
up the cmake rules for quick_exit which were different between CPU and GPU.
Remove darwin support for quick_exit. This isn't being tested, and we can bring
it back when necessary.
---
libc/src/__support/OSUtil/CMakeLists.txt | 27 +++++--------------
.../__support/OSUtil/baremetal/CMakeLists.txt | 5 ++--
.../__support/OSUtil/baremetal/quick_exit.h | 21 ---------------
.../__support/OSUtil/darwin/CMakeLists.txt | 1 -
libc/src/__support/OSUtil/darwin/quick_exit.h | 26 ------------------
libc/src/__support/OSUtil/gpu/CMakeLists.txt | 1 -
libc/src/__support/OSUtil/gpu/quick_exit.cpp | 9 ++-----
libc/src/__support/OSUtil/gpu/quick_exit.h | 18 -------------
.../src/__support/OSUtil/linux/CMakeLists.txt | 5 ++--
.../linux/{quick_exit.h => quick_exit.cpp} | 13 +++------
libc/src/__support/OSUtil/quick_exit.h | 15 +++--------
libc/src/stdlib/CMakeLists.txt | 5 ++--
libc/src/stdlib/_Exit.cpp | 3 +--
libc/src/stdlib/exit.cpp | 3 +--
14 files changed, 27 insertions(+), 125 deletions(-)
delete mode 100644 libc/src/__support/OSUtil/baremetal/quick_exit.h
delete mode 100644 libc/src/__support/OSUtil/darwin/quick_exit.h
delete mode 100644 libc/src/__support/OSUtil/gpu/quick_exit.h
rename libc/src/__support/OSUtil/linux/{quick_exit.h => quick_exit.cpp} (80%)
diff --git a/libc/src/__support/OSUtil/CMakeLists.txt b/libc/src/__support/OSUtil/CMakeLists.txt
index ca3b3bf1263e0a..94d1042ccbb4a0 100644
--- a/libc/src/__support/OSUtil/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/CMakeLists.txt
@@ -8,23 +8,10 @@ if(NOT TARGET ${target_os_util})
return()
endif()
-# The OSUtil is an object library in GPU mode.
-if(NOT LIBC_TARGET_OS_IS_GPU)
- add_header_library(
- osutil
- HDRS
- io.h
- quick_exit.h
- syscall.h
- DEPENDS
- ${target_os_util}
- )
-else()
- add_object_library(
- osutil
- ALIAS
- ${target_os_util}
- DEPENDS
- ${target_os_util}
- )
-endif()
+add_object_library(
+ osutil
+ ALIAS
+ ${target_os_util}
+ DEPENDS
+ ${target_os_util}
+)
diff --git a/libc/src/__support/OSUtil/baremetal/CMakeLists.txt b/libc/src/__support/OSUtil/baremetal/CMakeLists.txt
index 280ff87cf1470d..23da40326bbb72 100644
--- a/libc/src/__support/OSUtil/baremetal/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/baremetal/CMakeLists.txt
@@ -1,8 +1,9 @@
-add_header_library(
+add_object_library(
baremetal_util
+ SRCS
+ quick_exit.cpp
HDRS
io.h
- quick_exit.h
DEPENDS
libc.src.__support.common
libc.src.__support.CPP.string_view
diff --git a/libc/src/__support/OSUtil/baremetal/quick_exit.h b/libc/src/__support/OSUtil/baremetal/quick_exit.h
deleted file mode 100644
index 74f9142e21b81e..00000000000000
--- a/libc/src/__support/OSUtil/baremetal/quick_exit.h
+++ /dev/null
@@ -1,21 +0,0 @@
-//===----- Baremetal implementation of a quick exit function ----*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_BAREMETAL_QUICK_EXIT_H
-#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_BAREMETAL_QUICK_EXIT_H
-
-namespace LIBC_NAMESPACE {
-
-// This is intended to be provided by the vendor.
-extern "C" void __llvm_libc_quick_exit(int status);
-
-void quick_exit(int status) { __llvm_libc_quick_exit(status); }
-
-} // namespace LIBC_NAMESPACE
-
-#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_BAREMETAL_QUICK_EXIT_H
diff --git a/libc/src/__support/OSUtil/darwin/CMakeLists.txt b/libc/src/__support/OSUtil/darwin/CMakeLists.txt
index a3e6b93c8e9925..4241bb37684f7c 100644
--- a/libc/src/__support/OSUtil/darwin/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/darwin/CMakeLists.txt
@@ -8,7 +8,6 @@ add_header_library(
darwin_util
HDRS
io.h
- quick_exit.h
syscall.h
DEPENDS
.${LIBC_TARGET_ARCHITECTURE}.darwin_util
diff --git a/libc/src/__support/OSUtil/darwin/quick_exit.h b/libc/src/__support/OSUtil/darwin/quick_exit.h
deleted file mode 100644
index 71647f50def5e2..00000000000000
--- a/libc/src/__support/OSUtil/darwin/quick_exit.h
+++ /dev/null
@@ -1,26 +0,0 @@
-//===--------- Darwin implementation of a quick exit function ---*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_DARWIN_QUICK_EXIT_H
-#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_DARWIN_QUICK_EXIT_H
-
-#include "syscall.h" // For internal syscall function.
-
-#include "src/__support/common.h"
-
-namespace LIBC_NAMESPACE {
-
-LIBC_INLINE void quick_exit(int status) {
- for (;;) {
- LIBC_NAMESPACE::syscall_impl<long>(1 /* SYS_exit */, status);
- }
-}
-
-} // namespace LIBC_NAMESPACE
-
-#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_DARWIN_QUICK_EXIT_H
diff --git a/libc/src/__support/OSUtil/gpu/CMakeLists.txt b/libc/src/__support/OSUtil/gpu/CMakeLists.txt
index 8e892a7000c9b3..0c89f9223678b1 100644
--- a/libc/src/__support/OSUtil/gpu/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/gpu/CMakeLists.txt
@@ -4,7 +4,6 @@ add_object_library(
quick_exit.cpp
io.cpp
HDRS
- quick_exit.h
io.h
DEPENDS
libc.src.__support.common
diff --git a/libc/src/__support/OSUtil/gpu/quick_exit.cpp b/libc/src/__support/OSUtil/gpu/quick_exit.cpp
index 1a03be0ace6728..af4795905e7868 100644
--- a/libc/src/__support/OSUtil/gpu/quick_exit.cpp
+++ b/libc/src/__support/OSUtil/gpu/quick_exit.cpp
@@ -6,17 +6,14 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_GPU_QUICK_EXIT_H
-#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_GPU_QUICK_EXIT_H
-
-#include "quick_exit.h"
+#include "src/__support/OSUtil/quick_exit.h"
#include "src/__support/RPC/rpc_client.h"
#include "src/__support/macros/properties/architectures.h"
namespace LIBC_NAMESPACE {
-void quick_exit(int status) {
+[[noreturn]] void quick_exit(int status) {
// We want to first make sure the server is listening before we exit.
rpc::Client::Port port = rpc::client.open<RPC_EXIT>();
port.send_and_recv([](rpc::Buffer *) {}, [](rpc::Buffer *) {});
@@ -29,5 +26,3 @@ void quick_exit(int status) {
}
} // namespace LIBC_NAMESPACE
-
-#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_GPU_QUICK_EXIT_H
diff --git a/libc/src/__support/OSUtil/gpu/quick_exit.h b/libc/src/__support/OSUtil/gpu/quick_exit.h
deleted file mode 100644
index b51385defbc0de..00000000000000
--- a/libc/src/__support/OSUtil/gpu/quick_exit.h
+++ /dev/null
@@ -1,18 +0,0 @@
-//===---------- GPU implementation of a quick exit function -----*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_GPU_QUICK_EXIT_H
-#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_GPU_QUICK_EXIT_H
-
-namespace LIBC_NAMESPACE {
-
-void quick_exit(int status);
-
-} // namespace LIBC_NAMESPACE
-
-#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_GPU_QUICK_EXIT_H
diff --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index c27f9be7464894..239d115704927b 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -4,11 +4,12 @@ endif()
add_subdirectory(${LIBC_TARGET_ARCHITECTURE})
-add_header_library(
+add_object_library(
linux_util
+ SRCS
+ quick_exit.cpp
HDRS
io.h
- quick_exit.h
syscall.h
DEPENDS
.${LIBC_TARGET_ARCHITECTURE}.linux_${LIBC_TARGET_ARCHITECTURE}_util
diff --git a/libc/src/__support/OSUtil/linux/quick_exit.h b/libc/src/__support/OSUtil/linux/quick_exit.cpp
similarity index 80%
rename from libc/src/__support/OSUtil/linux/quick_exit.h
rename to libc/src/__support/OSUtil/linux/quick_exit.cpp
index 432395584d8467..bb83c5f0c2ad12 100644
--- a/libc/src/__support/OSUtil/linux/quick_exit.h
+++ b/libc/src/__support/OSUtil/linux/quick_exit.cpp
@@ -6,13 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
-#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
-
-#include "syscall.h" // For internal syscall function.
-
#include "src/__support/common.h"
-
+#include "syscall.h" // For internal syscall function.
#include <sys/syscall.h> // For syscall numbers.
namespace LIBC_NAMESPACE {
@@ -22,8 +17,8 @@ namespace LIBC_NAMESPACE {
#ifdef LIBC_TARGET_ARCH_IS_X86
__attribute__((no_stack_protector))
#endif
-LIBC_INLINE void
-quick_exit(int status) {
+__attribute__((noreturn))
+void quick_exit(int status) {
for (;;) {
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit_group, status);
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit, status);
@@ -31,5 +26,3 @@ quick_exit(int status) {
}
} // namespace LIBC_NAMESPACE
-
-#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_LINUX_QUICK_EXIT_H
diff --git a/libc/src/__support/OSUtil/quick_exit.h b/libc/src/__support/OSUtil/quick_exit.h
index 6c59c1afcda254..e445917059c3e1 100644
--- a/libc/src/__support/OSUtil/quick_exit.h
+++ b/libc/src/__support/OSUtil/quick_exit.h
@@ -9,17 +9,10 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_OSUTIL_QUICK_EXIT_H
#define LLVM_LIBC_SRC___SUPPORT_OSUTIL_QUICK_EXIT_H
-#include "src/__support/macros/properties/architectures.h"
+namespace LIBC_NAMESPACE {
-#if defined(LIBC_TARGET_ARCH_IS_GPU)
-#include "gpu/quick_exit.h"
-#elif defined(__APPLE__)
-#include "darwin/quick_exit.h"
-#elif defined(__linux__)
-#include "linux/quick_exit.h"
-#elif defined(__ELF__)
-// TODO: Ideally we would have LIBC_TARGET_OS_IS_BAREMETAL.
-#include "baremetal/quick_exit.h"
-#endif
+[[noreturn]] void quick_exit(int status);
+
+}
#endif // LLVM_LIBC_SRC___SUPPORT_OSUTIL_QUICK_EXIT_H
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 22f7f990fb08a8..76de9eb5579b88 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -394,10 +394,11 @@ add_entrypoint_object(
CXX_STANDARD
20 # For constinit of the atexit callback list.
DEPENDS
- libc.src.__support.fixedvector
+ libc.src.__support.CPP.new
+ libc.src.__support.OSUtil.osutil
libc.src.__support.blockstore
+ libc.src.__support.fixedvector
libc.src.__support.threads.mutex
- libc.src.__support.CPP.new
)
add_entrypoint_object(
diff --git a/libc/src/stdlib/_Exit.cpp b/libc/src/stdlib/_Exit.cpp
index 85684d1e90879e..233af209739244 100644
--- a/libc/src/stdlib/_Exit.cpp
+++ b/libc/src/stdlib/_Exit.cpp
@@ -13,9 +13,8 @@
namespace LIBC_NAMESPACE {
-LLVM_LIBC_FUNCTION(void, _Exit, (int status)) {
+[[noreturn]] LLVM_LIBC_FUNCTION(void, _Exit, (int status)) {
quick_exit(status);
- __builtin_unreachable();
}
} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index cc5ae6648d11f2..94cf2ae055fc30 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -16,10 +16,9 @@ namespace internal {
void call_exit_callbacks();
}
-LLVM_LIBC_FUNCTION(void, exit, (int status)) {
+[[noreturn]] LLVM_LIBC_FUNCTION(void, exit, (int status)) {
internal::call_exit_callbacks();
quick_exit(status);
- __builtin_unreachable();
}
} // namespace LIBC_NAMESPACE
>From fb7baf77387298a145848292730dd89939d1cbee Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 21 Mar 2024 14:29:34 -0700
Subject: [PATCH 2/4] fix bazel
---
utils/bazel/llvm-project-overlay/libc/BUILD.bazel | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 0d531a3dc12a7c..d47a9929574154 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -999,10 +999,7 @@ libc_support_library(
libc_support_library(
name = "__support_osutil_quick_exit",
hdrs = ["src/__support/OSUtil/quick_exit.h"],
- textual_hdrs = [
- "src/__support/OSUtil/linux/quick_exit.h",
- #TODO: add support for GPU quick_exit (isn't just in a header.)
- ],
+ srcs = ["src/__support/OSUtil/linux/quick_exit.cpp"],
deps = [
":__support_osutil_syscall",
],
>From 9ceeba4d5d7db1d58eeea33c31e46e9c5aea60a1 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 21 Mar 2024 14:35:24 -0700
Subject: [PATCH 3/4] use C++ attribute syntax
---
libc/src/__support/OSUtil/linux/quick_exit.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libc/src/__support/OSUtil/linux/quick_exit.cpp b/libc/src/__support/OSUtil/linux/quick_exit.cpp
index bb83c5f0c2ad12..a9b25656e33221 100644
--- a/libc/src/__support/OSUtil/linux/quick_exit.cpp
+++ b/libc/src/__support/OSUtil/linux/quick_exit.cpp
@@ -15,9 +15,9 @@ namespace LIBC_NAMESPACE {
// mark as no_stack_protector for x86 since TLS can be torn down before calling
// quick_exit so that the stack protector canary cannot be loaded.
#ifdef LIBC_TARGET_ARCH_IS_X86
-__attribute__((no_stack_protector))
+[[gnu::no_stack_protector]]
#endif
-__attribute__((noreturn))
+[[noreturn]]
void quick_exit(int status) {
for (;;) {
LIBC_NAMESPACE::syscall_impl<long>(SYS_exit_group, status);
>From cef3991578ada645bfe2b6bac2f4baefa80020bf Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 21 Mar 2024 14:38:15 -0700
Subject: [PATCH 4/4] reformat
---
libc/src/__support/OSUtil/linux/quick_exit.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/src/__support/OSUtil/linux/quick_exit.cpp b/libc/src/__support/OSUtil/linux/quick_exit.cpp
index a9b25656e33221..1298ab4f122e93 100644
--- a/libc/src/__support/OSUtil/linux/quick_exit.cpp
+++ b/libc/src/__support/OSUtil/linux/quick_exit.cpp
@@ -7,7 +7,7 @@
//===----------------------------------------------------------------------===//
#include "src/__support/common.h"
-#include "syscall.h" // For internal syscall function.
+#include "syscall.h" // For internal syscall function.
#include <sys/syscall.h> // For syscall numbers.
namespace LIBC_NAMESPACE {
More information about the libc-commits
mailing list