[SYCL][Graph] Avoid removing in-order event dependencies across the native recording capture boundary

[mmichel11]

Wait event dependencies across the native graph capture boundary cannot be removed and must be processed by the native backend. Due to transitive queue recording and interoperability with native backend calls (e.g. capturing direct L0 submissions), SYCL alone cannot track if a queue is recording. We can only determine if a queue is truly recording through an adapter call which is too expensive to perform on each command submission and muddles tracing tool output.

We solve this issue when a native recording is in progress while keeping the SYCL optimization for the general case with the following approach:

• Track on the context if any SYCL graph is native recording. Mark any events on any queue owned by the context during this period as PotentiallyNativeRecorded.
• For any PotentiallyNativeRecorded wait event or queue state, disable the in-order queue optimization removing the dependency.
• Acquire the primary queue lock around begin and end capture to ensure the native recording transition and atomic update are not split by a submission to the primary queue from a separate thread.

[Copilot]

Pull request overview

This PR fixes incorrect pruning of redundant in-order event wait-list dependencies when SYCL graphs use native backend recording, by conservatively tracking native recording at the context level and propagating that state onto events so dependencies that may cross the native capture boundary are preserved.

Changes:

• Add a context-level “native recording active” counter and use it to disable same-queue in-order dependency pruning during native recording.
• Tag events created while native recording is active as PotentiallyNativeRecorded and prevent pruning of those dependencies even after recording ends.
• Refactor UR begin/end capture into queue_impl::{beginNativeRecording,endNativeRecording} and add a new end-to-end test validating the wait-list behavior via SYCL_UR_TRACE.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sycl/test-e2e/Graph/RecordReplay/NativeRecording/inorder_event_dependencies.cpp	New E2E test asserting wait-list dependency preservation across native capture boundaries.
sycl/source/detail/scheduler/commands.cpp	Gate redundant in-order dependency pruning on context native-recording activity and event tagging.
sycl/source/detail/queue_impl.hpp	Expose queue-level wrappers for UR native capture begin/end.
sycl/source/detail/queue_impl.cpp	Implement begin/end native capture wrappers and mark scheduler-bypass events as potentially native recorded.
sycl/source/detail/graph/graph_impl.cpp	Use the new queue_impl wrappers for native capture begin/end and handle queue-destruction cleanup.
sycl/source/detail/event_impl.hpp	Add PotentiallyNativeRecorded flag to events to preserve cross-boundary dependencies.
sycl/source/detail/context_impl.hpp	Add atomic native-recording counter and query helpers on context.

[adamfidel]

I think we can replace the acquire/release stuff with memory_order_relaxed since I don't think a stricter ordering buys us anything.

[mmichel11]

The reason I used acquire-release is due to the transition of the counter from 1 -> 0 in end capture. The release update and acquire load guarantee the effects of the end capture are observed by another thread when it sees 0 in the counter. I was thinking of this for the case where a submission to a forked queue is done concurrent to end capture on the primary queue. This is always a user application race though for what is actually captured.

What do you think?

[adamfidel]

In that case, I think keeping acquire-release is fine as is.

[adamfidel]

Is it possible we can get in a weird state where UR_RESULT_SUCCESS is false but for some reason isNativeRecording() is also false, so we will decrement the shared counter and throw. If the queue is still in the list of recording queues when we call the dtor and clearQueues(), we will decrement the context's counter again.

In this scenario, the context's counter might end up being -1 even if we catch all of the exceptions. Not sure if this is caused by having the decrement in two different places.

[mmichel11]

I think we need to be resilient to the case where end capture returns an error but still stops the recording. I do not believe this happens currently, but the error behavior around end capture is changing in L0 graph.

I switched to handle the errors for begin / end capture in the graph and not the queue code. This allows us to make any changes we need to the graph class before throwing. In the case you list, we will remove the queue and then throw to avoid the double decrement.