[SYCL] Implement queue::ext_oneapi_empty() API to get queue status #7583

againull · 2022-11-29T23:09:17Z

sycl/plugins/level_zero/pi_level_zero.cpp

smaslov-intel · 2022-11-29T23:39:10Z

where is this extension documented?

againull · 2022-11-30T00:00:25Z

where is this extension documented?

Extension is documented here:
https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_queue_status_query.asciidoc

As we discussed with @gmlueck, we definitely need ext_oneapi_empy. ext_oneapi_size/ext_oneapi_get_wait_list necessity is going to be discussed and most likely will be dropped. That's why this PR is implementin ext_oneapi_empty only.

sycl/plugins/level_zero/pi_level_zero.cpp

sycl/include/sycl/detail/pi.h

sycl/plugins/cuda/pi_cuda.cpp

sycl/plugins/level_zero/pi_level_zero.cpp

sycl/include/sycl/detail/pi.h

* Catch exception in the cuda plugin * Use hasOpenCommandList in the L0 plugin * Rename PI_QUEUE_INFO_STATUS -> PI_EXT_ONEAPI_QUEUE_INFO_STATUS and update int value

Immediate command lists are not associated with any L0 command queue. So we need to check status of the events on eacn immediate command lists to get the status of the queue.

sycl/plugins/level_zero/pi_level_zero.cpp

sycl/source/detail/queue_impl.cpp

sycl/plugins/level_zero/pi_level_zero.cpp

sycl/plugins/opencl/pi_opencl.cpp

againull · 2022-12-01T23:03:02Z

I guess l will need to mention this limitation in the extension documentation, correct?

Yes, we usually do this by adding a NOTE callout in the "Status" section. Something like:

This extension is currently support fully by DPC++ in both the Level Zero and CUDA backends, but there are limitations when running on the OpenCL backend. Attempting to call queue::empty() for the OpenCL backend will trigger an assertion unless the queue has the in_order property.

What is the current behavior if the application attempts to call this AP on OpenCL? Does it raise an assertion failure? I'm not sure what we normally do for unsupported features.

Exception will be thrown if somebody attempts to call this API on OpenCL.

* Rename PI_EXT_ONEAPI_QUEUE_INFO_STATUS->PI_EXT_ONEAPI_QUEUE_INFO_EMPTY * Document return type in pi.h * Define feature macro

one more thing

smaslov-intel · 2022-12-02T00:25:44Z

sycl/include/sycl/detail/pi.h

@@ -58,6 +58,8 @@
 // piDeviceGetInfo.
 // 11.17 Added new PI_EXT_ONEAPI_QUEUE_PRIORITY_LOW and
 // PI_EXT_ONEAPI_QUEUE_PRIORITY_HIGH queue properties.
+// 11.18 Add new parameter name PI_EXT_ONEAPI_QUEUE_INFO_EMPTY to
+// _pi_queue_info.

 #define _PI_H_VERSION_MAJOR 11
 #define _PI_H_VERSION_MINOR 16


Please update the version

Oops, thanks, fixed.

keryell · 2022-12-02T05:15:12Z

sycl/plugins/cuda/pi_cuda.hpp

@@ -499,6 +499,30 @@ struct _pi_queue {
    return is_last_command && !has_been_synchronized(stream_token);
  }

+  template <typename T> bool all_of(T &&f) {
+    {
+      std::lock_guard<std::mutex> compute_guard(compute_stream_mutex_);


Suggested change

std::lock_guard<std::mutex> compute_guard(compute_stream_mutex_);

std::lock_guard compute_guard(compute_stream_mutex_);

keryell · 2022-12-02T05:16:22Z

sycl/plugins/cuda/pi_cuda.hpp

+      }
+    }
+    {
+      std::lock_guard<std::mutex> transfer_guard(transfer_stream_mutex_);


Suggested change

std::lock_guard<std::mutex> transfer_guard(transfer_stream_mutex_);

std::lock_guard transfer_guard(transfer_stream_mutex_);

keryell · 2022-12-02T05:20:23Z

sycl/plugins/cuda/pi_cuda.hpp

+      unsigned int end =
+          std::min(static_cast<unsigned int>(compute_streams_.size()),
+                   num_compute_streams_);
+      for (unsigned int i = 0; i < end; i++) {


This could be an algorithm.

Thanks for suggestions, fixed.

keryell · 2022-12-02T05:22:58Z

sycl/plugins/cuda/pi_cuda.hpp

+                   num_transfer_streams_);
+      for (unsigned int i = 0; i < end; i++) {
+        if (!f(transfer_streams_[i]))
+          return false;


This déjà vu suggests more abstraction.

keryell · 2022-12-02T05:25:40Z

sycl/plugins/hip/pi_hip.hpp

+        if (!f(transfer_streams_[i]))
+          return false;
+      }
+    }


