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

Jerry Crunchtime via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sat Feb 2 01:12:07 PST 2019


jerryct added inline comments.


================
Comment at: include/pstl/internal/algorithm_impl.h:181
 
 template <class _ExecutionPolicy, class _RandomAccessIterator, class _Size, class _Function, class _IsVector>
 _RandomAccessIterator
----------------
rodgert wrote:
> 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).
I addressed you comment in this new change: https://reviews.llvm.org/D57638


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