[PATCH] D56139: [pstl] Fix compile errors when PARALLEL_POLICIES is disabled

Thomas Rodgers via Phabricator reviews at reviews.llvm.org
Tue Jan 22 10:18:52 PST 2019


rodgert added inline comments.


================
Comment at: include/pstl/internal/algorithm_impl.h:181
 
 template <class _ExecutionPolicy, class _RandomAccessIterator, class _Size, class _Function, class _IsVector>
 _RandomAccessIterator
----------------
ldionne wrote:
> rodgert wrote:
> > Shouldn't this definition be protected by #ifdef __PSTL_USE_PAR_POLICIES?
> > 
> > Sure, it will fail to compile because it calls something that is __PSTL_USE_PAR_POLICIES but what was it ldionne said about being consistent recently?
> I don't understand your comment. What is inconsistent about `pattern_walk1_n` not being guarded by `#ifdef __PSTL_USE_PAR_POLICIES`? Sorry, just trying to understand.
> 
> FWIW, I think that removing code based on `__PSTL_USE_PAR_POLICIES` is a misfeature and we should find a more gracious way of handling the lack of parallel policies, for example by having a backend that does everything sequentially if that makes sense. I accepted this revision because it fixed a compilation error under a configuration that currently seems to be supported.
Sorry, I applied the inline comment to the wrong variant of pattern_walk1_n, it's the next one down, where /*is_parallel=*/std::true_type

I also agree that removing code based on __PSTL_USE_PAR_POLICIES is a misfeature. I want to propose some alternatives based on work I've started on an OpenMP based backend and discussions that came out of SG1 in San Diego about likely future directions with this part of the standard library, but I need to finish this merge and get a patch into libstdc++ before I can return to that work.

In doing the reconciliation with the libstdc++ version of this code with what was submitted upstream (which unfortunately differs far more than what I thought we had agreed for the initial commit), I came across these inconsistencies and am just reporting them (they are fixed in my local branch, and will be part of that submission when it's ready).


================
Comment at: include/pstl/internal/algorithm_impl.h:2506-2515
 template <class _ExecutionPolicy, class _OutputIterator, class _Size, class _Generator, class _IsVector>
 _OutputIterator
 pattern_generate_n(_ExecutionPolicy&& __exec, _OutputIterator __first, _Size __count, _Generator __g,
                    /*is_parallel=*/std::true_type, _IsVector __is_vector)
 {
     static_assert(internal::is_random_access_iterator<_OutputIterator>::value,
                   "Pattern-brick error. Should be a random access iterator.");
----------------
This should also be protected by #ifdef __PSTL_USE_PAR_POLICIES


Repository:
  rPSTL pstl

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

https://reviews.llvm.org/D56139





More information about the libcxx-commits mailing list