SYCL ext one api launch queries implementation #16709

dklochkov-emb · 2025-01-21T11:21:47Z

Added 4 sycl ext oneapi launch queries:
max_work_item_sizes,
max_work_group_size,
max_sub_group_size,
num_sub_groups
see docs
Unit tests added to check all new queries, mock class is updated for this purpose
e2e tests added

…nsions

[SYCL] fix formatting

AlexeySachkov · 2025-01-22T12:28:58Z

sycl/source/detail/kernel_impl.hpp

+      getSyclObjImpl(Queue.get_device())->getHandleRef();
+
+  size_t KernelWGSize = 0;
+  if (auto Result = Adapter->call_nocheck<UrApiKind::urKernelGetGroupInfo>(


Documentation for urKernelGetGroupInfo does not list UR_RESULT_ERROR_UNSUPPORTED_FEATURE as a possible error code, meaning that this API should be supported by every adapter.

Therefore, we should be able to simplify this code and other queries which use the same API call

AlexeySachkov · 2025-01-22T13:38:07Z

sycl/source/kernel.cpp

+        ext::oneapi::experimental::info::kernel_queue_specific::num_sub_groups>(
+        queue Queue, const range<2> &) const;
+
+template __SYCL_EXPORT typename ext::oneapi::experimental::info::


Lots of repeated code here, I would suggest that we hide it under a macro like it is done for max_num_work_groups below

AlexeySachkov · 2025-01-22T13:41:00Z

sycl/test-e2e/Basic/launch_queries/max_sub_group_size.cpp

@@ -0,0 +1,86 @@
+// REQUIRES aspect-subgroup


There is no dedicated aspect for sub-groups (SYCL 2020 spec) and the same goes for the launch queries extension spec - it does not say that any of those queries require any aspects to be supported by a device, i.e. they should work on every device.

This test would have been always skipped if not for missing : which is required for a LIT directive to work.

The concept is required to be supported by every device, but device can say that it only provides a single sub-group per work-group, or say that maximum sub-group size is equal to 1 to indicate lack of support for the feature.

AlexeySachkov · 2025-01-22T13:44:09Z

sycl/test-e2e/Basic/launch_queries/max_sub_group_size.cpp

+#include <sycl/detail/info_desc_helpers.hpp>
+#include <sycl/kernel.hpp>
+#include <sycl/kernel_bundle.hpp>
+#include <sycl/sycl.hpp>


You must not include sycl.hpp in E2E tests, it is way to heavy. Instead, you should you specific headers you need directly

AlexeySachkov · 2025-01-22T13:45:17Z

sycl/test-e2e/Basic/launch_queries/max_sub_group_size.cpp

I would also move this file into a separate sub-directory under test-e2e. SYCL extensions are not basic functionality. Personally, I don't like us having tests for non-standard things in Basic folder

AlexeySachkov · 2025-01-22T13:46:41Z

sycl/test-e2e/Basic/launch_queries/max_sub_group_size.cpp

+  auto bundle = sycl::get_kernel_bundle<sycl::bundle_state::executable>(ctx);
+  auto kernel = bundle.template get_kernel<kernels::TestKernel>();
+
+  // get value to compare with


Here you submit a distinct kernel, which is different from the one described by TestKernel functor. There is no guarantee that maximum sub-group size you get here is the same the one you will get for TestKernel.

[SYCL] fix windows dump test

[SYCL] fix max work group size e2e test

…unch-queries

dklochkov-emb · 2025-02-14T11:49:24Z

@AlexeySachkov @intel/llvm-reviewers-runtime @cperkinsintel Please, review it. Some of the tests fail due to other problems, which are not related to these changes - it was checked 2-3 last precommit checks.
Most of the comments are fixed.

sycl/source/detail/kernel_impl.hpp

steffenlarsen · 2025-02-21T14:08:24Z

sycl/source/detail/kernel_impl.hpp

+                         UR_KERNEL_SUB_GROUP_INFO_MAX_NUM_SUB_GROUPS)
+ADD_TEMPLATE_METHOD_SPEC(num_sub_groups, 2, urKernelGetSubGroupInfo,
+                         UR_KERNEL_SUB_GROUP_INFO_MAX_NUM_SUB_GROUPS)
+ADD_TEMPLATE_METHOD_SPEC(num_sub_groups, 1, urKernelGetSubGroupInfo,


Nit; Looks like they all use urKernelGetSubGroupInfo as their Kind argument. I think it would be easier to read if you just inline it instead.

sycl/source/detail/kernel_impl.hpp

sycl/test-e2e/Experimental/launch_queries/max_sub_group_size.cpp

sycl/test-e2e/Experimental/launch_queries/max_sub_groups.cpp

sycl/test-e2e/Experimental/launch_queries/max_work_item_sizes.cpp

Co-authored-by: Steffen Larsen <[email protected]>

…ch-queries

steffenlarsen

https://github.com/intel/llvm/pull/16709/files#r1965532820 is still applicable, but it's not critical, so you have my approval. Additionally, great work on the tests! 🚀

lbushi25 · 2025-02-25T19:03:39Z

sycl/source/detail/kernel_impl.hpp

+  /// \param Queue is a valid SYCL queue.
+  /// \param WG is a work group size
+  /// \return depends on information being queried.
+  template <typename Param>


Should we perhaps use some kind of SFINAE here to explicitly check that Param has the value defined in the spec?

It seems like at the moment we are doing SFINAE only on the return type by saying Param::return_type but there could be other descriptors that will have this type defined and we will end up instantiating declarations for them if the user supplies an invalid Param by mistake. This could lead to some weird linker errors or some runtime bug.

This isn't user-visible API. Don't we have proper constraints there?

The user-visible API in kernel.hpp which calls this one also does something similar with typename detail::is_kernel_queue_specific_info_desc<Param>::return_type which again, could potentially allow other Param types to slip through the cracks. Ideally, we'd like to follow the spec to the letter and mandate Param to be as required by the spec through SFINAE, in this case for example, to be max_num_work_groups.

sarnex · 2025-02-26T17:32:07Z

@dklochkov-emb Failing in postcommit on FPGA:

env ONEAPI_DEVICE_SELECTOR=opencl:fpga  /__w/llvm/llvm/build-e2e/Experimental/launch_queries/Output/max_sub_groups.cpp.tmp.out
# executed command: env ONEAPI_DEVICE_SELECTOR=opencl:fpga /__w/llvm/llvm/build-e2e/Experimental/launch_queries/Output/max_sub_groups.cpp.tmp.out
# .---command stderr------------
# | max_sub_groups.cpp.tmp.out: /__w/llvm/llvm/llvm/sycl/test-e2e/Experimental/launch_queries/max_sub_groups.cpp:67: int main(): Assertion `subSGSize <= maxDeviceValue' failed.
# | Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
# | 0  libsycl.so.8               0x00007f84036bc892
# | 1  libsycl.so.8               0x00007f84036b9d56
# | 2  libc.so.6                  0x00007f84031a7330
# | 3  libc.so.6                  0x00007f8403200b2c pthread_kill + 284
# | 4  libc.so.6                  0x00007f84031a727e gsignal + 30
# | 5  libc.so.6                  0x00007f840318a8ff abort + 223
# | 6  libc.so.6                  0x00007f840318a81b
# | 7  libc.so.6                  0x00007f840319d517
# | 8  max_sub_groups.cpp.tmp.out 0x00000000004038db
# | 9  libc.so.6                  0x00007f840318c1ca
# | 10 libc.so.6                  0x00007f840318c28b __libc_start_main + 139
# | 11 max_sub_groups.cpp.tmp.out 0x0000000000403585
# `-----------------------------
# error: command failed with exit status: -6

Please fix it ASAP and if not possible disable the test/revert the change. Thx

AlexeySachkov · 2025-02-26T21:31:05Z

@dklochkov-emb Failing in postcommit on FPGA:
Please fix it ASAP and if not possible disable the test/revert the change. Thx

Considering that we are dropping FPGA support, I would say let's just XFAIL the test on this platform for now

steffenlarsen · 2025-02-27T07:07:19Z

I have opened #17209 and #17210.

dklochkov-emb added 8 commits December 9, 2024 05:41

[SYCL] implement max work group size query

1ee3633

[SYCL] adding trats and dumps for new query

61dce52

[SYCL] rework max work group size

76e7b8f

[SYCL] add get kernel sub group size query

c746138

[SYCL] add new queries

ae047a2

[SYCL] make max_work_item_sizes dependent on dimensions

6da7073

[SYCL] make num_sub_groups and max_sub_group_size dependent from dime…

8da4056

…nsions

[SYCL] update tests of launch queries

8ba6d06

dklochkov-emb self-assigned this Jan 21, 2025

dklochkov-emb requested a review from a team as a code owner January 21, 2025 11:21

dklochkov-emb requested a review from cperkinsintel January 21, 2025 11:21

dklochkov-emb marked this pull request as draft January 21, 2025 11:21

Merge branch 'sycl' into sycl-ext-one-api-launch-queries

78c54cb

dklochkov-emb had a problem deploying to WindowsCILock January 21, 2025 11:28 — with GitHub Actions Failure

dklochkov-emb added 2 commits January 21, 2025 12:55

[SYCL] fix merge issues

38351d2

[SYCL] fix formatting

613aeac

dklochkov-emb had a problem deploying to WindowsCILock January 21, 2025 14:57 — with GitHub Actions Error

dklochkov-emb had a problem deploying to WindowsCILock January 21, 2025 15:13 — with GitHub Actions Error

[SYCL] fix formatting in tests

f98a316

[SYCL] fix formatting

dklochkov-emb force-pushed the sycl-ext-one-api-launch-queries branch from 3b47f16 to f98a316 Compare January 21, 2025 15:22

dklochkov-emb had a problem deploying to WindowsCILock January 21, 2025 15:23 — with GitHub Actions Failure

dklochkov-emb had a problem deploying to WindowsCILock January 21, 2025 15:52 — with GitHub Actions Failure

AlexeySachkov reviewed Jan 22, 2025

View reviewed changes

[SYCL] use sub group struct in mock

f09be72

[SYCL] fix windows dump test

dklochkov-emb temporarily deployed to WindowsCILock January 22, 2025 14:12 — with GitHub Actions Inactive

dklochkov-emb had a problem deploying to WindowsCILock January 22, 2025 14:41 — with GitHub Actions Failure

[SYCL] fix mock in unit tests

f4ffa52

[SYCL] fix max work group size e2e test

dklochkov-emb temporarily deployed to WindowsCILock January 23, 2025 18:11 — with GitHub Actions Inactive

dklochkov-emb had a problem deploying to WindowsCILock January 23, 2025 19:54 — with GitHub Actions Failure

[SYCL] do not include heavy headers into e2e test files

40cb651

Merge remote-tracking branch 'upstream/sycl' into sycl-ext-one-api-la…

1a5837b

…unch-queries

dklochkov-emb temporarily deployed to WindowsCILock February 14, 2025 11:45 — with GitHub Actions Inactive

dklochkov-emb temporarily deployed to WindowsCILock February 14, 2025 12:14 — with GitHub Actions Inactive

steffenlarsen reviewed Feb 20, 2025

View reviewed changes

sycl/source/detail/kernel_impl.hpp Outdated Show resolved Hide resolved

dklochkov-emb changed the title ~~Sycl ext one api launch queries implementation~~ SYCL ext one api launch queries implementation Feb 21, 2025

[SYCL] Update param description of new queries

d25062b

dklochkov-emb temporarily deployed to WindowsCILock February 21, 2025 10:52 — with GitHub Actions Inactive

dklochkov-emb temporarily deployed to WindowsCILock February 21, 2025 11:43 — with GitHub Actions Inactive

steffenlarsen reviewed Feb 21, 2025

View reviewed changes

fix variables name

5df8cd3

Co-authored-by: Steffen Larsen <[email protected]>

dklochkov-emb had a problem deploying to WindowsCILock February 24, 2025 09:01 — with GitHub Actions Error

[SYCL] fix typos and add lines to the end of files

703cf26

dklochkov-emb had a problem deploying to WindowsCILock February 24, 2025 09:23 — with GitHub Actions Error

[SYCL] fix formatting

d2d29fa

dklochkov-emb temporarily deployed to WindowsCILock February 24, 2025 09:46 — with GitHub Actions Inactive

dklochkov-emb temporarily deployed to WindowsCILock February 24, 2025 10:08 — with GitHub Actions Inactive

Merge remote-tracking branch 'origin/sycl' into sycl-ext-one-api-laun…

85a88ee

…ch-queries

dklochkov-emb temporarily deployed to WindowsCILock February 24, 2025 10:58 — with GitHub Actions Inactive

dklochkov-emb temporarily deployed to WindowsCILock February 24, 2025 11:25 — with GitHub Actions Inactive

AlexeySachkov requested a review from steffenlarsen February 24, 2025 13:34

steffenlarsen approved these changes Feb 24, 2025

View reviewed changes

lbushi25 reviewed Feb 25, 2025

View reviewed changes

AlexeySachkov merged commit 4849b71 into intel:sycl Feb 26, 2025
19 checks passed

steffenlarsen mentioned this pull request Feb 27, 2025

launch_queries/max_sub_groups.cpp fails on FPGA #17209

Closed

lbushi25 mentioned this pull request Mar 3, 2025

[SYCL] Add test plan for sycl_ext_oneapi_launch_queries extension #17145

Merged

SYCL ext one api launch queries implementation #16709

SYCL ext one api launch queries implementation #16709

Uh oh!

Conversation

dklochkov-emb commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dklochkov-emb commented Feb 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steffenlarsen left a comment

Choose a reason for hiding this comment

Uh oh!

lbushi25 Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lbushi25 Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sarnex commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexeySachkov commented Feb 26, 2025

Uh oh!

steffenlarsen commented Feb 27, 2025

Uh oh!

Uh oh!

dklochkov-emb commented Jan 21, 2025 •

edited

Loading

lbushi25 Feb 25, 2025 •

edited

Loading

lbushi25 Feb 25, 2025 •

edited

Loading

sarnex commented Feb 26, 2025 •

edited

Loading