enable GPU IPC tests on Windows #739

bratpiorka · 2024-09-18T16:19:23Z

This PR enables GPU IPC (Inter-Process Communication) tests on Windows by implementing Windows-specific socket functionality and memory exchange mechanisms. The changes introduce a new memory exchange policy for Level Zero providers to work around IPC limitations on Windows.

Key changes:

Implements Windows socket support in IPC test infrastructure using Winsock2
Adds new memory exchange policy (import/export) for Level Zero providers on Windows
Creates Windows batch scripts for running GPU IPC tests

test/providers/ipc_cuda_prov.bat

test/common/ipc_common.c

lukaszstolarczuk

good job in general, a few minor issues

test/CMakeLists.txt

test/common/ipc_common.c

KFilipek · 2025-09-23T11:52:23Z

src/provider/provider_level_zero.c

        memcpy(&ze_ipc_handle, &fd_local, sizeof(fd_local));
    }
-
+    


KFilipek · 2025-09-23T12:05:49Z

src/utils/utils_windows_common.c

+    CloseHandle(source_process_handle);
+
+    return result ? UMF_RESULT_SUCCESS : UMF_RESULT_ERROR_UNKNOWN;
 }


It will always return UMF_RESULT_SUCCESS since you check error at 79.
Otherwise:

Suggested change

}

release_current:

CloseHandle(current_process_handle);

release_source:

CloseHandle(source_process_handle);

return result;

}

KFilipek · 2025-09-23T12:06:33Z

src/utils/utils_windows_common.c

+    if (!result) {
+        LOG_ERR("DuplicateHandle() failed for pid=%d fd_in=%d.", pid, fd_in);
+        CloseHandle(current_process_handle);
+        CloseHandle(source_process_handle);
+        return UMF_RESULT_ERROR_UNKNOWN;
+    }


Suggested change

if (!result) {

LOG_ERR("DuplicateHandle() failed for pid=%d fd_in=%d.", pid, fd_in);

CloseHandle(current_process_handle);

CloseHandle(source_process_handle);

return UMF_RESULT_ERROR_UNKNOWN;

}

if (!result) {

LOG_ERR("DuplicateHandle() failed for pid=%d fd_in=%d.", pid, fd_in);

result = UMF_RESULT_ERROR_UNKNOWN;

goto release_current;

}

KFilipek · 2025-09-23T12:08:17Z

src/utils/utils_windows_common.c

+    if (!source_process_handle) {
+        LOG_ERR("OpenProcess() failed for pid=%d.", pid);
+        CloseHandle(current_process_handle);
+        return UMF_RESULT_ERROR_UNKNOWN;
+    }


Suggested change

if (!source_process_handle) {

LOG_ERR("OpenProcess() failed for pid=%d.", pid);

CloseHandle(current_process_handle);

return UMF_RESULT_ERROR_UNKNOWN;

}

BOOL result;

if (!source_process_handle) {

LOG_ERR("OpenProcess() failed for pid=%d.", pid);

result = UMF_RESULT_ERROR_UNKNOWN;

goto release_source;

}

KFilipek · 2025-09-23T12:15:34Z

