Implement remove and remove_if algorithms for datapar execution

[BhoomishGupta]

Partially Fixes #2333

Proposed Changes

-Added datapar/remove.hpp implementing SIMD-accelerated sequential_remove_if and sequential_remove via tag_invoke overloads for vectorpack execution policies
-Added detail/remove.hpp with sequential_remove_if_t and sequential_remove_t tag-fallback CPOs used by both scalar and datapar paths
-Added datapar unit tests remove_datapar.cpp and remove_if_datapar.cpp covering simd, par_simd, simd(task), and par_simd(task) policies with both random-access and forward iterators; registered both in CMakeLists.txt

Any background context you want to provide?

Checklist

Not all points below apply to all pull requests.

• I have added a new feature and have added tests to go along with it.
• I have fixed a bug and have added a regression test.
• I have added a test using random numbers; I have made sure it uses a seed, and that random numbers generated are valid inputs for the tests.

[StellarBot]

Can one of the admins verify this patch?

[hkaiser]

@BhoomishGupta From what I can see, your new implementations do not take advantage of data parallelisation but simply do a per-element processing once the first matching element is found. Do you see a way of taking advantage of data parallelism for the remainder of the sequence as well?

[BhoomishGupta]

@BhoomishGupta From what I can see, your new implementations do not take advantage of data parallelisation but simply do a per-element processing once the first matching element is found. Do you see a way of taking advantage of data parallelism for the remainder of the sequence as well?

Thanks for the feedback! I will look into it.

Just as a quick reminder, my university exams are still going on right now, so my availability is quite limited. I will dive into this and work on an improved approach right after my exams finish on March 14th.

Thanks for your patience!

[hkaiser]

These mapping CPOs are already defined here: https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/executors/include/hpx/executors/datapar/execution_policy_mappings.hpp

[BhoomishGupta]

I moved these mapping CPOs from https://github.com/STEllAR-GROUP/hpx/blob/master/libs/core/executors/include/hpx/executors/datapar/execution_policy_mappings.hpp to libs/core/executors/include/hpx/executors/execution_policy_mappings.hpp

These CPOs were originally defined in the datapar header, but I moved them to the common execution policy mappings header so they are available even when datapar/SIMD is not enabled. This is needed for code paths that reference the mappings independently of datapar support.
The datapar header now just includes the common header (guarded by HPX_HAVE_DATAPAR) for compatibility.

[hkaiser]

Why would this be needed if datapar is not enabled?

[BhoomishGupta]

Yeah, I reviewed this. And you are right.
I originally fell back to the non_simd policy (and consequently had to move the execution_policy_mappings.hpp header to make it globally visible) because the f1 loop was failing to compile under the SIMD backends.

However, doing this forced the parallel chunks to evaluate the predicate sequentially, which indeed defeats the purpose of the SIMD implementation.

The root cause of the original compilation failure was that the flags array was declared as bool[]. In the std::simd backends, boolean vector arrays instantiate as proxy mask objects rather than standard packed primitives, which broke the vector_pack traits when it tried to iterate and assign. Also, the f1 lambda was explicitly taking (zip_iterator it) instead of (auto it), which prevented the datapar loop_n from passing in the vectorized chunks.

To resolve this properly, I am thinking of doing this:

1. I will change the flags array allocation from bool to std::uint8_t so it reliably packs.
2. I will change the f1 lambda parameter to auto it and revert its loop policy back to the native std::decay_t so that the heavy predicate evaluation actually vectorizes.
3. I will completely revert the execution_policy_mappings.hpp move, as to_non_simd will no longer be leaking unconditionally into the generic algorithm.

[hkaiser]

Wouldn't invoking the loop with a non-simd policy defeat the whole purpose of introducing SIMD functionalities to remove_if?

[BhoomishGupta]

You're absolutely right. Invoking the loop with inner_policy_type (non-SIMD) was a workaround because of a compilation error with the SIMD backend. I've since identified that the root cause was the bool[] flags array and the explicit zip_iterator parameter in the lambda. I'm reverting this to use the original execution policy to ensure the predicate evaluation is properly vectorized, as detailed in my other comment

