Skip to content

Conversation

@nrspruit
Copy link
Contributor

@nrspruit nrspruit temporarily deployed to WindowsCILock May 24, 2024 21:23 — with GitHub Actions Inactive
@nrspruit nrspruit temporarily deployed to WindowsCILock May 24, 2024 21:47 — with GitHub Actions Inactive
@nrspruit nrspruit force-pushed the fix_multi_device_event_cache branch 2 times, most recently from 21f4237 to b1fa822 Compare May 28, 2024 14:50
@nrspruit nrspruit temporarily deployed to WindowsCILock May 28, 2024 15:09 — with GitHub Actions Inactive
@nrspruit nrspruit temporarily deployed to WindowsCILock May 28, 2024 17:30 — with GitHub Actions Inactive
@nrspruit nrspruit temporarily deployed to WindowsCILock May 28, 2024 18:59 — with GitHub Actions Inactive
@nrspruit
Copy link
Contributor Author

nrspruit commented May 29, 2024

There is something wrong in the group of commits in between my commit and the previous intel/llvm because with just my patch, the failing test passes:

C:\Users\nrspruit\sycl_workspace\llvm\build\bin>root_group.exe

C:\Users\nrspruit\sycl_workspace\llvm\build\bin>echo %errorlevel%
0

C:\Users\nrspruit\sycl_workspace\llvm\build\bin>git diff

C:\Users\nrspruit\sycl_workspace\llvm\build\bin>cd .._deps\unified-runtime-level_zero-src

C:\Users\nrspruit\sycl_workspace\llvm\build\_deps\unified-runtime-level_zero-src>git diff
diff --git a/source/adapters/level_zero/event.cpp b/source/adapters/level_zero/event.cpp
index d9135334..20ca6b0f 100644
--- a/source/adapters/level_zero/event.cpp
+++ b/source/adapters/level_zero/event.cpp
@@ -1500,7 +1500,20 @@ ur_result_t _ur_ze_event_list_t::createAndRetainUrZeEventList(

         std::shared_lock<ur_shared_mutex> Lock(EventList[I]->Mutex);

-        if (Queue && Queue->Device != CurQueueDevice &&
+        ur_device_handle_t QueueRootDevice;
+        ur_device_handle_t CurrentQueueRootDevice;
+        if (Queue) {
+          QueueRootDevice = Queue->Device;
+          CurrentQueueRootDevice = CurQueueDevice;
+          if (Queue->Device->isSubDevice()) {
+            QueueRootDevice = Queue->Device->RootDevice;
+          }
+          if (CurQueueDevice->isSubDevice()) {
+            CurrentQueueRootDevice = CurQueueDevice->RootDevice;
+          }
+        }
+
+        if (Queue && QueueRootDevice != CurrentQueueRootDevice &&
             !EventList[I]->IsMultiDevice) {
           ze_event_handle_t MultiDeviceZeEvent = nullptr;
           ur_event_handle_t MultiDeviceEvent;

C:\Users\nrspruit\sycl_workspace\llvm\build_deps\unified-runtime-level_zero-src>

If I use my change rebased onto latest unified-runtime, the error occurs ie:

C:\Users\nrspruit\sycl_workspace\llvm\build\bin>root_group.exe
Assertion failed: testResults[i], file root_group.cpp, line 112

@nrspruit
Copy link
Contributor Author

After oneapi-src/unified-runtime#1581 has been merged, root_group test will only pass on windows when #13653 is merged. Until then, the root_group e2e test will fail.

@omarahmed1111 omarahmed1111 force-pushed the fix_multi_device_event_cache branch from b1fa822 to c55712b Compare May 29, 2024 12:12
pre-commit PR for oneapi-src/unified-runtime#1667

Signed-off-by: Neil R. Spruit <neil.r.spruit@intel.com>
@omarahmed1111 omarahmed1111 force-pushed the fix_multi_device_event_cache branch from c55712b to 242a37d Compare May 29, 2024 14:19
@omarahmed1111 omarahmed1111 marked this pull request as ready for review May 29, 2024 14:20
@omarahmed1111 omarahmed1111 requested a review from a team as a code owner May 29, 2024 14:20
@kbenzie
Copy link
Contributor

kbenzie commented May 30, 2024

@intel/llvm-gatekeepers please merge

@steffenlarsen steffenlarsen merged commit 8086df5 into intel:sycl May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants