This is an archive of the discontinued LLVM Phabricator instance.

[libc++][PSTL] Move the already implemented functions to the new dispatching scheme
ClosedPublic

Authored by philnik on May 10 2023, 9:22 AM.

Diff Detail

Event Timeline

philnik created this revision.May 10 2023, 9:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 9:22 AM
Herald added a subscriber: miyuki. · View Herald Transcript
philnik requested review of this revision.May 10 2023, 9:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 10 2023, 9:22 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 521017.May 10 2023, 9:35 AM

Some fixes

ldionne added inline comments.May 10 2023, 1:54 PM
libcxx/include/__algorithm/pstl_any_all_none_of.h
42–44

I think you can just use std::find_if(__policy, __first, __last, __pred) here.

libcxx/include/__algorithm/pstl_backend.h
45

Also all_of and none_of?

libcxx/include/__algorithm/pstl_backends/cpu_backend.h
19–22

You should add cancel_execution here.

26

We should include any_of.

libcxx/include/__algorithm/pstl_backends/cpu_backends/fill.h
39

Missing __cpu_backend_tag

libcxx/include/__algorithm/pstl_backends/cpu_backends/find_if.h
10

_LIBCPP___ALGORITHM_PSTL_BACKENDS_FIND_IF_H => _LIBCPP___ALGORITHM_PSTL_BACKENDS_CPU_BACKENDS_FIND_IF_H

libcxx/include/__algorithm/pstl_fill.h
13

You also need __algorithm/fill.h right?

libcxx/include/__algorithm/pstl_find.h
20

I don't think you need that.

philnik updated this revision to Diff 521107.May 10 2023, 2:39 PM
philnik marked 8 inline comments as done.

Address comments

philnik updated this revision to Diff 521327.May 11 2023, 8:43 AM

Fix some stuff

philnik updated this revision to Diff 521382.May 11 2023, 11:19 AM

Generate files

philnik updated this revision to Diff 521438.May 11 2023, 1:59 PM
  • Rebased
  • Try to fix CI
philnik updated this revision to Diff 521470.May 11 2023, 3:25 PM

Try to fix CI

philnik updated this revision to Diff 521486.May 11 2023, 3:55 PM

Order CMakeLists.txt

philnik updated this revision to Diff 521498.May 11 2023, 4:55 PM

Try to fix CI

ldionne accepted this revision.May 11 2023, 5:21 PM

Nice! LGTM once CI is green.

libcxx/include/__algorithm/pstl_backends/cpu_backend.h
20
libcxx/include/__algorithm/pstl_backends/cpu_backends/fill.h
39

We should have a test for the CPU backend that lists all the functions it expects to provide, and make sure that they can indeed be called. Otherwise, refactorings can produce stuff like:

template <class _ExecutionPolicy, class _ForwardIterator, class _Tp, class = enable_if_t<...> >

template <class _ExecutionPolicy, class _ForwardIterator, class _Tp, enable_if_t<..., int> = 0>

template <class _ExecutionPolicy, class _ForwardIterator, class _Tp, enable_if_t<...> >

which are never valid overloads. This is actually quite tricky and we've been bitten by this in the past. This can be done in a separate patch, but I think it's a requirement of this approach or else we risk introducing performance regressions.

This revision is now accepted and ready to land.May 11 2023, 5:21 PM
philnik updated this revision to Diff 521681.May 12 2023, 9:15 AM

Try to fix CI

This revision was landed with ongoing or failed builds.May 12 2023, 1:11 PM
This revision was automatically updated to reflect the committed changes.
libcxx/include/__pstl/internal/parallel_backend_serial.h