Updated Driver In order lists check and required version #17620

winstonzhang-intel · 2025-03-24T22:32:13Z

@nrspruit's patch rebased and posted here as I work on fixing this patch.

Refactor's the check for driver in order lists into one place, but still disables driver in order lists by default.

- Cleaned up the checks for driver in order lists and migrated the check to platform. - Updated version needed to match version with fixes. - Enabled default usage only in L0 core adapter, command buffer is optional - Removed driver workaround in p2p Signed-off-by: Neil R. Spruit <[email protected]>

Signed-off-by: Zhang, Winston <[email protected]>

unified-runtime/source/adapters/level_zero/common.hpp

Signed-off-by: Zhang, Winston <[email protected]>

nrspruit

LGTM

unified-runtime/source/adapters/level_zero/command_buffer.cpp

unified-runtime/source/adapters/level_zero/platform.cpp

Signed-off-by: Zhang, Winston <[email protected]>

unified-runtime/source/adapters/level_zero/command_buffer.cpp

Signed-off-by: Zhang, Winston <[email protected]>

nrspruit · 2025-03-27T18:47:51Z

ok, unfortunately, it appears the command graph functionality fails if one runs with the driver in order lists enabled.

unified-runtime/source/adapters/level_zero/platform.cpp

Signed-off-by: Zhang, Winston <[email protected]>

EwanC · 2025-03-28T08:23:44Z

ok, unfortunately, it appears the command graph functionality fails if one runs with the driver in order lists enabled.

@nrspruit I believe your previous patch had a graph test(s) fail too, I'm not sure if these are similar. We could stagger this change, so we enable in-order command lists by default (if driver version met) on the path without command-buffers, and then do a follow-on PR that enabled it for command-buffers and can resolve these graph fails?

nrspruit · 2025-03-28T14:23:15Z

ok, unfortunately, it appears the command graph functionality fails if one runs with the driver in order lists enabled.

@nrspruit I believe your previous patch had a graph test(s) fail too, I'm not sure if these are similar. We could stagger this change, so we enable in-order command lists by default (if driver version met) on the path without command-buffers, and then do a follow-on PR that enabled it for command-buffers and can resolve these graph fails?

That appears to be the case. @winstonzhang-intel , please update this patch to specifically disable the command graphs from using driver in order lists unless explicitly requested, but leave the default for the rest of the adapter. That will let us know if the remaining failures are limited to only the command buffers.

Then, we can work on a PR to re-enable driver in order lists for command buffers once the failures have been addressed.

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel · 2025-03-28T17:50:08Z

ok, unfortunately, it appears the command graph functionality fails if one runs with the driver in order lists enabled.

@nrspruit I believe your previous patch had a graph test(s) fail too, I'm not sure if these are similar. We could stagger this change, so we enable in-order command lists by default (if driver version met) on the path without command-buffers, and then do a follow-on PR that enabled it for command-buffers and can resolve these graph fails?

That appears to be the case. @winstonzhang-intel , please update this patch to specifically disable the command graphs from using driver in order lists unless explicitly requested, but leave the default for the rest of the adapter. That will let us know if the remaining failures are limited to only the command buffers.

Then, we can work on a PR to re-enable driver in order lists for command buffers once the failures have been addressed.

Ok reverted back to when the tests were passing with allowDriverInOrderLists(true) in command buffer.

nrspruit

Because the functionality does not handle all cases of "mixing" driver in order and non-driver in order support, I don't think we can safely split this unless we do a major refactor in the event/queue code paths.

Signed-off-by: Zhang, Winston <[email protected]>

EwanC · 2025-03-28T20:53:49Z

unified-runtime/source/adapters/level_zero/platform.cpp

+  if (OnlyIfRequested) {
+    const char *UrRet = std::getenv("UR_L0_USE_DRIVER_INORDER_LISTS");
+    bool DriverInOrderRequested = UrRet ? std::atoi(UrRet) != 0 : false;
+    return DriverInOrderRequested;
+  }


Nitpick, but I'd move this if to the start of the function before the UseDriverInOrderLists() lamdba. Otherwise when OnlyIfRequested is true we're doing the extra work from the lambda call but it doesn't affect the value returned from allowDriverInOrderLists

yingcong-wu and others added 2 commits March 24, 2025 22:29

[UR][L0] Updated common.hpp params and rebased against sycl

bd2cb28

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel temporarily deployed to WindowsCILock March 24, 2025 22:32 — with GitHub Actions Inactive

winstonzhang-intel temporarily deployed to WindowsCILock March 24, 2025 22:50 — with GitHub Actions Inactive

nrspruit reviewed Mar 25, 2025

View reviewed changes

unified-runtime/source/adapters/level_zero/common.hpp Outdated Show resolved Hide resolved

