Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

*.pyc
/build
/build-pr6877
.DS_Store
.gitignore
.ptp-sync-folder
Expand Down
2 changes: 2 additions & 0 deletions libs/core/algorithms/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ set(algorithms_headers
hpx/parallel/algorithms/detail/pivot.hpp
hpx/parallel/algorithms/detail/reduce.hpp
hpx/parallel/algorithms/detail/reduce_deterministic.hpp
hpx/parallel/algorithms/detail/remove.hpp
hpx/parallel/algorithms/detail/replace.hpp
hpx/parallel/algorithms/detail/rfa.hpp
hpx/parallel/algorithms/detail/rotate.hpp
Expand Down Expand Up @@ -239,6 +240,7 @@ if(HPX_WITH_DATAPAR)
hpx/parallel/datapar/loop.hpp
hpx/parallel/datapar/mismatch.hpp
hpx/parallel/datapar/reduce.hpp
hpx/parallel/datapar/remove.hpp
hpx/parallel/datapar/replace.hpp
hpx/parallel/datapar/search.hpp
hpx/parallel/datapar/transfer.hpp
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright (c) 2026 Bhoomish Gupta
//
// SPDX-License-Identifier: BSL-1.0
// Distributed under the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)

#pragma once

#include <hpx/config.hpp>
#include <hpx/modules/execution.hpp>
#include <hpx/modules/functional.hpp>
#include <hpx/modules/tag_invoke.hpp>
#include <hpx/modules/type_support.hpp>
#include <hpx/parallel/algorithms/detail/find.hpp>
#include <hpx/parallel/util/loop.hpp>

#include <iterator>
#include <type_traits>
#include <utility>

namespace hpx::parallel::detail {

///////////////////////////////////////////////////////////////////////////
HPX_CXX_CORE_EXPORT template <typename ExPolicy>
struct sequential_remove_if_t final
: hpx::functional::detail::tag_fallback<sequential_remove_if_t<ExPolicy>>
{
private:
template <typename Iter, typename Sent, typename Pred, typename Proj>
friend constexpr Iter tag_fallback_invoke(sequential_remove_if_t,
ExPolicy&&, Iter first, Sent last, Pred pred, Proj proj)
{
first = hpx::parallel::detail::sequential_find_if<ExPolicy>(
first, last, pred, proj);

if (first != last)
{
for (Iter i = first; ++i != last;)
if (!HPX_INVOKE(pred, HPX_INVOKE(proj, *i)))
{
*first++ = std::ranges::iter_move(i);
}
Comment on lines +38 to +42
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sequential_remove_if_t moves elements with *first++ = HPX_MOVE(*i), which bypasses std::ranges::iter_move and can break for proxy/reference-like iterators (e.g. vector<bool>::iterator) and generally diverges from the previous implementation in remove.hpp that used std::ranges::iter_move(i). Use std::ranges::iter_move(i) (and/or std::ranges::iter_move/iter_swap-style primitives) for the move source to preserve iterator semantics.

Copilot uses AI. Check for mistakes.
}
return first;
}
};

#if !defined(HPX_COMPUTE_DEVICE_CODE)
HPX_CXX_CORE_EXPORT template <typename ExPolicy>
inline constexpr sequential_remove_if_t<ExPolicy> sequential_remove_if =
sequential_remove_if_t<ExPolicy>{};
#else
HPX_CXX_CORE_EXPORT template <typename ExPolicy, typename... Args>
HPX_HOST_DEVICE HPX_FORCEINLINE auto sequential_remove_if(Args&&... args)
{
return sequential_remove_if_t<ExPolicy>{}(std::forward<Args>(args)...);
}
#endif

///////////////////////////////////////////////////////////////////////////
HPX_CXX_CORE_EXPORT template <typename ExPolicy>
struct sequential_remove_t final
: hpx::functional::detail::tag_fallback<sequential_remove_t<ExPolicy>>
{
private:
template <typename Iter, typename Sent, typename T, typename Proj>
friend constexpr Iter tag_fallback_invoke(sequential_remove_t,
ExPolicy&& policy, Iter first, Sent last, T const& value, Proj proj)
{
return sequential_remove_if<ExPolicy>(
HPX_FORWARD(ExPolicy, policy), first, last,
[&value](auto const& a) { return value == a; }, proj);
}
};

#if !defined(HPX_COMPUTE_DEVICE_CODE)
HPX_CXX_CORE_EXPORT template <typename ExPolicy>
inline constexpr sequential_remove_t<ExPolicy> sequential_remove =
sequential_remove_t<ExPolicy>{};
#else
HPX_CXX_CORE_EXPORT template <typename ExPolicy, typename... Args>
HPX_HOST_DEVICE HPX_FORCEINLINE auto sequential_remove(Args&&... args)
{
return sequential_remove_t<ExPolicy>{}(std::forward<Args>(args)...);
}
#endif

} // namespace hpx::parallel::detail
68 changes: 32 additions & 36 deletions libs/core/algorithms/include/hpx/parallel/algorithms/remove.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ namespace hpx {
#else // DOXYGEN

#include <hpx/config.hpp>
#include <hpx/algorithms/traits/projected.hpp>
#include <hpx/execution/traits/vector_pack_conditionals.hpp>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please alsways use only the generted module includes (except inside a module itself):

Suggested change
#include <hpx/execution/traits/vector_pack_conditionals.hpp>
#include <hpx/modules/execution.hpp>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I will keep to generated module includes in non-module code paths from now on.

#include <hpx/modules/concepts.hpp>
#include <hpx/modules/execution.hpp>
#include <hpx/modules/executors.hpp>
Expand All @@ -223,6 +225,7 @@ namespace hpx {
#include <hpx/parallel/algorithms/detail/dispatch.hpp>
#include <hpx/parallel/algorithms/detail/distance.hpp>
#include <hpx/parallel/algorithms/detail/find.hpp>
#include <hpx/parallel/algorithms/detail/remove.hpp>
#include <hpx/parallel/util/detail/algorithm_result.hpp>
#include <hpx/parallel/util/detail/sender_util.hpp>
#include <hpx/parallel/util/loop.hpp>
Expand All @@ -243,25 +246,6 @@ namespace hpx::parallel {
namespace detail {

/// \cond NOINTERNAL
HPX_CXX_CORE_EXPORT template <typename Iter, typename Sent,
typename Pred, typename Proj>
constexpr Iter sequential_remove_if(
Iter first, Sent last, Pred pred, Proj proj)
{
first = hpx::parallel::detail::sequential_find_if<
hpx::execution::sequenced_policy>(first, last, pred, proj);

if (first != last)
{
for (Iter i = first; ++i != last;)
if (!HPX_INVOKE(pred, HPX_INVOKE(proj, *i)))
{
*first++ = std::ranges::iter_move(i);
}
}
return first;
}

HPX_CXX_CORE_EXPORT template <typename FwdIter>
struct remove_if : public algorithm<remove_if<FwdIter>, FwdIter>
{
Expand All @@ -272,10 +256,11 @@ namespace hpx::parallel {

template <typename ExPolicy, typename Iter, typename Sent,
typename Pred, typename Proj>
static constexpr Iter sequential(
ExPolicy, Iter first, Sent last, Pred&& pred, Proj&& proj)
static constexpr Iter sequential(ExPolicy&& policy, Iter first,
Sent last, Pred&& pred, Proj&& proj)
{
return sequential_remove_if(first, last,
return sequential_remove_if<ExPolicy>(
HPX_FORWARD(ExPolicy, policy), first, last,
HPX_FORWARD(Pred, pred), HPX_FORWARD(Proj, proj));
}

Expand All @@ -284,6 +269,17 @@ namespace hpx::parallel {
static decltype(auto) parallel(ExPolicy&& policy, Iter first,
Sent last, Pred&& pred, Proj&& proj)
{
using inner_policy_type = std::decay_t<ExPolicy>;
constexpr bool vectorpack_policy =
hpx::is_vectorpack_execution_policy_v<inner_policy_type>;

if constexpr (vectorpack_policy)
{
return sequential_remove_if<ExPolicy>(
HPX_FORWARD(ExPolicy, policy), first, last,
HPX_FORWARD(Pred, pred), HPX_FORWARD(Proj, proj));
}

using zip_iterator = hpx::util::zip_iterator<Iter, bool*>;
using algorithm_result =
util::detail::algorithm_result<ExPolicy, Iter>;
Expand All @@ -299,7 +295,6 @@ namespace hpx::parallel {
if (count == 0)
return algorithm_result::get(HPX_MOVE(first));
}

std::shared_ptr<bool[]> flags(new bool[count]);

using hpx::get;
Expand All @@ -311,12 +306,10 @@ namespace hpx::parallel {
zip_iterator part_begin,
std::size_t part_size) -> void {
// MSVC complains if pred or proj is captured by ref below
util::loop_n<std::decay_t<ExPolicy>>(part_begin, part_size,
util::loop_n<inner_policy_type>(part_begin, part_size,
[pred, proj](zip_iterator it) mutable {
bool f = hpx::invoke(
get<1>(*it) = hpx::invoke(
pred, hpx::invoke(proj, get<0>(*it)));

get<1>(*it) = f;
});
};

Expand All @@ -325,11 +318,10 @@ namespace hpx::parallel {
auto dest = first;
auto part_size = count;

using execution_policy_type = std::decay_t<ExPolicy>;
if (dest == get<0>(part_begin.get_iterator_tuple()))
{
// Self-assignment must be detected.
util::loop_n<execution_policy_type>(
util::loop_n<hpx::execution::sequenced_policy>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this lose information? Why is this change needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does not lose algorithm/data information.
That loop has a loop-carried dependency on dest plus self-assignment checks, so running it with vectorpack/unsequenced execution is not safe without a prefix-sum style compaction step. Using sequenced_policy here preserves correctness and ordering; the SIMD work remains in the predicate-evaluation phase f1. So the change drops vectorization in f2.

part_begin, part_size, [&dest](zip_iterator it) {
if (!get<1>(*it))
{
Expand All @@ -344,7 +336,7 @@ namespace hpx::parallel {
else
{
// Self-assignment can't be performed.
util::loop_n<execution_policy_type>(
util::loop_n<hpx::execution::sequenced_policy>(
part_begin, part_size, [&dest](zip_iterator it) {
if (!get<1>(*it))
*dest++ = std::ranges::iter_move(
Expand Down Expand Up @@ -396,9 +388,15 @@ namespace hpx {
requires (
hpx::is_execution_policy_v<ExPolicy> &&
hpx::traits::is_iterator_v<FwdIter> &&
hpx::is_invocable_v<Pred,
typename std::iterator_traits<FwdIter>::value_type
>
(
hpx::parallel::traits::is_indirect_callable_v<ExPolicy,
Pred, hpx::parallel::traits::projected<hpx::identity,
FwdIter>
Comment thread
hkaiser marked this conversation as resolved.
> ||
hpx::is_invocable_v<Pred,
typename std::iterator_traits<FwdIter>::value_type
>
)
)
// clang-format on
friend decltype(auto) tag_fallback_invoke(hpx::remove_if_t,
Expand Down Expand Up @@ -446,10 +444,8 @@ namespace hpx {
friend decltype(auto) tag_fallback_invoke(hpx::remove_t,
ExPolicy&& policy, FwdIter first, FwdIter last, T const& value)
{
using Type = typename std::iterator_traits<FwdIter>::value_type;

return hpx::remove_if(HPX_FORWARD(ExPolicy, policy), first, last,
[value](Type const& a) -> bool { return value == a; });
[value](auto const& a) { return value == a; });
}
} remove{};
} // namespace hpx
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,14 @@ namespace hpx::ranges {
hpx::traits::is_iterator_v<FwdIter> &&
std::sentinel_for<Sent, FwdIter> &&
hpx::parallel::traits::is_projected_v<Proj, FwdIter> &&
hpx::parallel::traits::is_indirect_callable_v<ExPolicy,
Pred, hpx::parallel::traits::projected<Proj, FwdIter>
>
(
hpx::parallel::traits::is_indirect_callable_v<ExPolicy,
Pred, hpx::parallel::traits::projected<Proj, FwdIter>
> ||
hpx::is_invocable_v<Pred,
typename std::iterator_traits<FwdIter>::value_type
>
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain the rationale of this change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale is to keep the range execution-policy overload aligned with current dispatch behavior in remove_if.
If we required only the policy-aware projected callable check, scalar-only predicates would be rejected at the CPO boundary, even though the implementation can still run them through the scalar predicate-evaluation path when vectorized invocation is not available.
Adding the scalar invocable check preserves backward compatibility and keeps iterator/range policy-overload behavior consistent.
This changes only whether predicate evaluation can be vectorized for a given call; it does not change remove_if semantics or output correctness.

If you prefer stricter behavior for par_simd and related policies, I can tighten this to require only the policy-aware projected callable check and fail early otherwise.

Comment on lines +558 to +565
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added || hpx::is_invocable_v<Pred, value_type> constraint can accept predicates that are not callable with the actual argument type used by the algorithm (a projected iterator reference). For example, a predicate taking value_type&& is invocable with value_type but not with value_type& from *first, leading to harder-to-diagnose errors inside the implementation. Consider removing this fallback or changing it to check invocability with the projected reference type (e.g. std::iter_reference_t<FwdIter> after projection).

Copilot uses AI. Check for mistakes.
)
// clang-format on
friend typename parallel::util::detail::algorithm_result<ExPolicy,
Expand All @@ -582,9 +587,16 @@ namespace hpx::ranges {
hpx::is_execution_policy_v<ExPolicy> &&
std::ranges::range<Rng> &&
hpx::parallel::traits::is_projected_range_v<Proj, Rng> &&
hpx::parallel::traits::is_indirect_callable_v<ExPolicy,
Pred, hpx::parallel::traits::projected_range<Proj, Rng>
>
(
hpx::parallel::traits::is_indirect_callable_v<ExPolicy,
Pred, hpx::parallel::traits::projected_range<Proj, Rng>
> ||
hpx::is_invocable_v<Pred,
typename std::iterator_traits<
std::ranges::iterator_t<Rng>
>::value_type
>
)
)
// clang-format on
friend parallel::util::detail::algorithm_result_t<ExPolicy,
Expand Down
1 change: 1 addition & 0 deletions libs/core/algorithms/include/hpx/parallel/datapar.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <hpx/parallel/datapar/loop.hpp>
#include <hpx/parallel/datapar/mismatch.hpp>
#include <hpx/parallel/datapar/reduce.hpp>
#include <hpx/parallel/datapar/remove.hpp>
#include <hpx/parallel/datapar/replace.hpp>
#include <hpx/parallel/datapar/search.hpp>
#include <hpx/parallel/datapar/transfer.hpp>
Expand Down
Loading
Loading