Skip to content

[NFC][SYCL] Refactor merged_properties_t #16093

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

Merged
merged 1 commit into from
Nov 15, 2024
Merged
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
14 changes: 2 additions & 12 deletions sycl/include/sycl/ext/oneapi/properties/properties.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,19 +343,9 @@ template <typename... PropertyValueTs>
using properties_t =
properties<detail::properties_type_list<PropertyValueTs...>>;

// Helper for merging two property lists;
template <typename LHSPropertiesT, typename RHSPropertiesT>
struct merged_properties;
template <typename... LHSPropertiesTs, typename... RHSPropertiesTs>
struct merged_properties<properties_t<LHSPropertiesTs...>,
properties_t<RHSPropertiesTs...>> {
using type = properties<
typename MergeProperties<properties_type_list<LHSPropertiesTs...>,
properties_type_list<RHSPropertiesTs...>>::type>;
};
template <typename LHSPropertiesT, typename RHSPropertiesT>
using merged_properties_t =
typename merged_properties<LHSPropertiesT, RHSPropertiesT>::type;
using merged_properties_t = decltype(merge_properties(
std::declval<LHSPropertiesT>(), std::declval<RHSPropertiesT>()));

template <typename Properties, typename PropertyKey, typename Cond = void>
struct ValueOrDefault {
Expand Down
129 changes: 38 additions & 91 deletions sycl/include/sycl/ext/oneapi/properties/property_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ template <int N, typename... Ts>
using nth_type_t = typename nth_type<N, Ts...>::type;
#endif

template <typename T, typename PropList> struct PrependProperty {};
template <typename T, typename... Ts>
struct PrependProperty<T, properties_type_list<Ts...>> {
using type = properties_type_list<T, Ts...>;
};

//******************************************************************************
// Property identification
//******************************************************************************
Expand All @@ -63,91 +57,6 @@ struct AllPropertyValues<std::tuple<T, Ts...>>
AllPropertyValues<std::tuple<Ts...>>,
std::false_type> {};

//******************************************************************************
// Property type sorting
//******************************************************************************

// Splits a tuple into head and tail if ShouldSplit is true. If ShouldSplit is
// false the head will be void and the tail will be the full tuple.
template <typename T1, bool ShouldSplit> struct HeadSplit {};
template <typename T, typename... Ts>
struct HeadSplit<properties_type_list<T, Ts...>, true> {
using htype = T;
using ttype = properties_type_list<Ts...>;
};
template <typename... Ts> struct HeadSplit<properties_type_list<Ts...>, false> {
using htype = void;
using ttype = properties_type_list<Ts...>;
};

// Selects the one of two types that is not void. This assumes that at least one
// of the two template arguemnts is void.
template <typename LHS, typename RHS> struct SelectNonVoid {};
template <typename LHS> struct SelectNonVoid<LHS, void> {
using type = LHS;
};
template <typename RHS> struct SelectNonVoid<void, RHS> {
using type = RHS;
};

//******************************************************************************
// Property merging
//******************************************************************************

// Merges two sets of properties, failing if two properties are the same but
// with different values.
// NOTE: This assumes that the properties are in sorted order.
template <typename LHSPropertyT, typename RHSPropertyT> struct MergeProperties;

template <>
struct MergeProperties<properties_type_list<>, properties_type_list<>> {
using type = properties_type_list<>;
};

template <typename... LHSPropertyTs>
struct MergeProperties<properties_type_list<LHSPropertyTs...>,
properties_type_list<>> {
using type = properties_type_list<LHSPropertyTs...>;
};

template <typename... RHSPropertyTs>
struct MergeProperties<properties_type_list<>,
properties_type_list<RHSPropertyTs...>> {
using type = properties_type_list<RHSPropertyTs...>;
};

// Identical properties are allowed, but only one will carry over.
template <typename PropertyT, typename... LHSPropertyTs,
typename... RHSPropertyTs>
struct MergeProperties<properties_type_list<PropertyT, LHSPropertyTs...>,
properties_type_list<PropertyT, RHSPropertyTs...>> {
using merge_tails =
typename MergeProperties<properties_type_list<LHSPropertyTs...>,
properties_type_list<RHSPropertyTs...>>::type;
using type = typename PrependProperty<PropertyT, merge_tails>::type;
};

template <typename... LHSPropertyTs, typename... RHSPropertyTs>
struct MergeProperties<properties_type_list<LHSPropertyTs...>,
properties_type_list<RHSPropertyTs...>> {
using l_head = nth_type_t<0, LHSPropertyTs...>;
using r_head = nth_type_t<0, RHSPropertyTs...>;
static_assert(
PropertyID<l_head>::value != PropertyID<r_head>::value,
"Failed to merge property lists due to conflicting properties.");
static constexpr bool left_has_min =
PropertyID<l_head>::value < PropertyID<r_head>::value;
using l_split =
HeadSplit<properties_type_list<LHSPropertyTs...>, left_has_min>;
using r_split =
HeadSplit<properties_type_list<RHSPropertyTs...>, !left_has_min>;
using min = typename SelectNonVoid<typename l_split::htype,
typename r_split::htype>::type;
using merge_tails = typename MergeProperties<typename l_split::ttype,
typename r_split::ttype>::type;
using type = typename PrependProperty<min, merge_tails>::type;
};

//******************************************************************************
// Property value tooling
//******************************************************************************
Expand Down Expand Up @@ -349,6 +258,44 @@ constexpr auto filter_properties(
return filter_properties_impl<predicate, property_tys...>::apply(props);
}

template <typename... lhs_property_tys> struct merge_filter {
template <typename rhs_property_ty>
struct predicate
: std::bool_constant<!((std::is_same_v<typename lhs_property_tys::key_t,
typename rhs_property_ty::key_t> ||
...))> {};
};

template <typename... lhs_property_tys, typename... rhs_property_tys>
constexpr auto merge_properties(
const properties<properties_type_list<lhs_property_tys...>> &lhs,
const properties<properties_type_list<rhs_property_tys...>> &rhs) {
auto rhs_unique_props =
filter_properties<merge_filter<lhs_property_tys...>::template predicate>(
rhs);
if constexpr (std::is_same_v<std::decay_t<decltype(rhs)>,
std::decay_t<decltype(rhs_unique_props)>>) {
// None of RHS properties share keys with LHS, no conflicts possible.
return properties{
lhs.template get_property<typename lhs_property_tys::key_t>()...,
rhs.template get_property<typename rhs_property_tys::key_t>()...};
} else {
// Ensure no conflicts, then merge.
constexpr auto has_conflict = [](auto *lhs_prop) constexpr {
using lhs_property_ty = std::remove_pointer_t<decltype(lhs_prop)>;
return (((std::is_same_v<typename lhs_property_ty::key_t,
typename rhs_property_tys::key_t> &&
(!std::is_same_v<lhs_property_ty, rhs_property_tys> ||
!std::is_empty_v<lhs_property_ty>)) ||
...));
};
static_assert(
!((has_conflict(static_cast<lhs_property_tys *>(nullptr)) || ...)),
"Failed to merge property lists due to conflicting properties.");
return merge_properties(lhs, rhs_unique_props);
}
}

} // namespace detail
} // namespace ext::oneapi::experimental
} // namespace _V1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ void check_work_group_size() {
sycl::queue Q;

// expected-error-re@sycl/ext/oneapi/properties/property_utils.hpp:* {{static assertion failed due to requirement {{.+}}: Failed to merge property lists due to conflicting properties.}}
// expected-error-re@sycl/handler.hpp:* {{static assertion failed due to requirement {{.+}}: Template type is not a property list.}}
// expected-note-re@+1 {{in instantiation of function template specialization {{.+}}}}
Q.single_task(
sycl::ext::oneapi::experimental::properties{
Expand Down
Loading