Skip to content

Failed to compile with -D_GLIBCXX_PARALLEL #489

Closed
Closed
Failed to compile with -D_GLIBCXX_PARALLEL#489
Assignees
nlohmann
Labels
confirmedkind: bug
Milestone
Release 3.0.0

Description

@joy13975
joy13975
opened on 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.

Activity

  1. nlohmann commented on Mar 8, 2017

    @nlohmann
    nlohmann
    on Mar 8, 2017
    Owner

    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?

  2. nlohmann commented on Mar 8, 2017

    @nlohmann
    nlohmann
    on Mar 8, 2017
    Owner

    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.

  3. nlohmann
    added
    state: help neededthe issue needs help to proceed
    on Mar 8, 2017

  4. TurpentineDistillery commented on Mar 9, 2017

    @TurpentineDistillery
    TurpentineDistillery
    on 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?

  5. joy13975 commented on Mar 9, 2017

    @joy13975
    joy13975
    on Mar 9, 2017
    Author

    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.

  6. nlohmann commented on Mar 9, 2017

    @nlohmann
    nlohmann
    on Mar 9, 2017
    Owner

    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>

  7. joy13975 commented on Mar 10, 2017

    @joy13975
    joy13975
    on Mar 10, 2017
    Author

    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.

  8. nlohmann
    removed
    state: help neededthe issue needs help to proceed
    on Mar 11, 2017

  9. nlohmann commented on Mar 11, 2017

    @nlohmann
    nlohmann
    on Mar 11, 2017
    Owner

    Right, I think this is the solution then.

  10. nlohmann
    self-assigned this
    on Mar 11, 2017
  11. nlohmann
    added this to the Release 3.0.0 milestone on Mar 11, 2017
  12. nlohmann
    added
    kind: enhancement/improvement
    confirmed
    kind: bug
    and removed
    kind: enhancement/improvement
    on Mar 11, 2017
  13. nlohmann
    added 2 commits that reference this issue on Mar 11, 2017

    🚑 fix for #489

    a801d76

    🔨 hid the test lambda #489

    9e232ec

  14. nlohmann commented on Mar 11, 2017

    @nlohmann
    nlohmann
    on Mar 11, 2017
    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?

  15. nlohmann
    added
    solution: proposed fixa fix for the issue has been proposed and waits for confirmation
    on Mar 11, 2017

  16. joy13975 commented on Mar 13, 2017

    @joy13975
    joy13975
    on Mar 13, 2017
    Author

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

    Thanks a lot.

  17. nlohmann
    removed
    solution: proposed fixa fix for the issue has been proposed and waits for confirmation
    on Mar 13, 2017
  18. nlohmann
    closed this as completedon Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

confirmedkind: bug

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions