This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Make sure we use the libdispatch backend on Apple platforms
ClosedPublic

Authored by ldionne on Jul 18 2023, 3:22 PM.

Details

Summary

The Apple.cmake cache wasn't set up properly, so we wouldn't enable
the libdispatch backend by default on Apple platforms. This patch
fixes the issue and adds a test.

We also need to make various drive-by fixes:

  • Drop the usage of std::vector in libdispatch.h to avoid changing the transitive includes only on Apple platforms.
  • Fix includes
  • Use __construct at since construct_at is unavailable in C++17
  • Get rid of the (unused) __get_memory_resource function since that adds a back-deployment requirement and we don't use it right now.
  • Fix bugs in the chunking logic around boundary conditions.

Diff Detail

Event Timeline

ldionne created this revision.Jul 18 2023, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2023, 3:22 PM
ldionne requested review of this revision.Jul 18 2023, 3:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2023, 3:22 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik accepted this revision.Jul 18 2023, 5:43 PM

LGTM % comment.

libcxx/include/__algorithm/pstl_backends/cpu_backends/libdispatch.h
31

pushing and popping the min/max macros is missing.

This revision is now accepted and ready to land.Jul 18 2023, 5:43 PM
ldionne updated this revision to Diff 541837.Jul 18 2023, 9:31 PM
ldionne marked an inline comment as done.

Push/pop macros.

ldionne updated this revision to Diff 541977.Jul 19 2023, 5:54 AM

Fix formatting.

ldionne updated this revision to Diff 542211.Jul 19 2023, 2:59 PM

Fix modules

ldionne updated this revision to Diff 542308.Jul 19 2023, 10:37 PM
ldionne edited the summary of this revision. (Show Details)

Fix remaining issues.

ldionne updated this revision to Diff 542514.Jul 20 2023, 7:31 AM
ldionne edited the summary of this revision. (Show Details)

Fix more issues caught by backdeployment CI

philnik added inline comments.Jul 20 2023, 8:38 AM
libcxx/include/__algorithm/pstl_backends/cpu_backends/libdispatch.h
178–179

I don't understand this change. Is this just an optimization, or is something here assuming a size > 0?

ldionne marked an inline comment as done.Jul 20 2023, 10:22 AM
ldionne added inline comments.
libcxx/include/__algorithm/pstl_backends/cpu_backends/libdispatch.h
178–179

Otherwise we crash below:

std::__construct_at(__values.get() + __chunk, __reduction(__first + __index + 2, __first + __index + __this_chunk_size, __combiner(__transform(__first + __index), __transform(__first + __index + 1))));
This revision was automatically updated to reflect the committed changes.
ldionne marked an inline comment as done.