[hkaiser]

For std::simd (std::experimental::simd) you could utilize https://en.cppreference.com/w/cpp/experimental/simd/find_first_set.html to possibly reduce the amount of iterations needed.

[hkaiser]

For the other datapar backends (EVE, VC, arm simd) you may have to emulate this or simply always return 0 and simd_width respectively)

[BhoomishGupta]

Thanks for the heads up! It looks like HPX already has a generic implementation for this across all backends via hpx::parallel::traits::find_first_of(msk) in libs/core/execution/include/hpx/execution/traits/vector_pack_find.hpp

I'll update the loop to use it.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric	Results
Duplication	0

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[hkaiser]

@BhoomishGupta Would you be able to follow up on the compilation errors reported by the CIs?

[BhoomishGupta]

@BhoomishGupta Would you be able to follow up on the compilation errors reported by the CIs?

Working on it!

[hkaiser]

@BhoomishGupta Please pay attention to the CIs. ARM is failing to compile. Also, the remove test is now failing to compile even on CIs that do not enable SIMD functionalities.

[BhoomishGupta]

@BhoomishGupta Please pay attention to the CIs. ARM is failing to compile. Also, the remove test is now failing to compile even on CIs that do not enable SIMD functionalities.

My mistake! I missed an include. Just pushed a fix. Watching the CIs now

[hkaiser]

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>

[BhoomishGupta]

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

[hkaiser]

Please refactor the code similarily to what we have done for he other algorithms. The code related to datapar should be kept fully separate from the non-datapar code paths. If datapar is disabled, e.g., the is_vectorpack_execution_policy you use here are not available.

[BhoomishGupta]

Agreed. I will refactor the code such that datapar-specific assumptions are out of generic algorithm paths so non-datapar builds remain valid.

[hkaiser]

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

[BhoomishGupta]

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.

[hkaiser]

Can you please explain the rationale of this change?

[BhoomishGupta]

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.

[hkaiser]

@BhoomishGupta ping?

[BhoomishGupta]

@BhoomishGupta ping?

Sorry for the delay. Resolving the comments asap

[Copilot]

Pull request overview

Implements remove/remove_if support for datapar (SIMD) execution policies by adding a datapar specialization and refactoring the sequential remove implementation into reusable CPOs, along with new unit tests.

Changes:

• Added SIMD-accelerated sequential_remove_if/sequential_remove via tag_invoke for vectorpack policies.
• Introduced sequential_remove_if_t/sequential_remove_t fallback CPOs shared by scalar and datapar paths.
• Added datapar unit tests for remove and remove_if, and adjusted supporting SIMD/EVE load/store and datapar loop internals.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test_package/test_package.cpp	Minor include order tweak.
libs/core/execution/include/hpx/execution/traits/vector_pack_conditionals.hpp	Moves HPX_HAVE_DATAPAR guard to only wrap backend includes.
libs/core/execution/include/hpx/execution/traits/detail/simd/vector_pack_simd.hpp	Exposes element_aligned for experimental SIMD backend.
libs/core/execution/include/hpx/execution/traits/detail/simd/vector_pack_load_store.hpp	Adjusts SIMD unaligned store to use copy_to(..., element_aligned) for class vector packs.
libs/core/execution/include/hpx/execution/traits/detail/eve/vector_pack_load_store.hpp	Uses eve::load/eve::store for class vector packs; scalar fallback for non-class V.
libs/core/algorithms/tests/unit/datapar_algorithms/remove_if_datapar.cpp	Adds datapar tests for remove_if across simd/par_simd and task variants.
libs/core/algorithms/tests/unit/datapar_algorithms/remove_datapar.cpp	Adds datapar tests for remove across simd/par_simd and task variants.
libs/core/algorithms/tests/unit/datapar_algorithms/CMakeLists.txt	Registers new datapar unit tests.
libs/core/algorithms/include/hpx/parallel/datapar/remove.hpp	New datapar implementation + tag_invoke hooks for sequential remove/remove_if CPOs.
libs/core/algorithms/include/hpx/parallel/datapar/loop.hpp	Tweaks loop bounds/alignment conditions (primarily readability/robustness).
libs/core/algorithms/include/hpx/parallel/datapar.hpp	Exposes the new datapar remove header.
libs/core/algorithms/include/hpx/parallel/container_algorithms/remove.hpp	Broadens constraints for ranges remove/remove_if with execution policies.
libs/core/algorithms/include/hpx/parallel/algorithms/remove.hpp	Switches to new sequential remove CPOs and adds vectorpack-policy fast-path handling.
libs/core/algorithms/include/hpx/parallel/algorithms/detail/remove.hpp	Adds new fallback CPOs for sequential remove/remove_if.
libs/core/algorithms/CMakeLists.txt	Registers new header(s) in build system.
.gitignore	Adds a PR-specific build directory ignore entry.

