Skip to content

[SYCL] Don't use boost/mp11 for properties filtering #16076

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 14, 2024
Merged

[SYCL] Don't use boost/mp11 for properties filtering #16076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 9 additions & 17 deletions sycl/include/sycl/ext/oneapi/experimental/annotated_ptr/annotated_ptr.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
#include <sycl/ext/oneapi/properties/property_value.hpp>
#include <sycl/pointers.hpp>

#include <sycl/detail/boost/mp11/algorithm.hpp>

#include <cstddef>
#include <string_view>
#include <tuple>
Expand Down Expand Up @@ -56,16 +54,10 @@ template <typename... Ts>
using contains_alignment =
detail::ContainsProperty<alignment_key, std::tuple<Ts...>>;

// properties filter
template <typename property_list, template <class...> typename filter>
using PropertiesFilter =
sycl::detail::boost::mp11::mp_copy_if<property_list, filter>;

// filter properties that are applied on annotations
template <typename... Props>
using annotation_filter =
properties<PropertiesFilter<detail::properties_type_list<Props...>,
propagateToPtrAnnotation>>;
template <typename PropertyListTy>
using annotation_filter = decltype(filter_properties<propagateToPtrAnnotation>(
std::declval<PropertyListTy>()));
} // namespace detail

template <typename I, typename P> struct annotationHelper {};
Expand Down Expand Up @@ -110,8 +102,8 @@ class annotated_ref<T, detail::properties_t<Props...>> {
// implicit conversion with annotaion
operator T() const {
#ifdef __SYCL_DEVICE_ONLY__
return annotationHelper<T, detail::annotation_filter<Props...>>::load(
m_Ptr);
return annotationHelper<
T, detail::annotation_filter<property_list_t>>::load(m_Ptr);
#else
return *m_Ptr;
#endif
Expand All @@ -121,8 +113,8 @@ class annotated_ref<T, detail::properties_t<Props...>> {
template <class O, typename = std::enable_if_t<!detail::is_ann_ref_v<O>>>
T operator=(O &&Obj) const {
#ifdef __SYCL_DEVICE_ONLY__
return annotationHelper<T, detail::annotation_filter<Props...>>::store(
m_Ptr, Obj);
return annotationHelper<
T, detail::annotation_filter<property_list_t>>::store(m_Ptr, Obj);
#else
return *m_Ptr = std::forward<O>(Obj);
#endif
Expand Down Expand Up @@ -387,8 +379,8 @@ __SYCL_TYPE(annotated_ptr) annotated_ptr<T, detail::properties_t<Props...>> {

T *get() const noexcept {
#ifdef __SYCL_DEVICE_ONLY__
return annotationHelper<T, detail::annotation_filter<Props...>>::annotate(
m_Ptr);
return annotationHelper<
T, detail::annotation_filter<property_list_t>>::annotate(m_Ptr);
#else
return m_Ptr;
#endif
Expand Down
44 changes: 44 additions & 0 deletions sycl/include/sycl/ext/oneapi/properties/property_utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,50 @@ struct ConditionalPropertyMetaInfo
: std::conditional_t<Condition, PropertyMetaInfo<PropT>,
IgnoredPropertyMetaInfo> {};

template <template <typename> typename predicate, typename... property_tys>
struct filter_properties_impl {
static constexpr auto idx_info = []() constexpr {
constexpr int N = sizeof...(property_tys);
std::array<int, N> indexes{};
int num_matched = 0;
int idx = 0;
(((predicate<property_tys>::value ? indexes[num_matched++] = idx++ : idx++),
Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of mental association, I would add another set of parenthesis:

Suggested change
(((predicate<property_tys>::value ? indexes[num_matched++] = idx++ : idx++),
(((predicate<property_tys>::value ? (indexes[num_matched++] = idx++) : idx++),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make a note of that and will implement in a separate PR with final touches after major refactorings are all finished.

...));

return std::pair{indexes, num_matched};
}();

// Helper to convert constexpr indices values to an std::index_sequence type.
// Values -> type is the key here.
template <int... Idx>
static constexpr auto idx_seq(std::integer_sequence<int, Idx...>) {
return std::integer_sequence<int, idx_info.first[Idx]...>{};
}

using selected_idx_seq =
decltype(idx_seq(std::make_integer_sequence<int, idx_info.second>{}));

// Using prop_list_ty so that we don't need to explicitly spell out
// `properties` template parameters' implementation-details.
template <typename prop_list_ty, int... Idxs>
static constexpr auto apply_impl(const prop_list_ty &props,
std::integer_sequence<int, Idxs...>) {
return properties{props.template get_property<
typename nth_type_t<Idxs, property_tys...>::key_t>()...};
}

template <typename prop_list_ty>
static constexpr auto apply(const prop_list_ty &props) {
return apply_impl(props, selected_idx_seq{});
}
};

template <template <typename> typename predicate, typename... property_tys>
constexpr auto filter_properties(
const properties<properties_type_list<property_tys...>> &props) {
return filter_properties_impl<predicate, property_tys...>::apply(props);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The naming scheme used in this patch does not seem to follow the surrounding code.

Copy link
Contributor Author

@aelovikov-intel aelovikov-intel Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Discussed offline - I'm following the style in properties.hpp because I'll be moving this new code into a single source and removing the surrounding code in near future. Also, we're already very inconsistent in different places with properties.

} // namespace detail
} // namespace ext::oneapi::experimental
} // namespace _V1
Expand Down
Loading