test/common/ipc_common.c


 int producer_connect(int port) {
    struct sockaddr_in consumer_addr;
+    int ret = -1;


Used only at the end of the function

Copilot

Pull Request Overview

This PR enables GPU IPC (Inter-Process Communication) tests on Windows by implementing Windows-specific socket functionality and memory exchange mechanisms. The changes introduce a new memory exchange policy for Level Zero providers to work around IPC limitations on Windows.

Key changes:

Implements Windows socket support in IPC test infrastructure using Winsock2
Adds new memory exchange policy (import/export) for Level Zero providers on Windows
Creates Windows batch scripts for running GPU IPC tests

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/common/ipc_common.c	Adds Windows socket implementation with Winsock2 support for IPC communication
test/providers/ipc_level_zero_prov_producer.c	Sets import/export memory exchange policy on Windows instead of IPC
test/providers/ipc_level_zero_prov_consumer.c	Sets import/export memory exchange policy on Windows instead of IPC
test/providers/ipc_level_zero_prov.bat	Windows batch script for Level Zero IPC tests
test/providers/ipc_cuda_prov.bat	Windows batch script for CUDA IPC tests
test/CMakeLists.txt	Moves GPU IPC test configuration to support Windows and adds Winsock2 library
src/provider/provider_level_zero.c	Implements new memory exchange policy with import/export APIs
src/utils/utils_windows_common.c	Implements Windows handle duplication functionality
test/providers/provider_level_zero_not_impl.cpp	Adds test for new memory exchange policy API
include/umf/providers/provider_level_zero.h	Defines new memory exchange policy enum and API
src/libumf.def	Exports new memory exchange policy function

test/common/ipc_common.c

Copilot · 2025-10-02T07:51:22Z

src/provider/provider_level_zero.c

+            *(uintptr_t *)lastNext =
+                (uintptr_t)&relaxed_device_allocation_desc_copy;


Using uintptr_t casting to manipulate pNext pointers is unclear and potentially unsafe. Consider using proper pointer casting like *(void **)lastNext = &relaxed_device_allocation_desc_copy; for better type safety and readability.

Copilot · 2025-10-02T07:51:22Z

src/provider/provider_level_zero.c

+        if (ze_provider->exchangePolicy ==
+            UMF_LEVEL_ZERO_MEMORY_PROVIDER_MEMORY_EXCHANGE_POLICY_IMPORT_EXPORT) {
+            // add external memory export desc to the pNext chain
+            *(uintptr_t *)lastNext = (uintptr_t)&memory_export_desc_copy;


Same issue as above - using uintptr_t casting to manipulate pNext pointers is unclear and potentially unsafe. Consider using proper pointer casting like *(void **)lastNext = &memory_export_desc_copy; for better type safety and readability.

Co-authored-by: Copilot <[email protected]>

lukaszstolarczuk

LGTM

bratpiorka force-pushed the rrudnick_ipc_win branch 5 times, most recently from 6da36e1 to da553d7 Compare September 19, 2024 08:08

bratpiorka force-pushed the rrudnick_ipc_win branch 3 times, most recently from a77f891 to 78361c2 Compare September 16, 2025 14:49

bratpiorka changed the title ~~enable IPC tests and examples on Windows~~ enable GPU IPC tests and examples on Windows Sep 16, 2025

bratpiorka force-pushed the rrudnick_ipc_win branch 6 times, most recently from 976028a to cfc6f31 Compare September 17, 2025 08:27

bratpiorka changed the title ~~enable GPU IPC tests and examples on Windows~~ enable GPU IPC tests on Windows Sep 17, 2025

bratpiorka force-pushed the rrudnick_ipc_win branch from cfc6f31 to 1140442 Compare September 17, 2025 09:51

bratpiorka marked this pull request as ready for review September 17, 2025 12:03

bratpiorka requested a review from a team as a code owner September 17, 2025 12:03

bratpiorka requested review from lukaszstolarczuk and ldorau September 17, 2025 12:03

PatKamin reviewed Sep 17, 2025

View reviewed changes

test/providers/ipc_cuda_prov.bat Show resolved Hide resolved

test/common/ipc_common.c Show resolved Hide resolved

lukaszstolarczuk reviewed Sep 17, 2025

View reviewed changes

test/CMakeLists.txt Outdated Show resolved Hide resolved

test/CMakeLists.txt Outdated Show resolved Hide resolved

test/CMakeLists.txt Show resolved Hide resolved

test/common/ipc_common.c Show resolved Hide resolved

bratpiorka force-pushed the rrudnick_ipc_win branch 7 times, most recently from d0aca41 to becaa7d Compare September 17, 2025 18:36

bratpiorka force-pushed the rrudnick_ipc_win branch 9 times, most recently from 1d41c75 to 152c072 Compare September 18, 2025 08:15

bratpiorka marked this pull request as draft September 19, 2025 09:40

KFilipek requested changes Sep 23, 2025

View reviewed changes

bratpiorka force-pushed the rrudnick_ipc_win branch 6 times, most recently from 89ec750 to c423e04 Compare October 2, 2025 07:42

bratpiorka requested a review from Copilot October 2, 2025 07:50

bratpiorka marked this pull request as ready for review October 2, 2025 07:50

Copilot AI reviewed Oct 2, 2025

View reviewed changes

bratpiorka and others added 3 commits October 2, 2025 09:55

Update test/common/ipc_common.c

857658f

Co-authored-by: Copilot <[email protected]>

add support for IPC on L0 + Win

c423e04

Update test/common/ipc_common.c

ce7d075

Co-authored-by: Copilot <[email protected]>

bratpiorka force-pushed the rrudnick_ipc_win branch from 304c413 to 4f03627 Compare October 2, 2025 08:56

add support for IPC on L0 + Win

4f03627

bratpiorka requested review from KFilipek, PatKamin, lukaszstolarczuk and vinser52 October 2, 2025 09:02

lukaszstolarczuk approved these changes Oct 2, 2025

View reviewed changes

-}
+release_current:
+    CloseHandle(current_process_handle);
+release_source:
+    CloseHandle(source_process_handle);
+    return result;
+}

		(uintptr_t )lastNext =
		(uintptr_t)&relaxed_device_allocation_desc_copy;

enable GPU IPC tests on Windows #739

Are you sure you want to change the base?

enable GPU IPC tests on Windows #739

Uh oh!

Conversation

bratpiorka commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

lukaszstolarczuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bratpiorka commented Sep 18, 2024 •

edited

Loading