[Copilot]

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]

In the vectorpack-policy fast path, sequential_remove_if<inner_policy_type>(HPX_FORWARD(ExPolicy, policy), ...) instantiates the CPO with a decayed policy type, but forwards an ExPolicy that is often an lvalue reference (e.g. calling remove_if(par_simd, ...) deduces ExPolicy as par_simd_policy const&). This makes the call require binding an lvalue/const policy to an inner_policy_type&& parameter in tag_fallback_invoke, which is ill-formed. Use the same policy type for the CPO template parameter as the forwarded argument (e.g. sequential_remove_if<ExPolicy>(HPX_FORWARD(ExPolicy, policy), ...), relying on is_vectorpack_execution_policy_v to decay internally), or otherwise ensure the policy argument type matches the CPO’s ExPolicy&& parameter.

Suggested change

	return sequential_remove_if<inner_policy_type>(
	return sequential_remove_if<ExPolicy>(

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 6 comments.

[Copilot]

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]

The SIMD loop uses last - first but Sent is only constrained as std::sentinel_for<Sent, Iter> in the call chain (e.g. hpx::ranges::remove_if passes an arbitrary sentinel). This will fail to compile for non-sized sentinels even when the iterator is random access. Consider constraining the datapar overload/path to std::sized_sentinel_for<Sent, Iter> (and falling back to to_non_simd otherwise), or rewrite the loop to avoid sentinel subtraction when not available.

Suggested change

	while (
	last - first >= static_cast<std::ptrdiff_t>(size)) //Safety
	{
	V tmp(hpx::parallel::traits::vector_pack_load<V,
	value_type>::aligned(first));
	auto msk = HPX_INVOKE(pred, HPX_INVOKE(proj, tmp));
	if (hpx::parallel::traits::none_of(msk))
	{
	//no elements match
	if (dest != first)
	{
	if (util::detail::is_data_aligned(dest))
	{
	hpx::parallel::traits::vector_pack_store<V,
	value_type>::aligned(tmp, dest);
	}
	else
	{
	hpx::parallel::traits::vector_pack_store<V,
	value_type>::unaligned(tmp, dest);
	}
	}
	std::advance(dest, size);
	}
	else if (!hpx::parallel::traits::all_of(msk))
	{
	//mixed
	for (std::size_t i = 0; i < size; ++i)
	{
	auto scalar_val =
	value_type(hpx::parallel::traits::get(tmp, i));
	bool match =
	HPX_INVOKE(pred, HPX_INVOKE(proj, scalar_val));
	if (!match)
	{
	*dest++ = scalar_val;
	}
	}
	}
	//all elements match
	std::advance(first, size);
	if constexpr (std::sized_sentinel_for<Sent, Iter>)
	{
	while (last - first >=
	static_cast<std::ptrdiff_t>(size)) // Safety
	{
	V tmp(hpx::parallel::traits::vector_pack_load<V,
	value_type>::aligned(first));
	auto msk = HPX_INVOKE(pred, HPX_INVOKE(proj, tmp));
	if (hpx::parallel::traits::none_of(msk))
	{
	//no elements match
	if (dest != first)
	{
	if (util::detail::is_data_aligned(dest))
	{
	hpx::parallel::traits::vector_pack_store<V,
	value_type>::aligned(tmp, dest);
	}
	else
	{
	hpx::parallel::traits::vector_pack_store<V,
	value_type>::unaligned(tmp, dest);
	}
	}
	std::advance(dest, size);
	}
	else if (!hpx::parallel::traits::all_of(msk))
	{
	//mixed
	for (std::size_t i = 0; i < size; ++i)
	{
	auto scalar_val =
	value_type(hpx::parallel::traits::get(tmp, i));
	bool match =
	HPX_INVOKE(pred, HPX_INVOKE(proj, scalar_val));
	if (!match)
	{
	*dest++ = scalar_val;
	}
	}
	}
	//all elements match
	std::advance(first, size);
	}

[Copilot]

std::is_class_v is used in vector_pack_store::unaligned but this header doesn't include <type_traits>, which will fail to compile on toolchains where it isn't transitively included. Add an explicit #include <type_traits> here (or in a common header included unconditionally).

[Copilot]

The vectorized loop condition last - first > (size + 1) is much stricter than the previous first < last - size + 1 check (which effectively allows vectorization when last - first >= size). This will skip valid SIMD iterations (e.g. when exactly size or size+1 elements remain), degrading performance. Consider changing the condition to last - first >= static_cast<std::ptrdiff_t>(size) (or equivalent) while still avoiding underflow.

Suggested change

	while (last - first > static_cast<std::ptrdiff_t>(size + 1))
	while (last - first >= static_cast<std::ptrdiff_t>(size))

[Copilot]

Same off-by-one issue as above: while (last - first > (size + 1)) in the predicate loop prevents vectorization when last - first == size (or size+1). This path is on hot code for many datapar algorithms (find, all_of/any_of/none_of, etc.), so it's worth keeping the condition equivalent to last - first >= size.

Suggested change

	while (last - first > static_cast<std::ptrdiff_t>(size + 1))
	while (last - first >= static_cast<std::ptrdiff_t>(size))

[Copilot]

In the count-based loop, while (len > size + 1) also skips vector iterations when len == size or len == size+1, even though callv only needs len >= size. To preserve vectorization while still avoiding unsigned underflow from the old len - size + 1 expression, prefer a condition like while (len >= size) (and adjust the decrement accordingly).

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.

[Copilot]

datapar_remove_if::call relies on last - first (and later std::advance(first, size)), but last is a generic Sent. This will fail to compile (or be ill-formed) for non-sized sentinels (e.g. some range/view sentinels). Consider constraining the datapar path to std::sized_sentinel_for<Sent, Iter>/std::same_as<Sent, Iter> or switching to a distance/count-based loop (like other datapar algorithms) and falling back to to_non_simd when the sentinel is not sized.

[Copilot]

The scalar prologue/epilogue in the datapar path calls the predicate on scalar elements (*first), while the vectorized loop calls it on V. This makes remove_if(simd, ...) require predicates/projections to be callable on both scalar and vector-pack types, which is inconsistent with other datapar algorithms that use V1 packs in scalar steps. To improve compatibility and avoid extra predicate evaluations, consider driving the prologue/epilogue (and the mixed-lane handling) off the already computed mask (or by evaluating on V1) instead of invoking the predicate on scalar values.

[Copilot]

Similarly in the indexed loop, while (len > size + 1) skips a valid final SIMD iteration when len is exactly size (or size+1). Using len >= size would keep the original vectorization coverage while still avoiding out-of-range iterator arithmetic.

[hkaiser]

@BhoomishGupta Could you please have an eye on the CIs?

[BhoomishGupta]

@BhoomishGupta Could you please have an eye on the CIs?

Yeah, I have been trying to resolve the Linux Crosscompile CI for a long time but it keeps failing.
Now, I have setup a local docker environment with the image taken from CI, will push a commit when Linux Crosscompile builds locally.

[arpittkhandelwal]

// Copyright (c) 2026 Bhoomish Gupta