[L0] Add support for in-order lists using L0 driver #1372

raiyanla · 2024-02-22T09:59:54Z

Have URT utilize L0 driver native implementation as opposed to L0 URT adapter implementation to ensure in-order semantics when creating inOrderQueues.

Verified working with SYCL e2e in-order queue related tests locally, both with adapter implementation and L0 driver native implementation. One sample that tested it well:

-- Testing: 1 tests, 1 workers --
PASS: SYCL :: Basic/in_order_queue_status.cpp (1 of 1)

Testing Time: 6.93s

Total Discovered Tests: 1
  Passed: 1 (100.00%)

LLVM Draft with all CI / SYCL e2e for this commit passing at: intel/llvm#12833

codecov-commenter · 2024-02-22T18:30:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 12.49%. Comparing base (78ef1ca) to head (11ecfd3).
Report is 126 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1372      +/-   ##
==========================================
- Coverage   14.82%   12.49%   -2.33%     
==========================================
  Files         250      239      -11     
  Lines       36220    35993     -227     
  Branches     4094     4081      -13     
==========================================
- Hits         5369     4498     -871     
- Misses      30800    31491     +691     
+ Partials       51        4      -47

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

raiyanla · 2024-02-27T03:08:59Z

LLVM Draft at intel/llvm#12833

source/adapters/level_zero/kernel.cpp

MichalMrozek · 2024-03-01T06:01:19Z

source/adapters/level_zero/kernel.cpp

@@ -191,9 +191,28 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueKernelLaunch(
  ZE2UR_CALL(zeKernelSetGroupSize, (ZeKernel, WG[0], WG[1], WG[2]));

  bool UseCopyEngine = false;
+
+  bool UseTmpWaitList = false;
+  if (Queue->Device->useDriverInOrderLists() && Queue->isInOrderQueue()) {


consider combining into single helper function

source/adapters/level_zero/kernel.cpp

source/adapters/level_zero/event.cpp

nrspruit

LGTM approved

nrspruit · 2024-03-02T01:01:11Z

@kbenzie , can you take a look to see if there are any issues with this PR that you see?

@raiyanla is checking to see if the intel/llvm failures are accurate or invalid. He will be rebasing the changes in both PRs to see if there are any remaining issues.

raiyanla · 2024-03-02T18:45:51Z

@nrspruit : I've rebased both this PR and the LLVM PR, and both are fully passing all CI tests. We are good to go now.

@kbenzie : This is ready for merging, please proceed with merging process as soon as possible as this one is for a priority deliverable. Thank you.

LLVM PR that references this commit: intel/llvm#12833

pbalcer · 2024-03-05T09:09:23Z

source/adapters/level_zero/queue.cpp

-  // Evaluate performance of explicit usage for "0" index.
-  if (QueueIndex != 0) {
+  if (Queue->Device->useDriverInOrderLists() && Queue->isInOrderQueue()) {
+    ZeCommandQueueDesc.flags = ZE_COMMAND_QUEUE_FLAG_IN_ORDER;


just out of curiosity: is the flag ZE_COMMAND_QUEUE_FLAG_IN_ORDER mutually exclusive with ZE_COMMAND_QUEUE_FLAG_EXPLICIT_ONLY?

it is not mutually exclusive, ZE_COMMAND_QUEUE_FLAG_EXPLICIT_ONLY doesn't do anything in today's driver

Thanks. I asked because this code seems to suggest that, picking one flag or the other depending on (seemingly independent) conditions.

So, ignoring the fact that ZE_COMMAND_QUEUE_FLAG_EXPLICIT_ONLY is a noop (assuming a future driver might take advantage of the flag to do something), should this code look more like this:

if (QueueIndex != 0) { ZeCommandQueueDesc.flags |= ZE_COMMAND_QUEUE_FLAG_EXPLICIT_ONLY; } if (Queue->Device->useDriverInOrderLists() && Queue->isInOrderQueue()) { ZeCommandQueueDesc.flags |= ZE_COMMAND_QUEUE_FLAG_IN_ORDER; }

?

Good suggestion, thanks @pbalcer. Yes if it's not mutually exclusive, we could set it in the way you suggested. I can make an update for that.

Update: Made the change in PR to set flags in that way.

kbenzie · 2024-03-11T16:31:05Z

Please pull in the main branch to have up to date testing, also update the tag in the intel/llvm PR.

Signed-off-by: Raiyan Latif <raiyan.latif@intel.com>

raiyanla force-pushed the in-order-lists branch 3 times, most recently from 7ecbcf2 to 319ad3f Compare February 22, 2024 18:13

raiyanla force-pushed the in-order-lists branch 2 times, most recently from 9e1175b to 4edad79 Compare February 27, 2024 03:03

raiyanla force-pushed the in-order-lists branch 5 times, most recently from 9b3bf62 to 3cadc0e Compare February 29, 2024 04:08

MichalMrozek reviewed Feb 29, 2024

View reviewed changes

raiyanla force-pushed the in-order-lists branch from 3cadc0e to 7d80559 Compare February 29, 2024 17:31

nrspruit requested review from nrspruit and winstonzhang-intel February 29, 2024 21:39

raiyanla force-pushed the in-order-lists branch from 7d80559 to a321009 Compare March 1, 2024 03:54

MichalMrozek approved these changes Mar 1, 2024

View reviewed changes

MichalMrozek reviewed Mar 1, 2024

View reviewed changes

raiyanla force-pushed the in-order-lists branch 2 times, most recently from 27829e7 to 3bf50fa Compare March 1, 2024 09:45

raiyanla marked this pull request as ready for review March 1, 2024 09:46

raiyanla requested a review from a team as a code owner March 1, 2024 09:46

raiyanla force-pushed the in-order-lists branch 3 times, most recently from 862364b to 8065e36 Compare March 1, 2024 10:27

MichalMrozek approved these changes Mar 1, 2024

View reviewed changes

pbalcer reviewed Mar 1, 2024

View reviewed changes

source/adapters/level_zero/event.cpp Outdated Show resolved Hide resolved

raiyanla force-pushed the in-order-lists branch 2 times, most recently from 1f1f181 to ad529c3 Compare March 1, 2024 14:11

raiyanla force-pushed the in-order-lists branch from ad529c3 to 0748743 Compare March 1, 2024 14:13

nrspruit approved these changes Mar 1, 2024

View reviewed changes

nrspruit added level-zero L0 adapter specific issues ready to merge Added to PR's which are ready to merge and removed ready to merge Added to PR's which are ready to merge labels Mar 1, 2024

raiyanla force-pushed the in-order-lists branch 2 times, most recently from a6269c7 to cb924a7 Compare March 2, 2024 16:46

raiyanla mentioned this pull request Mar 2, 2024

[L0] Support for counter-based events using L0 driver #1370

Merged

winstonzhang-intel approved these changes Mar 4, 2024

View reviewed changes

pbalcer reviewed Mar 5, 2024

View reviewed changes

raiyanla force-pushed the in-order-lists branch 2 times, most recently from 3b020f2 to 9d1372d Compare March 6, 2024 14:51

pbalcer approved these changes Mar 7, 2024

View reviewed changes

kbenzie added the v0.9.x Include in the v0.9.x release label Mar 11, 2024

[L0] Add support for in-order lists using L0 driver

11ecfd3

Signed-off-by: Raiyan Latif <raiyan.latif@intel.com>

kbenzie force-pushed the in-order-lists branch from 9d1372d to 11ecfd3 Compare March 12, 2024 20:24

kbenzie merged commit c49b116 into oneapi-src:main Mar 12, 2024
52 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[L0] Add support for in-order lists using L0 driver #1372

[L0] Add support for in-order lists using L0 driver #1372

raiyanla commented Feb 22, 2024 •

edited

Loading

codecov-commenter commented Feb 22, 2024 •

edited

Loading

raiyanla commented Feb 27, 2024 •

edited

Loading

MichalMrozek Mar 1, 2024

nrspruit left a comment

nrspruit commented Mar 2, 2024

raiyanla commented Mar 2, 2024

pbalcer Mar 5, 2024

MichalMrozek Mar 5, 2024

pbalcer Mar 5, 2024 •

edited

Loading

raiyanla Mar 5, 2024 •

edited

Loading

kbenzie commented Mar 11, 2024

[L0] Add support for in-order lists using L0 driver #1372

[L0] Add support for in-order lists using L0 driver #1372

Conversation

raiyanla commented Feb 22, 2024 • edited Loading

codecov-commenter commented Feb 22, 2024 • edited Loading

Codecov Report

raiyanla commented Feb 27, 2024 • edited Loading

MichalMrozek Mar 1, 2024

Choose a reason for hiding this comment

nrspruit left a comment

Choose a reason for hiding this comment

nrspruit commented Mar 2, 2024

raiyanla commented Mar 2, 2024

pbalcer Mar 5, 2024

Choose a reason for hiding this comment

MichalMrozek Mar 5, 2024

Choose a reason for hiding this comment

pbalcer Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

raiyanla Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

kbenzie commented Mar 11, 2024

raiyanla commented Feb 22, 2024 •

edited

Loading

codecov-commenter commented Feb 22, 2024 •

edited

Loading

raiyanla commented Feb 27, 2024 •

edited

Loading

pbalcer Mar 5, 2024 •

edited

Loading

raiyanla Mar 5, 2024 •

edited

Loading