enable GPU IPC tests on Windows - #739
Merged
Merged
Conversation
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
5 times, most recently
from
September 19, 2024 08:08
6da36e1 to
da553d7
Compare
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
3 times, most recently
from
September 16, 2025 14:49
a77f891 to
78361c2
Compare
bratpiorka
changed the title
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
6 times, most recently
from
September 17, 2025 08:27
976028a to
cfc6f31
Compare
bratpiorka
changed the title
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
from
September 17, 2025 09:51
cfc6f31 to
1140442
Compare
bratpiorka
marked this pull request as ready for review
September 17, 2025 12:03
PatKamin
reviewed
Sep 17, 2025
lukaszstolarczuk
left a comment
lukaszstolarczuk
left a comment
Collaborator
There was a problem hiding this comment.
good job in general, a few minor issues
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
7 times, most recently
from
September 17, 2025 18:36
d0aca41 to
becaa7d
Compare
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
from
October 2, 2025 08:56
304c413 to
4f03627
Compare
bratpiorka
requested review from
KFilipek,
PatKamin,
lukaszstolarczuk and
vinser52
October 2, 2025 09:02
lplewa
requested changes
Oct 9, 2025
lplewa
left a comment
lplewa
left a comment
Contributor
There was a problem hiding this comment.
Please update examples/documentation/readme(?)
| @@ -0,0 +1,38 @@ | |||
| @echo off | |||
Contributor
There was a problem hiding this comment.
Please write this in python so we do not add extra technical dept.
Comment on lines
+86
to
+100
| typedef enum umf_level_zero_memory_provider_memory_exchange_policy_t { | ||
| UMF_LEVEL_ZERO_MEMORY_PROVIDER_MEMORY_EXCHANGE_POLICY_IPC = | ||
| 0, ///< Memory exchange policy based on IPC. Default. | ||
| UMF_LEVEL_ZERO_MEMORY_PROVIDER_MEMORY_EXCHANGE_POLICY_IMPORT_EXPORT, | ||
| ///< Memory exchange policy based on import/export APIs. Should be used if IPC exchange policy is not supported. | ||
| } umf_level_zero_memory_provider_memory_exchange_policy_t; | ||
|
|
||
| /// @brief Set the memory exchange policy. | ||
| /// @param hParams handle to the parameters of the Level Zero Memory Provider. | ||
| /// @param policy memory exchange policy. | ||
| /// @return UMF_RESULT_SUCCESS on success or appropriate error code on failure. | ||
| umf_result_t umfLevelZeroMemoryProviderParamsSetMemoryExchangePolicy( | ||
| umf_level_zero_memory_provider_params_handle_t hParams, | ||
| umf_level_zero_memory_provider_memory_exchange_policy_t policy); | ||
|
|
Contributor
There was a problem hiding this comment.
If this is temporary hack for IPC on windows, i whould like to export this api as CTL, at least for now.
CTL is experimental, so we do not commit to this feature etc....
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
3 times, most recently
from
October 10, 2025 13:03
73e747a to
384c4c4
Compare
bratpiorka
marked this pull request as draft
October 10, 2025 13:04
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
10 times, most recently
from
October 14, 2025 10:01
b12a5d7 to
e3cfa20
Compare
Contributor
Author
bratpiorka
force-pushed
the
rrudnick_ipc_win
branch
from
October 14, 2025 10:07
e3cfa20 to
e248021
Compare
Contributor
Author
|
@lplewa please re-review |
lplewa
requested changes
Oct 15, 2025
lplewa
left a comment
lplewa
left a comment
Contributor
There was a problem hiding this comment.
Documentation and examples are missing
Comment on lines
+88
to
+94
| typedef enum umf_level_zero_memory_provider_memory_exchange_policy_t { | ||
| UMF_LEVEL_ZERO_MEMORY_PROVIDER_MEMORY_EXCHANGE_POLICY_IPC = | ||
| 0, ///< Memory exchange policy based on IPC. Default. | ||
| UMF_LEVEL_ZERO_MEMORY_PROVIDER_MEMORY_EXCHANGE_POLICY_IMPORT_EXPORT, | ||
| ///< Memory exchange policy based on import/export APIs. Should be used if IPC exchange policy is not supported. | ||
| } umf_level_zero_memory_provider_memory_exchange_policy_t; | ||
|
|
Contributor
There was a problem hiding this comment.
I'm not a fan of this enum - can we at least mark it as experimental as it is related to experimental CTL api?
IMHO best if CTL api just work on int - and we do not have this enum at all TBH
| void *ctx, umf_ctl_query_source_t source, void *arg, size_t size, | ||
| umf_ctl_index_utlist_t *indexes) { | ||
| /* suppress unused-parameter errors */ | ||
| (void)source, (void)indexes, (void)size; |
| void *ctx, umf_ctl_query_source_t source, void *arg, size_t size, | ||
| umf_ctl_index_utlist_t *indexes) { | ||
| /* suppress unused-parameter errors */ | ||
| (void)source, (void)indexes, (void)size; |
lplewa
approved these changes
Oct 16, 2025
KFilipek
approved these changes
Oct 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: