Skip to content

Commit 9ad21e1

Browse files
committed
[SYCL] Optimize handling of compile-time properties
We perform correctness check of compile-time properties in `handler::processProperties` helper function. That function was specialized by kernel name, meaning that if we have two kernels with absolutely the same properties applied to them there will be two instantiations of `processProperties`. That consumes compilation time for both front-end which has to emit extra isntantiations and for host compilation pass which gets more functions (with the same body!) to handle. The only use of kernel name within `processProperties` is to check if that kernel is a ESIMD kernel. That can be done at caller side and propagated to `processProperties` as a simple boolean, thus reducing amount of instantiations of that function. This patch does exactly that. Please note that this is technically a *functional* change: even though we still process the properties in the same way, we will now emit diagnostics in a slightly different manner: if there are two kernels with the same set of illegal properties there will be only one diagnostic for the first kernel. Once that first kernel is fixed, the diagnostic will be displayed for the second kernel, i.e. we won't display _all_ violations in a single compilation run. It seems like we only have one test which exposes that behavior and considering that with C++ it is almost always you should fix the first error first to see what happens to the rest, I don't think that such change in diagnostics is bad enough to outweight potential (even if they are the slightest) compilation time improvements.
1 parent 3e98b3a commit 9ad21e1

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

sycl/include/sycl/handler.hpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ class __SYCL_EXPORT handler {
10471047
///
10481048
/// Stores information about kernel properties into the handler.
10491049
template <
1050-
typename KernelName,
1050+
bool IsESIMDKernel,
10511051
typename PropertiesT = ext::oneapi::experimental::empty_properties_t>
10521052
void processProperties(PropertiesT Props) {
10531053
static_assert(
@@ -1058,7 +1058,7 @@ class __SYCL_EXPORT handler {
10581058
sycl::ext::intel::experimental::fp_control_key>() ||
10591059
(PropertiesT::template has_property<
10601060
sycl::ext::intel::experimental::fp_control_key>() &&
1061-
detail::isKernelESIMD<KernelName>()),
1061+
IsESIMDKernel),
10621062
"Floating point control property is supported for ESIMD kernels only.");
10631063
static_assert(
10641064
!PropertiesT::template has_property<
@@ -1398,7 +1398,7 @@ class __SYCL_EXPORT handler {
13981398
kernel_parallel_for_wrapper<NameT, TransformedArgType, KernelType,
13991399
PropertiesT>(KernelFunc);
14001400
#ifndef __SYCL_DEVICE_ONLY__
1401-
processProperties<NameT, PropertiesT>(Props);
1401+
processProperties<detail::isKernelESIMD<NameT>(), PropertiesT>(Props);
14021402
detail::checkValueRange<Dims>(UserRange);
14031403
setNDRangeDescriptor(std::move(UserRange));
14041404
StoreLambda<NameT, KernelType, Dims, TransformedArgType>(
@@ -1450,7 +1450,7 @@ class __SYCL_EXPORT handler {
14501450
#ifndef __SYCL_DEVICE_ONLY__
14511451
detail::checkValueRange<Dims>(ExecutionRange);
14521452
setNDRangeDescriptor(std::move(ExecutionRange));
1453-
processProperties<NameT, PropertiesT>(Props);
1453+
processProperties<detail::isKernelESIMD<NameT>(), PropertiesT>(Props);
14541454
StoreLambda<NameT, KernelType, Dims, TransformedArgType>(
14551455
std::move(KernelFunc));
14561456
setType(detail::CGType::Kernel);
@@ -1532,7 +1532,7 @@ class __SYCL_EXPORT handler {
15321532
kernel_parallel_for_work_group_wrapper<NameT, LambdaArgType, KernelType,
15331533
PropertiesT>(KernelFunc);
15341534
#ifndef __SYCL_DEVICE_ONLY__
1535-
processProperties<NameT, PropertiesT>(Props);
1535+
processProperties<detail::isKernelESIMD<NameT>(), PropertiesT>(Props);
15361536
detail::checkValueRange<Dims>(NumWorkGroups);
15371537
setNDRangeDescriptor(NumWorkGroups, /*SetNumWorkGroups=*/true);
15381538
StoreLambda<NameT, KernelType, Dims, LambdaArgType>(std::move(KernelFunc));
@@ -1574,7 +1574,7 @@ class __SYCL_EXPORT handler {
15741574
kernel_parallel_for_work_group_wrapper<NameT, LambdaArgType, KernelType,
15751575
PropertiesT>(KernelFunc);
15761576
#ifndef __SYCL_DEVICE_ONLY__
1577-
processProperties<NameT, PropertiesT>(Props);
1577+
processProperties<detail::isKernelESIMD<NameT>(), PropertiesT>(Props);
15781578
nd_range<Dims> ExecRange =
15791579
nd_range<Dims>(NumWorkGroups * WorkGroupSize, WorkGroupSize);
15801580
detail::checkValueRange<Dims>(ExecRange);
@@ -1774,7 +1774,7 @@ class __SYCL_EXPORT handler {
17741774
if constexpr (ext::oneapi::experimental::detail::
17751775
HasKernelPropertiesGetMethod<
17761776
_KERNELFUNCPARAMTYPE>::value) {
1777-
processProperties<KernelName>(
1777+
processProperties<detail::isKernelESIMD<KernelName>()>(
17781778
KernelFunc.get(ext::oneapi::experimental::properties_tag{}));
17791779
}
17801780
#endif
@@ -1853,7 +1853,7 @@ class __SYCL_EXPORT handler {
18531853
// No need to check if range is out of INT_MAX limits as it's compile-time
18541854
// known constant.
18551855
setNDRangeDescriptor(range<1>{1});
1856-
processProperties<NameT, PropertiesT>(Props);
1856+
processProperties<detail::isKernelESIMD<NameT>(), PropertiesT>(Props);
18571857
StoreLambda<NameT, KernelType, /*Dims*/ 1, void>(KernelFunc);
18581858
setType(detail::CGType::Kernel);
18591859
#endif

sycl/test/virtual-functions/properties-negative.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ int main() {
1919

2020
// expected-error-re@sycl/handler.hpp:* {{static assertion failed due to requirement {{.*}} indirectly_callable property cannot be applied to SYCL kernels}}
2121
q.single_task(props_empty, [=]() {});
22-
// expected-error-re@sycl/handler.hpp:* {{static assertion failed due to requirement {{.*}} indirectly_callable property cannot be applied to SYCL kernels}}
22+
// When both "props_empty" and "props_void" are in use, we won't see the
23+
// static assert firing for the second one, because there will be only one
24+
// instantiation of handler::processProperties.
2325
q.single_task(props_void, [=]() {});
2426
// expected-error-re@sycl/handler.hpp:* {{static assertion failed due to requirement {{.*}} indirectly_callable property cannot be applied to SYCL kernels}}
2527
q.single_task(props_int, [=]() {});

0 commit comments

Comments
 (0)