Skip to content

Commit

Permalink
[SYCL] Remove UR objects release checks on Windows (#15425)
Browse files Browse the repository at this point in the history
These e2e tests started failing after PI removal and replacing it with
UR ([PR](#14145)) However, they were
not related to it.

On Windows, dlls unloading is inconsistent and if we try to release
these UR objects manually, inconsistent hangs happen due to a race
between unloading the UR adapters dlls (in addition to their dependency
dlls) and the releasing of these UR objects (The proxy loader have
solved this problem partially
[here](#15262)). So, we currently
shutdown without releasing them and windows should handle the memory
cleanup.

This behaviour is the same old behaviour as before removing PI as on
Investigations on this. This was only hidden before PI removal as it was
calling the PI entry-point but doesn't make it to UR entry-point and
Filecheck logs check for objects release would pass as it only check for
the call to the PI entry-point without check that the call was a
successful call. That was the PI call for `piContextRelease` before PI
removal:

```
---> piContextRelease(
        <unknown> : 0000023CC0EBA6C0
) ---> API Called After Plugin Teardown, Functon Call ignored.
```

Fixes #14768
Fixes #14950
Fixes #14968
  • Loading branch information
omarahmed1111 authored Oct 7, 2024
1 parent 91eaa05 commit 006dc42
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 31 deletions.
20 changes: 13 additions & 7 deletions sycl/test-e2e/Basic/queue/release.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// RUN: %{build} -o %t.out
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out | FileCheck %s
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out | FileCheck %s %if !windows %{--check-prefixes=CHECK-RELEASE%}
//
// TODO: Reenable on Windows, see https://github.com/intel/llvm/issues/14768
// XFAIL: hip_nvidia, windows
// XFAIL: hip_nvidia

#include <sycl/detail/core.hpp>
int main() {
Expand All @@ -19,7 +18,14 @@ int main() {
// specific queue workaround.
// CHECK-DAG: <--- urEventRelease(
// CHECK-DAG: <--- urQueueRelease(
// CHECK: <--- urContextRelease(
// CHECK: <--- urKernelRelease(
// CHECK: <--- urProgramRelease(
// CHECK: <--- urDeviceRelease(

// On Windows, dlls unloading is inconsistent and if we try to release these UR
// objects manually, inconsistent hangs happen due to a race between unloading
// the UR adapters dlls (in addition to their dependency dlls) and the releasing
// of these UR objects. So, we currently shutdown without releasing them and
// windows should handle the memory cleanup.

// CHECK-RELEASE: <--- urContextRelease(
// CHECK-RELEASE: <--- urKernelRelease(
// CHECK-RELEASE: <--- urProgramRelease(
// CHECK-RELEASE: <--- urDeviceRelease(
23 changes: 13 additions & 10 deletions sycl/test-e2e/KernelAndProgram/disable-caching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
// RUN: env ZE_DEBUG=-6 SYCL_UR_TRACE=2 SYCL_CACHE_IN_MEM=0 %{run} %t.out \
// RUN: | FileCheck %s
// RUN: env ZE_DEBUG=-6 SYCL_UR_TRACE=2 %{run} %t.out \
// RUN: | FileCheck %s --check-prefixes=CHECK-CACHE

// TODO: Reenable on Windows, see https://github.com/intel/llvm/issues/14768
// XFAIL: windows
// RUN: | FileCheck %s --check-prefixes=CHECK-CACHE%if !windows %{,CHECK-RELEASE%}

#include <sycl/detail/core.hpp>

Expand Down Expand Up @@ -93,10 +90,16 @@ int main() {
free(p, q);
}

// On Windows, dlls unloading is inconsistent and if we try to release these UR
// objects manually, inconsistent hangs happen due to a race between unloading
// the UR adapters dlls (in addition to their dependency dlls) and the releasing
// of these UR objects. So, we currently shutdown without releasing them and
// windows should handle the memory cleanup.

// (Program cache releases)
// CHECK-CACHE: <--- urKernelRelease
// CHECK-CACHE: <--- urKernelRelease
// CHECK-CACHE: <--- urKernelRelease
// CHECK-CACHE: <--- urProgramRelease
// CHECK-CACHE: <--- urProgramRelease
// CHECK-CACHE: <--- urProgramRelease
// CHECK-RELEASE: <--- urKernelRelease
// CHECK-RELEASE: <--- urKernelRelease
// CHECK-RELEASE: <--- urKernelRelease
// CHECK-RELEASE: <--- urProgramRelease
// CHECK-RELEASE: <--- urProgramRelease
// CHECK-RELEASE: <--- urProgramRelease
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// REQUIRES: gpu

// RUN: %{build} -o %t.out
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out 2>&1 | FileCheck %s
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out %if !windows %{2>&1 | FileCheck %s %}
//
// TODO: Reenable on Windows, see https://github.com/intel/llvm/issues/14768
// XFAIL: hip_nvidia, windows
// XFAIL: hip_nvidia

#include <sycl/detail/core.hpp>

Expand Down Expand Up @@ -32,4 +31,10 @@ int main() {
return 0;
}

// On Windows, dlls unloading is inconsistent and if we try to release these UR
// objects manually, inconsistent hangs happen due to a race between unloading
// the UR adapters dlls (in addition to their dependency dlls) and the releasing
// of these UR objects. So, we currently shutdown without releasing them and
// windows should handle the memory cleanup.

// CHECK: <--- urContextRelease(
11 changes: 7 additions & 4 deletions sycl/test-e2e/Regression/pi_release.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// REQUIRES: opencl || level_zero || cuda
// RUN: %{build} -o %t.out
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out 2>&1 | FileCheck %s
//
// TODO: Reenable on Windows, see https://github.com/intel/llvm/issues/14768
// XFAIL: windows
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out %if !windows %{2>&1 | FileCheck %s %}

#include <sycl/detail/core.hpp>

Expand All @@ -12,5 +9,11 @@ int main() {
return 0;
}

// On Windows, dlls unloading is inconsistent and if we try to release these UR
// objects manually, inconsistent hangs happen due to a race between unloading
// the UR adapters dlls (in addition to their dependency dlls) and the releasing
// of these UR objects. So, we currently shutdown without releasing them and
// windows should handle the memory cleanup.

// CHECK: <--- urQueueRelease
// CHECK: <--- urContextRelease
20 changes: 13 additions & 7 deletions sycl/test-e2e/Scheduler/ReleaseResourcesTest.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// RUN: %{build} -Wno-error=unused-command-line-argument -fsycl-dead-args-optimization -o %t.out
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out 2>&1 | FileCheck %s
// RUN: env SYCL_UR_TRACE=2 %{run} %t.out 2>&1 | FileCheck %s %if !windows %{--check-prefix=CHECK-RELEASE%}
//
// TODO: Reenable on Windows, see https://github.com/intel/llvm/issues/14768
// XFAIL: hip_nvidia, windows
// XFAIL: hip_nvidia

//==------------------- ReleaseResourcesTests.cpp --------------------------==//
//
Expand Down Expand Up @@ -50,7 +49,14 @@ int main() {
// CHECK: <--- urQueueCreate
// CHECK: <--- urProgramCreate
// CHECK: <--- urKernelCreate
// CHECK: <--- urQueueRelease
// CHECK: <--- urContextRelease
// CHECK: <--- urKernelRelease
// CHECK: <--- urProgramRelease

// On Windows, dlls unloading is inconsistent and if we try to release these UR
// objects manually, inconsistent hangs happen due to a race between unloading
// the UR adapters dlls (in addition to their dependency dlls) and the releasing
// of these UR objects. So, we currently shutdown without releasing them and
// windows should handle the memory cleanup.

// CHECK-RELEASE: <--- urQueueRelease
// CHECK-RELEASE: <--- urContextRelease
// CHECK-RELEASE: <--- urKernelRelease
// CHECK-RELEASE: <--- urProgramRelease

0 comments on commit 006dc42

Please sign in to comment.