Rebase correction

b54c2ad

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel temporarily deployed to WindowsCILock March 25, 2025 17:47 — with GitHub Actions Inactive

winstonzhang-intel had a problem deploying to WindowsCILock March 25, 2025 18:35 — with GitHub Actions Error

winstonzhang-intel temporarily deployed to WindowsCILock March 25, 2025 18:35 — with GitHub Actions Inactive

Enable command buffer in order default

998843a

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel temporarily deployed to WindowsCILock March 25, 2025 22:26 — with GitHub Actions Inactive

winstonzhang-intel temporarily deployed to WindowsCILock March 25, 2025 22:45 — with GitHub Actions Inactive

winstonzhang-intel marked this pull request as ready for review March 26, 2025 17:52

winstonzhang-intel requested review from a team as code owners March 26, 2025 17:52

winstonzhang-intel requested review from reble and pbalcer March 26, 2025 17:52

nrspruit approved these changes Mar 26, 2025

View reviewed changes

EwanC reviewed Mar 27, 2025

View reviewed changes

unified-runtime/source/adapters/level_zero/command_buffer.cpp Show resolved Hide resolved

unified-runtime/source/adapters/level_zero/command_buffer.cpp Outdated Show resolved Hide resolved

unified-runtime/source/adapters/level_zero/platform.cpp Show resolved Hide resolved

Corrections on command_buffer and moving definitions

648ef0c

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel had a problem deploying to WindowsCILock March 27, 2025 17:21 — with GitHub Actions Failure

nrspruit reviewed Mar 27, 2025

View reviewed changes

unified-runtime/source/adapters/level_zero/command_buffer.cpp Show resolved Hide resolved

Change default behavior for command buffer

874ceb5

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel temporarily deployed to WindowsCILock March 27, 2025 17:56 — with GitHub Actions Inactive

winstonzhang-intel temporarily deployed to WindowsCILock March 27, 2025 18:16 — with GitHub Actions Inactive

winstonzhang-intel had a problem deploying to WindowsCILock March 27, 2025 18:16 — with GitHub Actions Error

nrspruit reviewed Mar 27, 2025

View reviewed changes

unified-runtime/source/adapters/level_zero/platform.cpp Show resolved Hide resolved

Fixes allowDriverInOrderLists upon request

2933b3d

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel temporarily deployed to WindowsCILock March 27, 2025 19:20 — with GitHub Actions Inactive

winstonzhang-intel temporarily deployed to WindowsCILock March 27, 2025 19:32 — with GitHub Actions Inactive

Graph tests adjustment with allowDriverInOrderLists(true)

4a17e23

Signed-off-by: Zhang, Winston <[email protected]>

winstonzhang-intel temporarily deployed to WindowsCILock March 28, 2025 17:35 — with GitHub Actions Inactive

winstonzhang-intel had a problem deploying to WindowsCILock March 28, 2025 18:31 — with GitHub Actions Error

winstonzhang-intel temporarily deployed to WindowsCILock March 28, 2025 18:31 — with GitHub Actions Inactive

nrspruit requested changes Mar 28, 2025

View reviewed changes

In Order Lists only enabled upon request until in order list is stable.

2e931e7

Signed-off-by: Zhang, Winston <[email protected]>

nrspruit approved these changes Mar 28, 2025

View reviewed changes

winstonzhang-intel temporarily deployed to WindowsCILock March 28, 2025 19:42 — with GitHub Actions Inactive

winstonzhang-intel temporarily deployed to WindowsCILock March 28, 2025 19:54 — with GitHub Actions Inactive

EwanC approved these changes Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated Driver In order lists check and required version #17620

Updated Driver In order lists check and required version #17620

winstonzhang-intel commented Mar 24, 2025 •

edited by nrspruit

Loading

nrspruit left a comment

nrspruit commented Mar 27, 2025

EwanC commented Mar 28, 2025

nrspruit commented Mar 28, 2025

winstonzhang-intel commented Mar 28, 2025

nrspruit left a comment

EwanC Mar 28, 2025

Updated Driver In order lists check and required version #17620

Are you sure you want to change the base?

Updated Driver In order lists check and required version #17620

Conversation

winstonzhang-intel commented Mar 24, 2025 • edited by nrspruit Loading

nrspruit left a comment

Choose a reason for hiding this comment

nrspruit commented Mar 27, 2025

EwanC commented Mar 28, 2025

nrspruit commented Mar 28, 2025

winstonzhang-intel commented Mar 28, 2025

nrspruit left a comment

Choose a reason for hiding this comment

EwanC Mar 28, 2025

Choose a reason for hiding this comment

winstonzhang-intel commented Mar 24, 2025 •

edited by nrspruit

Loading