Skip to content

Remove key_t #12246

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

Closed
wants to merge 6 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -152,19 +152,16 @@ struct device_has_key {

template <size_t... Dims>
struct property_value<work_group_size_key, std::integral_constant<size_t, Dims>...> {
using key_t = work_group_size_key;
constexpr size_t operator[](int dim) const;
};

template <size_t... Dims>
struct property_value<work_group_size_hint_key, std::integral_constant<size_t, Dims>...> {
using key_t = work_group_size_hint_key;
constexpr size_t operator[](int dim) const;
};

template <sycl::aspect... Aspects>
struct property_value<device_has_key, std::integral_constant<sycl::aspect, Aspects>...> {
using key_t = device_has_key;
static constexpr std::array<sycl::aspect, sizeof...(Aspects)> value;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ The intention is to provide a robust mechanism with which to pass compile-time-c
property:: A property is represented by a key and value. Properties can be used to provide extra values to classes or functions.

property value:: An object of the property value class. A property value has zero or more property parameters.
For runtime properties the value type is the same as the key type.
For compile time properties the value type is given by the `value_t` type alias of the key type.
The value type is given by the `value_t` type alias of the key type.

property key:: A class representing the property key. It is used to query properties.

Expand Down Expand Up @@ -138,27 +137,32 @@ value to determine which of the extension's APIs the implementation supports.

Properties have a value and key type,
and by convention, these classes are declared in the root of the
`sycl::ext::oneapi::experimental` namespace. For a runtime property the key and value types are the same and the name of the property value
class has no suffix. A runtime property value typically has a constructor
`sycl::ext::oneapi::experimental` namespace.
The value type is a template specialization of `property_value`.
The property key class contains a `value_t` alias which is templated on the property parameters.

A runtime property value typically has a constructor
which takes the value(s) of the properties and member function(s) which return those values.

```c++
namespace sycl::ext::oneapi::experimental {

// The runtime property key
struct foo_key {
using value_t = property_value<foo_key>;
};
// This is a runtime property value with one integer parameter.
// The name of the property value class is the the name of the property without any suffix.
struct foo {
foo(int);
using foo = property_value<foo_key>;
template <> struct property_value<foo_key> {
constexpr property_value(int v) : value(v) {}
int value;
};
// A runtime property key is an alias to the value type.
using foo_key = foo;

} // namespace experimental::oneapi::ext::sycl
```

For compile-time constant parameters the value type is a template specialization of `property_value`.
The property key class contains a `value_t` alias which is templated on the property parameters. The `property_value` class holds the
For compile-time constant parameters the `property_value` class holds the
values of the compile-time parameters as template arguments. The parameters to a compile-
time-constant property can be either types or non-type values.
The implementation provides a variable with the property value type. The variable has the name of the property without a suffix.
Expand Down Expand Up @@ -228,10 +232,10 @@ template<typename V, typename O, typename=void> struct is_property_value_of;
template<typename V> struct is_property_value<V, std::enable_if_t<(sizeof(V)>0)>> : is_property_key<V> {};
template<typename V, typename O> struct is_property_value_of<V, O, std::enable_if_t<(sizeof(V)>0)>> : is_property_key_of<V,O> {};
// Specialization for compile-time-constant properties
template<typename V> struct is_property_value<V, std::void_t<typename V::key_t>> :
is_property_key<typename V::key_t> {};
template<typename V, typename O> struct is_property_value_of<V, O, std::void_t<typename V::key_t>> :
is_property_key_of<typename V::key_t, O> {};
template<typename V> struct is_property_value<V, std::void_t<key_from_value<V>>> : //key_from_value is exposition only
is_property_key<key_from_value<V>> {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is is_property_key actual useful? This could instead just check that it is of type property_value<T...>. And if we remove is_property_key properties don't need to set that trait. And I'm only aware of useful usage of is_property_key_of. Why would a user care whether something is a property key rather than only whether it is a valid property for a certain usage?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the constraint for properties::has_property and properties::get_property requires us to know whether a type is a property key. How do we do that without the is_property_key trait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right the trait is useful for that and in that case we might as well expose it in case a user has some use for it. And we can still remove the need for the each property to set the trait in the implementation. We can instead e.g. check PropertyToKind.

template<typename V, typename O> struct is_property_value_of<V, O, std::void_t<key_from_value<V>>> :
is_property_key_of<key_from_value<V>, O> {};

} // namespace experimental::oneapi::ext::sycl
```
Expand All @@ -250,9 +254,6 @@ namespace sycl::ext::oneapi::experimental {

template<typename Property, typename First, typename...Others>
struct property_value {
// Alias of the property key type
using key_t = Property;

// Each property with multi-parameter property_value must define template
// specializations for accessing the parameters.

Expand Down Expand Up @@ -285,11 +286,6 @@ using value_t = First;
| Available only when there is exactly one parameter. When the parameter's value is a type, `value_t`
is that type. When the parameter's value is a non-type, `value_t` is an implementation-defined type
with a member constant `value` equal to the value.
a|
```c++
using key_t = property;
```
| The property key type.
|===
--

Expand Down
3 changes: 0 additions & 3 deletions sycl/include/sycl/ext/intel/esimd/memory_properties.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,18 @@ namespace ext::oneapi::experimental {
template <__ESIMD_NS::cache_hint Hint>
struct property_value<__ESIMD_NS::cache_hint_L1_key,
std::integral_constant<__ESIMD_NS::cache_hint, Hint>> {
using key_t = __ESIMD_NS::cache_hint_L1_key;
static constexpr __ESIMD_NS::cache_level level = __ESIMD_NS::cache_level::L1;
static constexpr __ESIMD_NS::cache_hint hint = Hint;
};
template <__ESIMD_NS::cache_hint Hint>
struct property_value<__ESIMD_NS::cache_hint_L2_key,
std::integral_constant<__ESIMD_NS::cache_hint, Hint>> {
using key_t = __ESIMD_NS::cache_hint_L2_key;
static constexpr __ESIMD_NS::cache_level level = __ESIMD_NS::cache_level::L2;
static constexpr __ESIMD_NS::cache_hint hint = Hint;
};
template <__ESIMD_NS::cache_hint Hint>
struct property_value<__ESIMD_NS::cache_hint_L3_key,
std::integral_constant<__ESIMD_NS::cache_hint, Hint>> {
using key_t = __ESIMD_NS::cache_hint_L3_key;
static constexpr __ESIMD_NS::cache_level level = __ESIMD_NS::cache_level::L3;
static constexpr __ESIMD_NS::cache_hint hint = Hint;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,18 @@ inline constexpr cache_config_enum large_slm =
inline constexpr cache_config_enum large_data =
cache_config_enum::large_data;

struct cache_config {
cache_config(cache_config_enum v) : value(v) {}
cache_config_enum value;
struct cache_config_key {
using value_t = oneapi::experimental::property_value<cache_config_key>;
};

using cache_config_key = cache_config;
using cache_config = cache_config_key::value_t;
} // namespace ext::intel::experimental
namespace ext::oneapi::experimental {
template <> struct property_value<intel::experimental::cache_config_key> {
property_value(intel::experimental::cache_config_enum v) : value(v) {}
intel::experimental::cache_config_enum value;
};
} // namespace ext::oneapi::experimental
namespace ext::intel::experimental {

inline bool operator==(const cache_config &lhs,
const cache_config &rhs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ struct ValidAllocPropertyList<T, detail::properties_t<Prop, Props...>>
is_valid_property<T *, Prop>::value,
"Found invalid compile-time property in the property list.");
// check if a runtime property is valid for malloc
static_assert(!detail::IsRuntimeProperty<Prop>::value ||
IsRuntimePropertyValid<Prop>::value,
static_assert(!detail::IsRuntimePropertyValue<Prop>::value ||
IsRuntimePropertyValid<key_from_value<Prop>>::value,
"Found invalid runtime property in the property list.");
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ struct property_value<work_group_size_key, std::integral_constant<size_t, Dim0>,
static_assert(detail::AllNonZero<Dim0, Dims...>::value,
"work_group_size property must only contain non-zero values.");

using key_t = work_group_size_key;

constexpr size_t operator[](int Dim) const {
return std::array<size_t, sizeof...(Dims) + 1>{Dim0, Dims...}[Dim];
}
Expand All @@ -85,8 +83,6 @@ struct property_value<work_group_size_hint_key,
detail::AllNonZero<Dim0, Dims...>::value,
"work_group_size_hint property must only contain non-zero values.");

using key_t = work_group_size_hint_key;

constexpr size_t operator[](int Dim) const {
return std::array<size_t, sizeof...(Dims) + 1>{Dim0, Dims...}[Dim];
}
Expand All @@ -98,15 +94,13 @@ struct property_value<sub_group_size_key,
static_assert(Size != 0,
"sub_group_size_key property must contain a non-zero value.");

using key_t = sub_group_size_key;
using value_t = std::integral_constant<uint32_t, Size>;
static constexpr uint32_t value = Size;
};

template <aspect... Aspects>
struct property_value<device_has_key,
std::integral_constant<aspect, Aspects>...> {
using key_t = device_has_key;
static constexpr std::array<aspect, sizeof...(Aspects)> value{Aspects...};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ struct property_value<
std::integral_constant<int, Target>,
std::integral_constant<intel::experimental::latency_control_type, Type>,
std::integral_constant<int, Cycle>> {
using key_t = intel::experimental::latency_constraint_key;
static constexpr int target = Target;
static constexpr intel::experimental::latency_control_type type = Type;
static constexpr int cycle = Cycle;
Expand Down
21 changes: 8 additions & 13 deletions sycl/include/sycl/ext/oneapi/properties/properties.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ template <typename... Ts> struct RuntimePropertyStorage<std::tuple<Ts...>> {
};
template <typename T, typename... Ts>
struct RuntimePropertyStorage<std::tuple<T, Ts...>>
: std::conditional_t<IsRuntimeProperty<T>::value,
: std::conditional_t<IsRuntimePropertyValue<T>::value,
PrependTuple<T, typename RuntimePropertyStorage<
std::tuple<Ts...>>::type>,
RuntimePropertyStorage<std::tuple<Ts...>>> {};
Expand Down Expand Up @@ -149,9 +149,9 @@ template <typename PropertiesT> class properties {
template <typename PropertyT>
typename std::enable_if_t<detail::IsRuntimeProperty<PropertyT>::value &&
has_property<PropertyT>(),
PropertyT>
typename PropertyT::value_t>
get_property() const {
return std::get<PropertyT>(Storage);
return std::get<typename PropertyT::value_t>(Storage);
}

template <typename PropertyT>
Expand Down Expand Up @@ -247,22 +247,17 @@ template <typename SyclT, typename PropertiesT>
struct all_props_are_keys_of : std::true_type {};

template <typename SyclT>
struct all_props_are_keys_of<
SyclT, ext::oneapi::experimental::detail::empty_properties_t>
: std::true_type {};
struct all_props_are_keys_of<SyclT, empty_properties_t> : std::true_type {};

template <typename SyclT, typename PropT>
struct all_props_are_keys_of<
SyclT, ext::oneapi::experimental::properties<std::tuple<PropT>>>
struct all_props_are_keys_of<SyclT, properties<std::tuple<PropT>>>
: std::bool_constant<
ext::oneapi::experimental::is_property_key_of<PropT, SyclT>::value> {
};
is_property_key_of<key_from_value<PropT>, SyclT>::value> {};

template <typename SyclT, typename PropT, typename... PropTs>
struct all_props_are_keys_of<
SyclT, ext::oneapi::experimental::properties<std::tuple<PropT, PropTs...>>>
struct all_props_are_keys_of<SyclT, properties<std::tuple<PropT, PropTs...>>>
: std::bool_constant<
ext::oneapi::experimental::is_property_key_of<PropT, SyclT>::value &&
is_property_key_of<key_from_value<PropT>, SyclT>::value &&
all_props_are_keys_of<SyclT, PropTs...>()> {};

} // namespace detail
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ struct HasValue<T, decltype((void)T::value, 0)> : std::true_type {};
template <typename PropertyT>
struct IsCompileTimePropertyValue : std::false_type {};

template <typename PropertyT>
struct IsRuntimePropertyValue : std::false_type {};
// Checks if a type is either a runtime property or if it is a compile-time
// property
template <typename T> struct IsProperty {
Expand All @@ -73,7 +75,7 @@ template <typename T> struct IsProperty {
// property_value with a valid compile-time property
template <typename T> struct IsPropertyValue {
static constexpr bool value =
IsRuntimeProperty<T>::value || IsCompileTimePropertyValue<T>::value;
IsRuntimePropertyValue<T>::value || IsCompileTimePropertyValue<T>::value;
};

// Checks that all types in a tuple are valid properties.
Expand Down
38 changes: 21 additions & 17 deletions sycl/include/sycl/ext/oneapi/properties/property_value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,22 @@ struct PropertyValueBase<T> : public detail::SingleNontypePropertyValueBase<T> {
using value_t = T;
};

// Get property key from value. Key is always first template argument.
template <class T>
using key_from_value = sycl::detail::boost::mp11::mp_front<T>;

template <class T>
using key_from_value_ignore_const = key_from_value<std::remove_const_t<T>>;

// Return void if not a valid value
template <class T>
using key_from_value_or_void =
sycl::detail::boost::mp11::mp_eval_or<void, key_from_value_ignore_const, T>;

} // namespace detail

template <typename PropertyT, typename... Ts>
struct property_value : public detail::PropertyValueBase<Ts...> {
using key_t = PropertyT;
};
struct property_value : public detail::PropertyValueBase<Ts...> {};

template <typename PropertyT, typename... A, typename... B>
constexpr std::enable_if_t<detail::IsCompileTimeProperty<PropertyT>::value,
Expand All @@ -56,22 +66,12 @@ operator!=(const property_value<PropertyT, A...> &,
const property_value<PropertyT, B...> &) {
return (!std::is_same<A, B>::value || ...);
}

template <typename V, typename = void> struct is_property_value {
static constexpr bool value =
detail::IsRuntimeProperty<V>::value && is_property_key<V>::value;
};
template <typename V, typename O, typename = void> struct is_property_value_of {
static constexpr bool value =
detail::IsRuntimeProperty<V>::value && is_property_key_of<V, O>::value;
};
// Specialization for compile-time-constant properties
template <typename V>
struct is_property_value<V, std::void_t<typename V::key_t>>
: is_property_key<typename V::key_t> {};
using is_property_value = is_property_key<detail::key_from_value_or_void<V>>;

template <typename V, typename O>
struct is_property_value_of<V, O, std::void_t<typename V::key_t>>
: is_property_key_of<typename V::key_t, O> {};
using is_property_value_of =
is_property_key_of<detail::key_from_value_or_void<V>, O>;

namespace detail {

Expand All @@ -85,6 +85,10 @@ template <typename PropertyT, typename... PropertyValueTs>
struct IsCompileTimePropertyValue<property_value<PropertyT, PropertyValueTs...>>
: IsCompileTimeProperty<PropertyT> {};

template <typename PropertyT, typename... PropertyValueTs>
struct IsRuntimePropertyValue<property_value<PropertyT, PropertyValueTs...>>
: IsRuntimeProperty<PropertyT> {};

} // namespace detail
} // namespace ext::oneapi::experimental
} // namespace _V1
Expand Down
29 changes: 17 additions & 12 deletions sycl/include/sycl/kernel_bundle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -813,12 +813,15 @@ namespace ext::oneapi::experimental {
/////////////////////////
// PropertyT syclex::build_options
/////////////////////////
struct build_options {
struct build_options_key {
using value_t = property_value<build_options_key>;
};
using build_options = property_value<build_options_key>;
template <> struct property_value<build_options_key> {
std::vector<std::string> opts;
build_options(const std::string &optsArg) : opts{optsArg} {}
build_options(const std::vector<std::string> &optsArg) : opts(optsArg) {}
property_value(const std::string &optsArg) : opts{optsArg} {}
property_value(const std::vector<std::string> &optsArg) : opts(optsArg) {}
};
using build_options_key = build_options;

template <> struct is_property_key<build_options_key> : std::true_type {};

Expand Down Expand Up @@ -847,12 +850,14 @@ struct IsCompileTimeProperty<sycl::ext::oneapi::experimental::build_options_key>
/////////////////////////
// PropertyT syclex::save_log
/////////////////////////
struct save_log {
struct save_log_key {
using value_t = property_value<save_log_key>;
};
using save_log = property_value<save_log_key>;
template <> struct property_value<save_log_key> {
std::string *log;
save_log(std::string *logArg) : log(logArg) {}
property_value(std::string *logArg) : log(logArg) {}
};
using save_log_key = save_log;

template <> struct is_property_key<save_log_key> : std::true_type {};

template <>
Expand Down Expand Up @@ -917,11 +922,11 @@ build(kernel_bundle<bundle_state::ext_oneapi_source> &SourceKB,
const std::vector<device> &Devices, PropertyListT props = {}) {
std::vector<std::string> BuildOptionsVec;
std::string *LogPtr = nullptr;
if constexpr (props.template has_property<build_options>()) {
BuildOptionsVec = props.template get_property<build_options>().opts;
if constexpr (props.template has_property<build_options_key>()) {
BuildOptionsVec = props.template get_property<build_options_key>().opts;
}
if constexpr (props.template has_property<save_log>()) {
LogPtr = props.template get_property<save_log>().log;
if constexpr (props.template has_property<save_log_key>()) {
LogPtr = props.template get_property<save_log_key>().log;
}
return detail::build_from_source(SourceKB, Devices, BuildOptionsVec, LogPtr);
}
Expand Down
Loading