[PATCH] D103165: Threading: use independent llvm::thread implementation on Apple platforms to increase stack size
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 6 12:43:20 PDT 2021
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.
This LGTM, with some whitespace nits inline.
In D103165#2790932 <https://reviews.llvm.org/D103165#2790932>, @t.p.northover wrote:
>> Also this way llvm::thread users that don't need full-sized stacks can continue getting the Darwin default of smaller pages.
>
> I'm not sure I've implemented this, since I've just made 8MB the Darwin default here. The case I care about is LTOBackend via `ThreadPool`. Would you prefer me to push an optional `StackSize` argument into `ThreadPool` too and make LTOBackend specify it?
>
> I kind of think this has come up often enough (it's at least the second time I've had to fix it somewhere), and 8MB is small enough for anything running Clang that it's better to make sure it doesn't happen again.
8MB by default is probably fine; I don't feel strongly either way.
================
Comment at: llvm/include/llvm/Support/thread.h:107
+
+ bool joinable() const noexcept {
+ return Thread != native_handle_type();
----------------
Seems like you might as well run `git-clang-format` to pick up these changes...
================
Comment at: llvm/include/llvm/Support/thread.h:261
+#endif // LLVM_SUPPORT_THREAD_H
\ No newline at end of file
----------------
Looks like it's missing a newline here at the end of the file.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103165/new/
https://reviews.llvm.org/D103165
More information about the cfe-commits
mailing list