Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to compile with -D_GLIBCXX_PARALLEL #489

Closed
joy13975 opened this issue Mar 8, 2017 · 9 comments
Closed

Failed to compile with -D_GLIBCXX_PARALLEL #489

joy13975 opened this issue Mar 8, 2017 · 9 comments

Comments

@joy13975
Copy link

joy13975 commented Mar 8, 2017

Hello,

json.hpp builds OK normally (even with -fopenmp) but would not build with gcc flags "-D_GLIBCXX_PARALLEL". I suspect this is because of different type rules in STL when parallel flag is on:

g++ -fdiagnostics-color=always -MMD -std=c++11 -fmax-errors=1 -O3  -fopenmp -D_GLIBCXX_PARALLEL  -I./jutil/src/  -o .obj/./core/Kabsch.o -c core/Kabsch.cpp
In file included from /usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/parallel/algo.h:47:0,
                 from /usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/parallel/algorithm:37,
                 from /usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/algorithm:65,
                 from ./input/json.hpp:32,
                 from ./input/JSONParser.hpp:4,
                 from core/Kabsch.cpp:9:
/usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/parallel/par_loop.h: In instantiation of '_Op __gnu_parallel::__for_each_template_random_access_ed(_RAIter, _RAIter, _Op, _Fu&, _Red, _Result, _Result&, typename std::iterator_traits<_Iter>::difference_type) [with _RAIter = const char*; _Op = __gnu_parallel::_Nothing; _Fu = __gnu_parallel::__accumulate_selector<const char*>; _Red = __gnu_parallel::__accumulate_binop_reduct<nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parse(IteratorType, IteratorType, nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t) [with IteratorType = const char*; typename std::enable_if<std::is_base_of<std::random_access_iterator_tag, typename std::iterator_traits<_InputIterator>::iterator_category>::value, int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long long int; NumberUnsignedType = long long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t = std::function<bool(int, nlohmann::basic_json<>::parse_event_t, nlohmann::basic_json<>&)>]::<lambda(std::pair<bool, int>, const char&)> >; _Result = std::pair<bool, int>; typename std::iterator_traits<_Iter>::difference_type = long int]':
/usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/parallel/numeric:99:49:   required from '_Tp std::__parallel::__accumulate_switch(_RAIter, _RAIter, _Tp, _BinaryOper, std::random_access_iterator_tag, __gnu_parallel::_Parallelism) [with _RAIter = const char*; _Tp = std::pair<bool, int>; _BinaryOper = nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parse(IteratorType, IteratorType, nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t) [with IteratorType = const char*; typename std::enable_if<std::is_base_of<std::random_access_iterator_tag, typename std::iterator_traits<_InputIterator>::iterator_category>::value, int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long long int; NumberUnsignedType = long long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t = std::function<bool(int, nlohmann::basic_json<>::parse_event_t, nlohmann::basic_json<>&)>]::<lambda(std::pair<bool, int>, const char&)>]'
/usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/parallel/numeric:160:33:   required from '_Tp std::__parallel::accumulate(_IIter, _IIter, _Tp, _BinaryOper) [with _IIter = const char*; _Tp = std::pair<bool, int>; _BinaryOper = nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parse(IteratorType, IteratorType, nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t) [with IteratorType = const char*; typename std::enable_if<std::is_base_of<std::random_access_iterator_tag, typename std::iterator_traits<_InputIterator>::iterator_category>::value, int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long long int; NumberUnsignedType = long long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t = std::function<bool(int, nlohmann::basic_json<>::parse_event_t, nlohmann::basic_json<>&)>]::<lambda(std::pair<bool, int>, const char&)>]'
./input/json.hpp:6461:9:   required from 'static nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer> nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parse(IteratorType, IteratorType, nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t) [with IteratorType = const char*; typename std::enable_if<std::is_base_of<std::random_access_iterator_tag, typename std::iterator_traits<_InputIterator>::iterator_category>::value, int>::type <anonymous> = 0; ObjectType = std::map; ArrayType = std::vector; StringType = std::__cxx11::basic_string<char>; BooleanType = bool; NumberIntegerType = long long int; NumberUnsignedType = long long unsigned int; NumberFloatType = double; AllocatorType = std::allocator; JSONSerializer = nlohmann::adl_serializer; nlohmann::basic_json<ObjectType, ArrayType, StringType, BooleanType, NumberIntegerType, NumberUnsignedType, NumberFloatType, AllocatorType, JSONSerializer>::parser_callback_t = std::function<bool(int, nlohmann::basic_json<>::parse_event_t, nlohmann::basic_json<>&)>]'
./input/json.hpp:12816:42:   required from here
/usr/local/Cellar/gcc/6.1.0/include/c++/6.1.0/parallel/par_loop.h:102:17: error: no matching function for call to 'std::pair<bool, int>::pair(std::iterator_traits<const char*>::value_type)'
      __reduct = new _Result(__f(__o, __begin + __start));

