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
Distributed define_spmd_block #2633
Distributed define_spmd_block #2633
Conversation
hpx/lcos/spmd_block.hpp
Outdated
|
||
int * dummy = nullptr; | ||
hpx::util::invoke( | ||
reinterpret_cast<const F&>(*dummy), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How should this work? invoke will certainly dereference a nullptr. This is UB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lambda overload removed in 4657f0e
hpx/lcos/spmd_block.hpp
Outdated
decltype( &helper_type::call ), &helper_type::call >::type; | ||
|
||
// Initialize f at compile-time in case when f is a lambda function | ||
hpx::lcos::detail::initialize_if_lambda(f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really work. It only might work if f
has no state. This limits the usefulness of spmd_block
tremenedously.The nullptr dereference and that you don't transfer the callable to the remote is just calling for trouble.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4657f0e
hpx/parallel/spmd_block.hpp
Outdated
|
||
// Asynchronous version | ||
template <typename ExPolicy, typename F, typename ... Args, | ||
typename = typename std::enable_if< | ||
hpx::parallel::v1::is_async_execution_policy<ExPolicy>::value>::type | ||
> | ||
std::vector<hpx::future<void>> | ||
inline std::vector<hpx::future<void>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
templated functions have inline
linkage by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix done in 2fbc1d6
f48b87e
to
062db7f
Compare
@sithhell Is the PR now good for U? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, expect for the extract_first_argument helper.
hpx/lcos/spmd_block.hpp
Outdated
{ | ||
using type = typename std::decay<Arg0>::type; | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be factored out and merged with the one defined in hpx/lcos/local/spmd_block.hpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
77ecb48
to
0b2d5ae
Compare
0b2d5ae
to
7d6afc1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, thanks!
This patch tries to implement a distributed version of
define_spmd_block()
.Here are the following changes:define_spmd_block()
and localspmd_block()
have been moved tohpx::lcos::local
hpx::parallel::v2::spmd_block
is now defined as a type aliashpx::parallel::v2::define_spmd_block()
is now wrapping a call tohpx::lcos::local::define_spmd_block()
define_spmd_block()
and globalspmd_block()
(based onbroadcast
andhpx::lcos::barrier
) added and located inhpx::lcos