We really need some heavy refactoring across the various back-ends some day... :-(

gmlueck · 2022-12-02T16:47:31Z

If my PR #7612 is merged before yours, then you should also move the extension document from "proposed" to "supported" as described in the extension process README. Be sure to change the "Status" section of the spec as indicated.

Note that #7612 was merged. Please merge the main thread into this PR, and then move / update the spec.

gmlueck

Changes to extension spec LGTM.

againull · 2022-12-02T18:56:28Z

@romanovvlad Friendly ping

keryell

Thanks.

keryell · 2022-12-02T20:31:58Z

sycl/source/detail/queue_impl.cpp

+  // If we have in-order queue where events are not discarded then just check
+  // the status of the last event.
+  if (isInOrder() && !MDiscardEvents) {
+    std::lock_guard<std::mutex> Lock(MLastEventMtx);


Use C++17 CTAD everywhere. There are still a lot in the new code.

againull · 2022-12-05T04:59:47Z

Reported unrelated HIP backend failures here: #7634

againull added 5 commits November 23, 2022 10:53

[SYCL] Implement queue::ext_oneapi_empty() API to get queue status

0035ffd

Add piQueueGetInfo parameter to query queue status

cfe66d9

[SYCL] Add implementation for CUDA and HIP

92e5286

Support for out-of-order queues except OpenCL

7883ffe

Execute open command lists before querying queue status

da0de48

againull requested review from a team as code owners November 29, 2022 23:09

againull requested review from romanovvlad and smaslov-intel November 29, 2022 23:09

Merge remote-tracking branch 'origin/sycl' into ext_oneapi_empty

50977e4

smaslov-intel reviewed Nov 29, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed Nov 29, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

Address review comments

133df85

smaslov-intel reviewed Nov 30, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed Nov 30, 2022

View reviewed changes

sycl/include/sycl/detail/pi.h Show resolved Hide resolved

Fix ABI test files

1069bad

romanovvlad reviewed Nov 30, 2022

View reviewed changes

sycl/plugins/cuda/pi_cuda.cpp Outdated Show resolved Hide resolved

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

sycl/include/sycl/detail/pi.h Outdated Show resolved Hide resolved

againull added 2 commits November 30, 2022 10:01

Address review comments

6dc2c24

* Catch exception in the cuda plugin * Use hasOpenCommandList in the L0 plugin * Rename PI_QUEUE_INFO_STATUS -> PI_EXT_ONEAPI_QUEUE_INFO_STATUS and update int value

Take care of immediate command lists

2f39b72

Immediate command lists are not associated with any L0 command queue. So we need to check status of the events on eacn immediate command lists to get the status of the queue.

smaslov-intel reviewed Nov 30, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Show resolved Hide resolved

smaslov-intel reviewed Nov 30, 2022

View reviewed changes

sycl/source/detail/queue_impl.cpp Show resolved Hide resolved

smaslov-intel reviewed Nov 30, 2022

View reviewed changes

sycl/source/detail/queue_impl.cpp Outdated Show resolved Hide resolved

againull added 2 commits November 30, 2022 15:44

Throw an error from opencl plugin instead of SYCL RT

565ba4d

Handle in-order and discarded queues in the L0 plugin

e9b0355

againull requested review from smaslov-intel and romanovvlad December 1, 2022 00:11

againull mentioned this pull request Dec 1, 2022

[SYCL] Test queue::ext_oneapi_empty() API intel/llvm-test-suite#1427

Merged

smaslov-intel reviewed Dec 1, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed Dec 1, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Outdated Show resolved Hide resolved

smaslov-intel reviewed Dec 1, 2022

View reviewed changes

sycl/plugins/level_zero/pi_level_zero.cpp Show resolved Hide resolved

smaslov-intel reviewed Dec 1, 2022

View reviewed changes

sycl/plugins/opencl/pi_opencl.cpp Show resolved Hide resolved

againull added 3 commits December 1, 2022 16:02

Address review comments

b41149c

* Rename PI_EXT_ONEAPI_QUEUE_INFO_STATUS->PI_EXT_ONEAPI_QUEUE_INFO_EMPTY * Document return type in pi.h * Define feature macro

Merge remote-tracking branch 'origin/sycl' into ext_oneapi_empty

04d5549

Add comment in OpenCL plugin

54e624c

smaslov-intel previously approved these changes Dec 2, 2022

View reviewed changes

smaslov-intel reviewed Dec 2, 2022

View reviewed changes

Update PI version

18f40f7

smaslov-intel approved these changes Dec 2, 2022

View reviewed changes

keryell reviewed Dec 2, 2022

View reviewed changes

Address review comments in CUDA/HIP plugins

706b19e

againull requested a review from keryell December 2, 2022 15:48

Merge remote-tracking branch 'origin/sycl' into ext_oneapi_empty

93155e5

Move extension to supported, update the Status

ebf7fe7

againull requested a review from a team as a code owner December 2, 2022 17:40

gmlueck approved these changes Dec 2, 2022

View reviewed changes

Merge remote-tracking branch 'origin/sycl' into ext_oneapi_empty

71ea2ba

keryell approved these changes Dec 2, 2022

View reviewed changes

romanovvlad approved these changes Dec 4, 2022

View reviewed changes

againull added 2 commits December 4, 2022 17:34

Use C++17 CTAD everywhere

bed1f70

Merge remote-tracking branch 'origin/sycl' into ext_oneapi_empty

8be6a14

againull merged commit c493295 into intel:sycl Dec 5, 2022

againull deleted the ext_oneapi_empty branch December 13, 2022 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Implement queue::ext_oneapi_empty() API to get queue status #7583

[SYCL] Implement queue::ext_oneapi_empty() API to get queue status #7583

againull commented Nov 29, 2022 •

edited

Loading

smaslov-intel commented Nov 29, 2022

againull commented Nov 30, 2022 •

edited

Loading

againull commented Dec 1, 2022

smaslov-intel Dec 2, 2022

againull Dec 2, 2022

keryell Dec 2, 2022

keryell Dec 2, 2022

keryell Dec 2, 2022

againull Dec 2, 2022

keryell Dec 2, 2022

keryell Dec 2, 2022

gmlueck commented Dec 2, 2022

gmlueck left a comment

againull commented Dec 2, 2022

keryell left a comment

keryell Dec 2, 2022

againull commented Dec 5, 2022

	std::lock_guard<std::mutex> compute_guard(compute_stream_mutex_);
	std::lock_guard compute_guard(compute_stream_mutex_);

	std::lock_guard<std::mutex> transfer_guard(transfer_stream_mutex_);
	std::lock_guard transfer_guard(transfer_stream_mutex_);

[SYCL] Implement queue::ext_oneapi_empty() API to get queue status #7583

[SYCL] Implement queue::ext_oneapi_empty() API to get queue status #7583

Conversation

againull commented Nov 29, 2022 • edited Loading

smaslov-intel commented Nov 29, 2022

againull commented Nov 30, 2022 • edited Loading

againull commented Dec 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmlueck commented Dec 2, 2022

gmlueck left a comment

Choose a reason for hiding this comment

againull commented Dec 2, 2022

keryell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

againull commented Dec 5, 2022

againull commented Nov 29, 2022 •

edited

Loading

againull commented Nov 30, 2022 •

edited

Loading