[libcxx-commits] [PATCH] D150284: [libc++][PSTL] Add a simple std::thread backend

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri May 19 08:24:41 PDT 2023


ldionne accepted this revision as: ldionne.
ldionne added a comment.

As I've said before, the goal of this backend is for us to figure out the customization points and create the plumbing for adding new CPU back-ends, and to weed out any issues we see while bringing up this "toy" backend. This is *not* the way the `std::thread` backend will look in the end, however it would be a waste of time to spend time optimizing it at this point since we're not certain what the CPU backend API will need to look like in the end. We suspect it'll look pretty much like the original PSTL customization points, but we might discover the need to do some things differently as we look into other backends (e.g. GCD). If that happens, any work put in making the `std::thread` backend good right now would potentially be wasted.

So given the spirit of this patch, I'm fine with it -- basically my bar here is:

1. I'm happy with the pluming it adds
2. The implementation looks correct and it passes tests

@EricWF Please let us know if you still see an issue with this plan.



================
Comment at: libcxx/include/__algorithm/pstl_backends/cpu_backends/thread.h:29
+
+_LIBCPP_FUNC_VIS unsigned __hardware_concurrency();
+_LIBCPP_FUNC_VIS void* __make_thread_pool(unsigned __expected_thread_count);
----------------
Can you add a comment here explaining that this implementation is experimental and will be replaced by a real implementation when the library is ready for production? Just to make it clear it was never meant to be used prime time. This should be clear from the need to use `-fexperimental-library`, but a comment here clarifies even more, I guess.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150284/new/

https://reviews.llvm.org/D150284



More information about the libcxx-commits mailing list