This is on MacOSX 10.12.3 with Homebrew gcc 6.1.0.
The assertion at line 6461 was causing the problem. I found that if we follow http://stackoverflow.com/a/35008842/266378 more faithfully, e.g. by defining

template<class I>
static bool is_contiguous(I first, I last)
{ 
    bool test = true;
    auto const n = std::distance(first, last);
    for (int i = 0; i < n && test; ++i) {
        test &= *(std::next(first, i)) == *(std::next(std::addressof(*first), i));
    }        
    return test;        
}

and replacing the assertion by calling

assert(is_contiguous(first, last));

this error seems to go away. However I'm not an expert in STL so was hoping someone could verify this.

@nlohmann
Copy link
Owner

nlohmann commented Mar 8, 2017

I remember transforming that for loop into the accumulate function we currently use:

assert(std::accumulate(first, last, std::pair<bool, int>(true, 0),
                       [&first](std::pair<bool, int> res, decltype(*first) val)
{
    res.first &= (val == *(std::next(std::addressof(*first), res.second++)));
    return res;
}).first);

Unless I made a mistake and this does not do the same as the for loop, then the problem is GCC's GLIBCXX_PARALLEL, right?

@nlohmann
Copy link
Owner

nlohmann commented Mar 8, 2017

I could reproduce the problem with

gcc (GCC) 7.0.1 20170305 (experimental)
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

running macOS Sierra 10.12.3.

@nlohmann nlohmann added the state: help needed the issue needs help to proceed label Mar 8, 2017
@TurpentineDistillery
Copy link

TurpentineDistillery commented Mar 9, 2017

Perhaps you could simply check that the distance between the pointers to first and last is the same as the distance between the corresponding iterators?

@joy13975
Copy link
Author

joy13975 commented Mar 9, 2017

This bug track seems to explain it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61075

TLDR: the specification has an extra requirement for std::accumulate() because each thread needs and init value. GCC assumes an init value per thread by de-referencing the first iterator.

@nlohmann
Copy link
Owner

nlohmann commented Mar 9, 2017

Right, it's this:

Therefore the parallal accumulate has an additional requirement not present on the serial accumulate: is_convertible<iterator_traits::value_type, T>

@joy13975
Copy link
Author

joy13975 commented Mar 10, 2017

Since in a threaded loop the addresses get split up, it won't be easy to check that for thread_id > 0, the beginning address (assigned to this thread) is continuous wrt. to the previous chunk of addresses given to another thread.

Therefore I think the only way to check for address continuity is to use the single threaded loop as I have quoted from that original SO post.

@nlohmann nlohmann removed the state: help needed the issue needs help to proceed label Mar 11, 2017
@nlohmann
Copy link
Owner

Right, I think this is the solution then.

@nlohmann nlohmann self-assigned this Mar 11, 2017
@nlohmann nlohmann added this to the Release 3.0.0 milestone Mar 11, 2017
nlohmann added a commit that referenced this issue Mar 11, 2017
OpenMP added a requirement to std::accumulate which yielded a
compilation error. This commit replaced it by a raw for loop.
nlohmann added a commit that referenced this issue Mar 11, 2017
The is_contiguous lambda is only used in an assertion, so it can be
removed if NDEBUG is defined.
@nlohmann
Copy link
Owner

@joy13975 I added a fix for this with a801d76. I could compile it with GCC 5 and -fopenmp -D_GLIBCXX_PARALLEL. Could you please check if it works for you as well?

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 11, 2017
@joy13975
Copy link
Author

I can confirm that it compiles fine now with "-D_GLIBCXX_PARALLEL" on Homebrew GCC 6.1.0.

Thanks a lot.

@nlohmann nlohmann removed the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants