Skip to content

Commit 507679a

Browse files
authored
[SYCL] Correct spec about constness of get(properties_tag) member and add respective warning (#17947)
Currently some of our docs suggest using `get(properties_tag) {...}` member in kernel functors to specify kernel properties while some suggest using `get(properties_tag) const {...}`. But actually `get(properties_tag) {...}` might not work under certain conditions, so we should update the incorrect docs that they suggest readers to use `get(properties_tag) const {...}` rather than `get(properties_tag) {...}`. Also as @Pennycook suggested, added a diagnostic warning to indicate the user when: -they have declared a `get(properties_tag)` member in the kernel functor AND -they declared `get(properties_tag){...}` rather than `get(properties_tag) const {...}` Updated the tests containing the non-const version of `get(properties_tag)` as well. --------- Signed-off-by: Hu, Peisen <[email protected]>
1 parent 3439c78 commit 507679a

23 files changed

+110
-53
lines changed

sycl/doc/design/CompileTimeProperties.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ void foo(handler &cgh) {
298298
```
299299

300300
The second way an application can specify kernel properties is by adding a
301-
member function named `get(sycl::ext::oneapi::properties_tag)` to a named
301+
const member function named `get(sycl::ext::oneapi::properties_tag)` to a named
302302
kernel function object:
303303

304304
```
@@ -309,7 +309,7 @@ class MyKernel {
309309
public:
310310
void operator()() {/* ... */}
311311
312-
auto get(properties_tag) {
312+
auto get(properties_tag) const {
313313
return properties{sub_group_size<32>, device_has<aspect::fp16>};
314314
}
315315
};

sycl/doc/extensions/experimental/sycl_ext_oneapi_kernel_properties.asciidoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ that it depends upon for correctness.
295295

296296
To enable this use-case, this extension adds a mechanism for implementations to
297297
extract a property list from a kernel functor, if a kernel functor declares
298-
a member function named `get` accepting a `sycl::ext::oneapi::experimental::properties_tag`
298+
a const member function named `get` accepting a `sycl::ext::oneapi::experimental::properties_tag`
299299
tag type and returning an instance of `sycl::ext::oneapi::experimental::properties`.
300300

301301
```c++
@@ -338,7 +338,7 @@ struct KernelFunctor {
338338
a[i] = b[i] + c[i];
339339
}
340340

341-
auto get(sycl::ext::oneapi::experimental::properties_tag) {
341+
auto get(sycl::ext::oneapi::experimental::properties_tag) const {
342342
return sycl::ext::oneapi::experimental::properties{sycl::ext::oneapi::experimental::work_group_size<8, 8>,
343343
sycl::ext::oneapi::experimental::sub_group_size<8>};
344344
}

sycl/doc/extensions/proposed/sycl_ext_intel_fpga_kernel_interface_properties.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ struct KernelFunctor {
319319
*a = *b + *c;
320320
}
321321
322-
auto get(properties_tag) {
322+
auto get(properties_tag) const {
323323
return properties{streaming_interface_accept_downstream_stall};
324324
}
325325

sycl/include/sycl/handler.hpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,6 +1596,17 @@ class __SYCL_EXPORT handler {
15961596
KernelFunc.get(ext::oneapi::experimental::properties_tag{}));
15971597
}
15981598
#endif
1599+
// Note: the static_assert below need to be run on both the host and the
1600+
// device ends to avoid test issues, so don't put it into the #ifdef
1601+
// __SYCL_DEVICE_ONLY__ directive above print out diagnostic message if
1602+
// the kernel functor has a get(properties_tag) member, but it's not const
1603+
static_assert(
1604+
(ext::oneapi::experimental::detail::HasKernelPropertiesGetMethod<
1605+
const KernelType &>::value) ||
1606+
!(ext::oneapi::experimental::detail::HasKernelPropertiesGetMethod<
1607+
KernelType>::value),
1608+
"get(sycl::ext::oneapi::experimental::properties_tag) member in "
1609+
"kernel functor class must be declared as a const member function");
15991610
auto L = [&](auto &&...args) {
16001611
if constexpr (WrapAsVal == WrapAs::single_task) {
16011612
h->kernel_single_task<KernelName, KernelType, MergedProps...>(

sycl/include/syclcompat/launch_policy.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ struct KernelFunctor {
219219
: _kernel_properties{kernel_props}, _local_acc{local_acc},
220220
_argument_tuple(std::make_tuple(args...)) {}
221221

222-
auto get(sycl_exp::properties_tag) { return _kernel_properties; }
222+
auto get(sycl_exp::properties_tag) const { return _kernel_properties; }
223223

224224
__syclcompat_inline__ void
225225
operator()(syclcompat::detail::range_to_item_t<Range>) const {

sycl/test-e2e/Basic/max_linear_work_group_size_props.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ template <size_t I> struct KernelFunctorWithMaxWGSizeProp {
5252
void operator()(nd_item<1>) const {}
5353
void operator()(item<1>) const {}
5454

55-
auto get(sycl::ext::oneapi::experimental::properties_tag) {
55+
auto get(sycl::ext::oneapi::experimental::properties_tag) const {
5656
return sycl::ext::oneapi::experimental::properties{
5757
sycl::ext::oneapi::experimental::max_linear_work_group_size<I>};
5858
}

sycl/test-e2e/Basic/max_work_group_size_props.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ template <size_t... Is> struct KernelFunctorWithMaxWGSizeProp {
4343
void operator()(nd_item<sizeof...(Is)>) const {}
4444
void operator()(item<sizeof...(Is)>) const {}
4545

46-
auto get(sycl::ext::oneapi::experimental::properties_tag) {
46+
auto get(sycl::ext::oneapi::experimental::properties_tag) const {
4747
return sycl::ext::oneapi::experimental::properties{
4848
sycl::ext::oneapi::experimental::max_work_group_size<Is...>};
4949
}

sycl/test-e2e/Basic/sub_group_size_prop.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ template <size_t SGSize> struct KernelFunctorWithSGSizeProp {
2424
Acc[0] = SG.get_local_linear_range();
2525
}
2626

27-
auto get(sycl::ext::oneapi::experimental::properties_tag) {
27+
auto get(sycl::ext::oneapi::experimental::properties_tag) const {
2828
return sycl::ext::oneapi::experimental::properties{
2929
sycl::ext::oneapi::experimental::sub_group_size<SGSize>};
3030
}

sycl/test-e2e/Basic/work_group_size_prop.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ template <size_t... Is> struct KernelFunctorWithWGSizeProp {
3939
void operator()(nd_item<sizeof...(Is)>) const {}
4040
void operator()(item<sizeof...(Is)>) const {}
4141

42-
auto get(sycl::ext::oneapi::experimental::properties_tag) {
42+
auto get(sycl::ext::oneapi::experimental::properties_tag) const {
4343
return sycl::ext::oneapi::experimental::properties{
4444
sycl::ext::oneapi::experimental::work_group_size<Is...>};
4545
}

sycl/test-e2e/Graph/Inputs/sub_group_prop.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ template <size_t SGSize> struct KernelFunctorWithSGSizeProp {
1919
Acc[0] = SG.get_local_linear_range();
2020
}
2121

22-
auto get(sycl::ext::oneapi::experimental::properties_tag) {
22+
auto get(sycl::ext::oneapi::experimental::properties_tag) const {
2323
return sycl::ext::oneapi::experimental::properties{
2424
sycl::ext::oneapi::experimental::sub_group_size<SGSize>};
2525
}

0 commit comments

Comments
 (0)