boostorg / compute

A C++ GPU Computing Library for OpenCL
http://boostorg.github.io/compute/
Boost Software License 1.0
1.55k stars 333 forks source link

compute/algorithm/random_shuffle.hpp breaks build with C++1z because std::random_shuffle missing #760

Closed jeffhammond closed 6 years ago

jeffhammond commented 6 years ago

LLVM's libc++ has implemented the deprecation and removal of std::random_shuffle( RandomIt first, RandomIt last );, which is used unconditionally in compute/algorithm/random_shuffle.hpp. https://github.com/llvm-mirror/libcxx/blob/master/include/algorithm#L3043 has details.

There is a straightforward fix involving the use of std::shuffle, which was introduced in C++11. If this change is made, should it be guarded by BOOST_COMPUTE_USE_CPP11 or should it use something associated with C++14/17 so as to avoid breaking backwards-compatibility (of the pseudo-randomization) for C++11 (and possibly C++14) users?

Also, do you have a preference on which PRNG to use? I used std::default_random_engine in my local modification, but that is the least reproducible, given that it is implementation-defined.

I'll work on the fix once I know in which direction to go.

Details

/usr/local/Cellar/llvm/5.0.1/bin/clang++ -std=c++1z -pthread -g -O3 -mtune=native -ffast-math -DPRKVERSION="2.16" stencil-vector-pstl.cc  -DUSE_BOOST -I/usr/local/Cellar/boost/1.65.1/include -o stencil-vector-stl
In file included from stencil-vector-pstl.cc:63:
In file included from ./prk_util.h:185:
In file included from /usr/local/include/boost/compute.hpp:14:
In file included from /usr/local/include/boost/compute/algorithm.hpp:64:
/usr/local/include/boost/compute/algorithm/random_shuffle.hpp:49:5: error: no member named 'random_shuffle' in namespace 'std';
      did you mean simply 'random_shuffle'?
    std::random_shuffle(random_indices.begin(), random_indices.end());
    ^~~~~
/usr/local/include/boost/compute/algorithm/random_shuffle.hpp:35:13: note: 'random_shuffle' declared here
inline void random_shuffle(Iterator first,
            ^
1 error generated.
make: *** [stencil-vector-stl] Error 1

It works fine with -std=c++14.

jszuppe commented 6 years ago

There is a straightforward fix involving the use of std::shuffle, which was introduced in C++11. If this change is made, should it be guarded by BOOST_COMPUTE_USE_CPP11 or should it use something associated with C++14/17 so as to avoid breaking backwards-compatibility (of the pseudo-randomization) for C++11 (and possibly C++14) users?

I think your change is good. Thanks for it!

Also, do you have a preference on which PRNG to use? I used std::default_random_engine in my local modification, but that is the least reproducible, given that it is implementation-defined.

IMHO, it's a good choice.

jszuppe commented 6 years ago

Fixed in #761