[UR][L0v2] Implement IPC event sharing#22445
Conversation
againull
changed the title
againull
changed the title
againull
force-pushed
the
ipc_event_l0_adapter_v2
branch
2 times, most recently
from
9327d9d to
61fdeb8
Compare
Adds the producer/consumer side of UR's IPC event extension on top of
Level Zero's counter-based event IPC APIs (zeEventCounterBased{Create,
Get/Open/CloseIpcHandle}).
* urEventCreateExp handles UR_EXP_EVENT_FLAG_IPC_EXP only; the non-IPC
reusable-event path is left to PR intel#22349. IPC + profiling is
rejected.
* IPC events are not pooled. The primary use case is IPC + reusable
events, where the user reuses one event many times, so pool reuse
buys little; and sharing pooled events across processes is unsafe
because a recycled event hands one process state another process
is still relying on. The producer wraps a freshly-created
ze_event_handle_t; the consumer uses a dedicated raii variant whose
destructor calls zeEventCounterBasedCloseIpcHandle, so
urEventRelease tears the imported event down correctly with no
separate close entry point.
* UR_DEVICE_INFO_IPC_EVENT_SUPPORT_EXP is gated on v2/Linux, BMG+,
and latest driver - this might need to be revised eventually.
Conformance tests signal the producer event via Level Zero directly so
they do not depend on the reusable-event extension.
Assisted-By: Claude
againull
force-pushed
the
ipc_event_l0_adapter_v2
branch
from
61fdeb8 to
bda438a
Compare
|
This squashed commits from #22349, making it difficult to review just the IPC related changes. |
|
|
||
| *phEvent = new ur_event_handle_t_( | ||
| hContext, v2::raii::ze_event_handle_t{hZeEvent, /*ownZeHandle=*/true}, | ||
| v2::EVENT_FLAGS_COUNTER | v2::EVENT_FLAGS_IPC); |
There was a problem hiding this comment.
EVENT_FLAGS_IPC needs to be a flag that maps to the corresponding sycl event property:
https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/experimental/sycl_ext_oneapi_inter_process_communication.asciidoc#use-of-enable_ipc-property-for-the-make_event-function
There was a problem hiding this comment.
Exp event creation flags should probably be extended here:
https://github.com/intel/llvm/blob/sycl/unified-runtime/scripts/core/exp-reusable-events.yml#L29-L38
There was a problem hiding this comment.
UR_EXP_EVENT_FLAG_IPC_EXP that we check on line 446 corresponds to the sycl event property.
We define UR_EXP_EVENT_FLAG_IPC_EXP in the corresponding IPC UR yml:
llvm/unified-runtime/scripts/core/exp-inter-process-communication.yml
Lines 49 to 52 in 14c7adc
as an additional value for Exp event creation flags from reusable events yml.
v2::EVENT_FLAGS_IPC is internal flag which is used at event construction and is
carried on the event.: https://github.com/intel/llvm/pull/22445/changes#diff-7cbdb507814de03b8e4a778c3f26932d4de43c2f2717524fc35c216a4342499dL149
The UR-level flag lives in the input desc and isn't retained in the object.
There was a problem hiding this comment.
Right, but you set it unconditionally. Is that intentional? Won't that have perf implications for non-IPC uses of urEventCreateExp?
Nvm, I didn't spot the if at the beginning of this function. I'm not the biggest fan of how this is done -it entirely bypasses our entire event pooling mechanism.
This needs to wait until #22349 is merged that introduces context event pools.
|
|
||
| *phEvent = new ur_event_handle_t_( | ||
| hContext, v2::raii::ze_event_handle_t{hZeEvent, /*ownZeHandle=*/true}, | ||
| v2::EVENT_FLAGS_COUNTER | v2::EVENT_FLAGS_IPC); |
There was a problem hiding this comment.
Exp event creation flags should probably be extended here:
https://github.com/intel/llvm/blob/sycl/unified-runtime/scripts/core/exp-reusable-events.yml#L29-L38
To be honest, I intentionally tried to make this PR independent from #22349. The only overlap with #22349 is a couple of nullptr checks at the top of urEventCreateExp, every other change in this PR is IPC-specific. Nothing else has been squashed in from #22349. This is based the current state of both PRs, if review feedback on either side reshapes urEventCreateExp, the merge strategy may need to be adjusted. |
Assisted-By: Claude
…Zero The IPC event conformance fixture signaled the producer event by driving the Level Zero API directly. Now that urEventCreateExp produces a counter-based reusable event, the fixture can signal it through the reusable-events API instead. Assisted-By: Claude
|
I have adjusted changes on top of the merged reusable events. |
4 participants
Adds the producer/consumer side of UR's IPC event extension on top of Level Zero's counter-based event IPC APIs (zeEventCounterBased{Create,Get/Open/CloseIpcHandle}).
Conformance tests signal the producer event via APIs introduced by the reusable-event extension.
Assisted-By: Claude