De-pessimize SigSetConverter usage.
While looking at the disassembly for the epoll stuff I noticed that this
expands to quite a lot of code that the compiler can't optimize out for
LP64 (because it doesn't know that the "copy the argument into a local
and then use the local" bit isn't important).
There are two obvious options here. Something like this:
```
int signalfd64(int fd, const sigset64_t* mask, int flags) {
return __signalfd4(fd, mask, sizeof(*mask), flags);
}
int signalfd(int fd, const sigset_t* mask, int flags) {
#if defined(__LP64__)
return signalfd64(fd, mask, flags);
#else
SigSetConverter set = {.sigset = *mask};
return signalfd64(fd, &set.sigset64, flags);
#endif
}
```
Or something like this:
```
int signalfd64(int fd, const sigset64_t* mask, int flags) {
return __signalfd4(fd, mask, sizeof(*mask), flags);
}
#if defined(__LP64__)
__strong_alias(signalfd, signalfd64);
#else
int signalfd(int fd, const sigset_t* mask, int flags) {
SigSetConverter set = {};
set.sigset = *mask;
return signalfd64(fd, &set.sigset64, flags);
}
#endif
```
The former is slightly more verbose, but seems a bit more obvious, so I
initially went with that. (The former is more verbose in the generated
code too, given that the latter expands to _no_ code, just another symbol
pointing to the same code address.)
Having done that, I realized that slight changes to the interface would
let clang optimize away most/all of the overhead for LP64 with the only
preprocessor hackery being in SigSetConverter itself.
I also pulled out the legacy bsd `int` conversions since they're only
used in two (secret!) functions, so it's clearer to just have a separate
union for them. While doing so, I suppressed those functions for
riscv64, since there's no reason to keep carrying that mistake forward.
posix_spawn() is another simple case that doesn't actually benefit from
SigSetConverter, so I've given that its own anonymous union too.
Test: treehugger
Change-Id: Iaf67486da40d40fc53ec69717c3492ab7ab81ad6
diff --git a/libc/bionic/poll.cpp b/libc/bionic/poll.cpp
index 3290315..7d80b4c 100644
--- a/libc/bionic/poll.cpp
+++ b/libc/bionic/poll.cpp
@@ -50,14 +50,8 @@
int ppoll(pollfd* fds, nfds_t fd_count, const timespec* ts, const sigset_t* ss) {
// The underlying `__ppoll` system call only takes `sigset64_t`.
- SigSetConverter set;
- sigset64_t* ss_ptr = nullptr;
- if (ss != nullptr) {
- set = {};
- set.sigset = *ss;
- ss_ptr = &set.sigset64;
- }
- return ppoll64(fds, fd_count, ts, ss_ptr);
+ SigSetConverter set{ss};
+ return ppoll64(fds, fd_count, ts, set.ptr);
}
int ppoll64(pollfd* fds, nfds_t fd_count, const timespec* ts, const sigset64_t* ss) {
@@ -99,14 +93,8 @@
int pselect(int fd_count, fd_set* read_fds, fd_set* write_fds, fd_set* error_fds,
const timespec* ts, const sigset_t* ss) {
// The underlying `__pselect6` system call only takes `sigset64_t`.
- SigSetConverter set;
- sigset64_t* ss_ptr = nullptr;
- if (ss != nullptr) {
- set = {};
- set.sigset = *ss;
- ss_ptr = &set.sigset64;
- }
- return pselect64(fd_count, read_fds, write_fds, error_fds, ts, ss_ptr);
+ SigSetConverter set{ss};
+ return pselect64(fd_count, read_fds, write_fds, error_fds, ts, set.ptr);
}
int pselect64(int fd_count, fd_set* read_fds, fd_set* write_fds, fd_set* error_fds,
diff --git a/libc/bionic/signal.cpp b/libc/bionic/signal.cpp
index b581b5a..2cf9940 100644
--- a/libc/bionic/signal.cpp
+++ b/libc/bionic/signal.cpp
@@ -76,13 +76,23 @@
return SigAddSet(set, sig);
}
-// This isn't in our header files, but is exposed on all architectures.
+union BsdSigSet {
+ int mask;
+ sigset64_t set;
+};
+
+// This isn't in our header files, but is exposed on all architectures except riscv64.
extern "C" int sigblock(int mask) {
- SigSetConverter in, out;
- sigemptyset(&in.sigset);
- in.bsd = mask;
- if (sigprocmask(SIG_BLOCK, &in.sigset, &out.sigset) == -1) return -1;
- return out.bsd;
+ BsdSigSet in{.mask = mask}, out;
+ if (sigprocmask64(SIG_BLOCK, &in.set, &out.set) == -1) return -1;
+ return out.mask;
+}
+
+// This isn't in our header files, but is exposed on all architectures except riscv64.
+extern "C" int sigsetmask(int mask) {
+ BsdSigSet in{.mask = mask}, out;
+ if (sigprocmask64(SIG_SETMASK, &in.set, &out.set) == -1) return -1;
+ return out.mask;
}
template <typename SigSetT>
@@ -198,10 +208,9 @@
}
int sigpending(sigset_t* bionic_set) {
- SigSetConverter set = {};
- set.sigset = *bionic_set;
- if (__rt_sigpending(&set.sigset64, sizeof(set.sigset64)) == -1) return -1;
- *bionic_set = set.sigset;
+ SigSetConverter set{bionic_set};
+ if (__rt_sigpending(set.ptr, sizeof(sigset64_t)) == -1) return -1;
+ set.copy_out();
return 0;
}
@@ -245,19 +254,9 @@
return sigismember64(&old_mask, sig) ? SIG_HOLD : old_sa.sa_handler;
}
-// This isn't in our header files, but is exposed on all architectures.
-extern "C" int sigsetmask(int mask) {
- SigSetConverter in, out;
- sigemptyset(&in.sigset);
- in.bsd = mask;
- if (sigprocmask(SIG_SETMASK, &in.sigset, &out.sigset) == -1) return -1;
- return out.bsd;
-}
-
int sigsuspend(const sigset_t* bionic_set) {
- SigSetConverter set = {};
- set.sigset = *bionic_set;
- return sigsuspend64(&set.sigset64);
+ SigSetConverter set{bionic_set};
+ return sigsuspend64(set.ptr);
}
int sigsuspend64(const sigset64_t* set) {
@@ -271,9 +270,8 @@
}
int sigtimedwait(const sigset_t* bionic_set, siginfo_t* info, const timespec* timeout) {
- SigSetConverter set = {};
- set.sigset = *bionic_set;
- return sigtimedwait64(&set.sigset64, info, timeout);
+ SigSetConverter set{bionic_set};
+ return sigtimedwait64(set.ptr, info, timeout);
}
int sigtimedwait64(const sigset64_t* set, siginfo_t* info, const timespec* timeout) {
@@ -287,9 +285,8 @@
}
int sigwait(const sigset_t* bionic_set, int* sig) {
- SigSetConverter set = {};
- set.sigset = *bionic_set;
- return sigwait64(&set.sigset64, sig);
+ SigSetConverter set{bionic_set};
+ return sigwait64(set.ptr, sig);
}
int sigwait64(const sigset64_t* set, int* sig) {
diff --git a/libc/bionic/sigprocmask.cpp b/libc/bionic/sigprocmask.cpp
index 8781c9b..6d436a6 100644
--- a/libc/bionic/sigprocmask.cpp
+++ b/libc/bionic/sigprocmask.cpp
@@ -44,24 +44,13 @@
int sigprocmask(int how,
const sigset_t* bionic_new_set,
sigset_t* bionic_old_set) __attribute__((__noinline__)) {
- SigSetConverter new_set;
- sigset64_t* new_set_ptr = nullptr;
- if (bionic_new_set != nullptr) {
- sigemptyset64(&new_set.sigset64);
- new_set.sigset = *bionic_new_set;
- new_set_ptr = &new_set.sigset64;
+ SigSetConverter new_set{bionic_new_set};
+ SigSetConverter old_set{bionic_old_set};
+ int rc = sigprocmask64(how, new_set.ptr, old_set.ptr);
+ if (rc == 0 && bionic_old_set != nullptr) {
+ old_set.copy_out();
}
-
- SigSetConverter old_set;
- if (sigprocmask64(how, new_set_ptr, &old_set.sigset64) == -1) {
- return -1;
- }
-
- if (bionic_old_set != nullptr) {
- *bionic_old_set = old_set.sigset;
- }
-
- return 0;
+ return rc;
}
int sigprocmask64(int how,
diff --git a/libc/bionic/spawn.cpp b/libc/bionic/spawn.cpp
index 5d76f77..38f99ad 100644
--- a/libc/bionic/spawn.cpp
+++ b/libc/bionic/spawn.cpp
@@ -41,7 +41,6 @@
#include <android/fdsan.h>
#include "private/ScopedSignalBlocker.h"
-#include "private/SigSetConverter.h"
static int set_cloexec(int i) {
int v = fcntl(i, F_GETFD);
@@ -131,8 +130,10 @@
pid_t pgroup;
sched_param schedparam;
int schedpolicy;
- SigSetConverter sigmask;
- SigSetConverter sigdefault;
+ union {
+ sigset_t sigset;
+ sigset64_t sigset64;
+ } sigmask, sigdefault;
};
static void ApplyAttrs(short flags, const posix_spawnattr_t* attr) {
diff --git a/libc/bionic/sys_epoll.cpp b/libc/bionic/sys_epoll.cpp
index 22d0a98..cffa173 100644
--- a/libc/bionic/sys_epoll.cpp
+++ b/libc/bionic/sys_epoll.cpp
@@ -48,14 +48,8 @@
}
int epoll_pwait(int fd, epoll_event* events, int max_events, int timeout, const sigset_t* ss) {
- SigSetConverter set;
- sigset64_t* ss_ptr = nullptr;
- if (ss != nullptr) {
- set = {};
- set.sigset = *ss;
- ss_ptr = &set.sigset64;
- }
- return epoll_pwait64(fd, events, max_events, timeout, ss_ptr);
+ SigSetConverter set{ss};
+ return epoll_pwait64(fd, events, max_events, timeout, set.ptr);
}
int epoll_pwait64(int fd, epoll_event* events, int max_events, int timeout, const sigset64_t* ss) {
diff --git a/libc/bionic/sys_signalfd.cpp b/libc/bionic/sys_signalfd.cpp
index 53d1f25..1e62cf4 100644
--- a/libc/bionic/sys_signalfd.cpp
+++ b/libc/bionic/sys_signalfd.cpp
@@ -32,12 +32,12 @@
extern "C" int __signalfd4(int, const sigset64_t*, size_t, int);
-int signalfd(int fd, const sigset_t* mask, int flags) {
- SigSetConverter set = {};
- set.sigset = *mask;
- return signalfd64(fd, &set.sigset64, flags);
-}
-
int signalfd64(int fd, const sigset64_t* mask, int flags) {
return __signalfd4(fd, mask, sizeof(*mask), flags);
}
+
+int signalfd(int fd, const sigset_t* mask, int flags) {
+ // The underlying `__signalfd4` system call only takes `sigset64_t`.
+ SigSetConverter set{mask};
+ return signalfd64(fd, set.ptr, flags);
+}