LGTM

In spite of my minor comments it LGTM.

LGTM

Commit 6069a6a5
by Mikhail Dvorskiy, 10/15/2021 03:36 PM

A hot performance fix for parallel reduce pattern of OpenMP backend. The recursion should be limited by a grain size - __default_chunk_size.

clang format

Christopher, one more comment - we discussed and agreed that a better name for macro _PSTL_PAR_BACKEND_OMP is

Commit 0b2e0e80 (open)
by Dvorskiy, Mikhail, 09/14/2020 02:20 PM
committed 09/14/2020 02:21 PM
parent 5cac85c9

In D61340#1558095, @ldionne wrote:

...

I don't mind. All I care is that this stuff stays out of the main include directory.

Louis,
thanks for the alternative proposal.

1. The test was improved.
2. Minor changes in PSTL code. It needs for possibility of coverage that case in "negative test scenario".

+ fix filename in a file header

Added a negative test for exclusive_scan algorithm. It checks non-participation the algo declare in overload resolution.

Add uglification for "merge_task" and Rebased to the HEAD

In D62719#1525450, @rodgert wrote:

Actually, we should probably get a test of some sort here before accepting the change. Yes, it's fairly obviously wrong and the fix is fairly obviously correct, but it's a regression and we should provide tests for regressions, lest certain standard library maintainers ask uncomfortable questions.

In D62719#1525424, @ldionne wrote:

Do you have commit access?

+ "internal::" for "brick_move"

Rebased of the HEAD of the master

What do you think about my last proposal regarding

But let's avoid any name/path that suggests these headers are part of the public interface of the library, because they are not

If the problem with passing test inplace_merge.pass.cpp" is actual, I'll investigate and fix it by proposed new patch over the top your changes.

I don't think the pstl should include headers that are useless to all but one specific shipping vehicle of the PSTL (the standalone version)

I have an issue regarding the dummy stdlib headers.
Proposed location is valid only for the test. (/test/support/stdlib)

In that case, may we "reserve" MINOR version to differ PSTL code version?

// The version is XYYZ, where X is major, YY is minor, and Z is patch (i.e. X.YY.Z)

Continuing the discussion - the code snippet of the test engine - pay your attention to using namespace __pstl::execution

we should have a way to customize the namespace in which pstl injects the algorithms

Eventually, I tend to that keeping "cout" code is redundant , just "return 0" is enough.
We will try to re-config our test system to accept just returning zero;

I don't see any "bookmarks" (technical, and others) on scheme versioning on our side.
So, I've no objections against the proposed approach.

Guys,
I understand that given pstl code is becoming a part of C++ standard library and , of course should be std::execution::unsequenced_policy and so on..

I have no objections in general..
But let add the line

I think we all agreed that this was worth implementing

Of course. I will configure "ninja" for running the upstream test.

..no matching function for call to 'equal' return std::equal(first1, last1, first2, last2, __p);

+ compilation fixes (lost "__").

The lost "__" has been returned.

Rebased on top of latest master (+ uglification)

Yes, agree.

Yes,... I doubted that is relevant to LLVM release.. and so as the some other macros form pstl_config.h
But as far as pstl_config.h is, PSTL_VERSION may be useful, at least to differ major changes, I guess...

1. Usage of std::equal with "4 iterators" (due to it is available in C++ standard library since C++14)
2. Clang formatted

At least, it "extends" signature of functions of a back-end that helps (now) to support several back-ends in the same time (by overloading).

a significant gain in readability

Hi Louis,

In D59704#1439794, @ldionne wrote:

How would you somehow handle allocation through a custom policy?

pstl_config.h is not required

Yes, a captured __comp instance is redundant in the all cases here due to it is passed via argument's list explicitly.