[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