search

laristra / flecsi

A mirror of FleCSI's internal gitlab repository.
Other
67 stars 34 forks source link

Serialization for task parameters #629

Closed opensdh closed 5 years ago

opensdh commented 5 years ago

This allows "arbitrary" types as task parameters, falling back on memcpy (as was effectively already in use) for basic types. If the extra malloc/copy is problematic, we can if constexpr it on the new memcpyable_v trait—but note that field_handle<T> doesn't satisfy that for want of a default constructor. (You have to have some object to memcpy into; reinterpret_cast isn't actually allowed here, although surely it works.)

staleyLANL commented 5 years ago

I didn't check line-by-line, but, offhand, all of this looks very good. Am I understanding correctly that the changes allowed some Boost dependencies (in particular in serialize.hh) to be removed entirely?

opensdh commented 5 years ago

I didn't check line-by-line, but, offhand, all of this looks very good.

Thanks for looking.

Am I understanding correctly that the changes allowed some Boost dependencies (in particular in serialize.hh) to be removed entirely?

Yes, the dependency on boost::serialization is gone (as per the last commit message). That wasn't really the point—that serialization library just wasn't quite what I wanted to use, and @tuxfan (reasonably!) asked that we not maintain a wrapper for that one on top of our own implementation.

tuxfan commented 5 years ago

@opensdh: I merged this. However, I get an error when I try to compile with clang:

[ 26%] Building CXX object flecsi/execution/CMakeFiles/task.dir/test/task.cc.o In file included from /home/bergen/devel/tuxfan/flecsi/flecsi/execution/test/task.cc:15: In file included from /home/bergen/devel/tuxfan/flecsi/flecsi/../flecsi/utils/ftest.hh:22: In file included from /home/bergen/devel/tuxfan/flecsi/flecsi/../flecsi/control/control.hh:20: In file included from /home/bergen/devel/tuxfan/flecsi/flecsi/../flecsi/control/control_point_walker.hh:21: In file included from /home/bergen/devel/tuxfan/flecsi/flecsi/../flecsi/utils/tuple_walker.hh:18: /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:133:17: error: reference member '_M_head_impl' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object : _M_head_impl(std::forward<_UHead>(__h)) { } ^~~~~~~~~ /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:366:4: note: in instantiation of function template specialization 'std::_Head_base<0, const float &, false>::_Head_base' requested here : _Base(std::forward<_UHead>(__head)) { } ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/tuple:647:11: note: in instantiation of function template specialization 'std::_Tuple_impl<0, const float &>::_Tuple_impl' requested here : _Inherited(std::forward<_UElements>(__elements)...) { } ^ /home/bergen/devel/tuxfan/flecsi/flecsi/../flecsi/execution/legion/execution_policy.hh:73:28: note: in instantiation of function template specialization 'std::tuple<const float &>::tuple<double, true>' requested here return utils::serial_put(std::tuple<std::conditional_t< ^

opensdh commented 5 years ago

/home/bergen/devel/tuxfan/flecsi/flecsi/../flecsi/execution/legion/execution_policy.hh:73:28: note: in instantiation of function template specialization 'std::tuple<const float &>::tuple<double, true>' requested here return utils::serial_put(std::tuple<std::conditional_t<

Good catch (you and Clang, I guess): we shouldn't be attempting to construct a tuple of const float& from a double. The code actually notices that it can't bind the reference, but then proceeds to do so anyway because it forgets to replace const float& with just float. Should be fixed in #631, but please double-check.

tuxfan commented 5 years ago

@opensdh Thanks! That fixes the build issue.