[libcxx-commits] [PATCH] D141779: [libc++][PSTL] Copy the headers into libc++

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added subscribers: var-const, rarutyun, Mordante.
ldionne added a comment.

Thanks for the stack of patches! This is a really nice start. Before we commit to going into a specific direction, I think it is useful to lay out how we envision things being at the end, and then roughly how we're going to get there.

Here's a starting point. At the end, I think what we want to have is (this doesn't necessarily reflect *how* we're going to get there):

1. No more `<__pstl_algorithm>` & other glue headers -- instead the PSTL versions would be defined in suitably named `<__algorithm/pstl_any_of.h>` headers and included directly inside `<algorithm>`.
2. No more `<pstl/internal/algorithm_impl.h>` and `<pstl/internal/algorithm_fwd.h>` headers & friends -- those contain the implementation of algorithms and they would be in `<__algorithm/pstl_any_of.h>` & friends instead.
3. No more `<pstl/internal/glue_algorithm_defs.h>` and `<pstl/internal/glue_algorithm_impl.h>` headers & friends -- those contain the standard facing algorithm names, which would now be directly in `<__algorithm/pstl_any_of.h>` & friends.
4. The headers implementing backends are kept and we try not to make significant changes to them, if possible. This should allow keeping the backends in sync with `pstl/` as needed. We might want to reorganize them under something like `<__pstl_backends/{omp.h,tbb.h,serial.h,etc.}>` or something similar.
5. The configuration headers `__pstl_config_site.in`, `pstl_config.h` can be simplified to only contain the macros we actually need (pruning platforms and compilers we don't support), and then folded into `__config_site` and `__config` as appropriate.
6. The current `pstl/` has several layers of forward declarations and boilerplate, and I think we can simplify that since we don't need the flexibility of e.g. `std::any_of(ExecutionPolicy, ...)` being forward-declared anymore. So as you iterate to achieve the above steps (in particular 1-2-3), it would be reasonable to fold some of the layers of complexity. I see you've done that already in e.g. D143161 <https://reviews.llvm.org/D143161>.
7. We should have our own tests for these algorithms and they should follow the usual libc++ testing conventions. I think we should base them on the existing pstl tests but we can add them as we iterate on steps (1-2-3).

In terms of how to get there, I think your current approach makes sense, however I would start with a clean slate. So before this patch, I would create a patch that basically removes the current integration (including the `pstl` test symlink) so that we start the new integration from a clean slate.

@philnik I think this properly describes what you had planned on doing (and which I agree is the right thing to do). Additional thoughts/comments?

Also CCing a few folks for awareness since they might have opinions and it would be good to get a common agreement on the direction before we start chipping away at the algorithms. @rarutyun @Mordante @var-const


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141779