[libcxx-commits] [PATCH] D141780: [libc++][PSTL] Integrate the headers and add a CI job

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/cmake/caches/With-pstl.cmake:1-2
+set(LIBCXX_TEST_PARAMS "std=c++17" CACHE STRING "")
+set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
+set(LIBCXX_ENABLE_PARALLEL_ALGORITHMS ON CACHE BOOL "")
----------------
These should be removed once we move to `-fexperimental-library`. Can you add a comment?

Actually, this whole CI job is going to go away once we move to `-fexperimental-library` for the PSTL.


================
Comment at: libcxx/include/CMakeLists.txt:869
   ostream
+  pstl/internal/algorithm_fwd.h
+  pstl/internal/algorithm_impl.h
----------------
Can we rename all of those to `__pstl/FOOOO` immediately after this patch? I'd be fine with doing it before as well, both ways work.


================
Comment at: libcxx/include/CMakeLists.txt:954
 configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY)
+configure_file("__pstl_config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__pstl_config_site" @ONLY)
 configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY)
----------------
Not needed anymore.


================
Comment at: libcxx/include/pstl/internal/pstl_config.h:13
 
 #include <__pstl_config_site>
 
----------------
I would remove `__pstl_config_site` entirely and define `_PSTL_PAR_BACKEND_SERIAL` temporarily, like you do right now.


================
Comment at: libcxx/utils/libcxx/test/features.py:212
   '_LIBCPP_ENABLE_DEBUG_MODE': 'libcpp-has-debug-mode',
+  '_LIBCPP_HAS_PARALLEL_ALGORITHMS': 'with-pstl',
 }
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141780