[SYCL][UR] Implement SYCL_LAUNCH_BLOCKING in UR validation layer - #23149
uditagarwal97 wants to merge 5 commits into
Conversation
SYCL_LAUNCH_BLOCKING in UR validation layer
SYCL_LAUNCH_BLOCKING=1 makes the device work of a submission synchronous, so that a device fault is reported at the submission that caused it rather than at the next wait. This is the DPC++ counterpart of CUDA_LAUNCH_BLOCKING, and is a debugging aid only - it serializes the application. The blocking is done below the SYCL runtime, as a mode of the Unified Runtime validation layer named UR_LAYER_LAUNCH_BLOCKING, which the SYCL runtime enables when the environment variable is set. UR_LAYER_FULL_VALIDATION does not enable it: it changes when commands run, which is not something a run asking to be validated should have done to it. Doing it below the SYCL runtime is what keeps the usual host task patterns working: Unified Runtime has no notion of a `handler::host_task`, so one that can only complete through host code running after the submission returns - or a submission made from inside a host task, or a queue shared with a thread blocked in one - is never waited for here. The hook is emitted by valddi.cpp.mako into the generated wrappers, so an entry point added to the spec later is covered. The wait is urQueueFinish: the adapter's own drain, which adds nothing to the queue and handles whatever it batched. It has no deadline, so an application whose enqueued work can only complete through host progress that happens after the submission returns - a kernel spinning on a host-written flag, or a barrier waiting on an interop event signalled later - hangs under this mode where it would otherwise run. That is documented in EnvironmentVariables.md. The only commands left out are the ones that enqueue no work of their own, whose wait list may hold an event the application signals later: urEnqueueEventsWait, urEnqueueEventsWaitWithBarrier(Ext) and urEnqueueTimestampRecordingExp. The work they order is waited for by the next command that does drain the queue. A queue capturing a graph is skipped as well, since it records commands instead of running them; launching a finalized graph blocks like any other submission. Tests: the mode is covered by a UR validation-layer test, which is the only place it can be exercised deterministically - the SYCL runtime initializes the loader once per process, so a SYCL unit test cannot enable a layer per test. End to end, Basic/launch_blocking.cpp checks that each submission path returns a completed event, and the Graph tests check that recording is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e40273d to
a880cea
Compare
Co-authored-by: Udit Kumar Agarwal <udit.agarwal@intel.com>
There was a problem hiding this comment.
🟡 Changes recommended
Queue-finish errors are discarded and external semaphore commands bypass launch blocking.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds SYCL_LAUNCH_BLOCKING=1 by enabling a Unified Runtime validation layer that synchronizes queue submissions.
Changes:
- Adds launch-blocking configuration and generated enqueue interception.
- Skips synchronization during graph capture and selected marker commands.
- Adds UR unit tests, SYCL end-to-end tests, and documentation.
File summaries
| File | Description |
|---|---|
unified-runtime/test/loader/loader_config/urLoaderConfigGetInfo.cpp |
Recognizes the new layer. |
unified-runtime/test/layers/validation/launch_blocking.cpp |
Tests layer behavior. |
unified-runtime/test/layers/validation/CMakeLists.txt |
Registers the test. |
unified-runtime/source/loader/layers/validation/ur_validation_layer.hpp |
Declares layer state and synchronization helper. |
unified-runtime/source/loader/layers/validation/ur_validation_layer.cpp |
Implements queue draining. |
unified-runtime/source/loader/layers/validation/ur_valddi.cpp |
Adds generated blocking hooks. |
unified-runtime/scripts/templates/valddi.cpp.mako |
Generates blocking interception. |
unified-runtime/scripts/core/INTRO.rst |
Documents the UR layer. |
sycl/test-e2e/Graph/RecordReplay/launch_blocking.cpp |
Tests record/replay graphs. |
sycl/test-e2e/Graph/Inputs/launch_blocking.cpp |
Provides shared graph coverage. |
sycl/test-e2e/Graph/Explicit/launch_blocking.cpp |
Tests explicit graphs. |
sycl/test-e2e/Basic/launch_blocking.cpp |
Tests kernels, memory, and host tasks. |
sycl/source/detail/ur.cpp |
Enables the layer from SYCL. |
sycl/source/detail/config.hpp |
Parses the environment setting. |
sycl/source/detail/config.def |
Registers the configuration variable. |
sycl/doc/EnvironmentVariables.md |
Documents the user-facing variable. |
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
iclsrc
left a comment
There was a problem hiding this comment.
Reviewed the SYCL_LAUNCH_BLOCKING implementation across the SYCL runtime config plumbing, the UR validation-layer codegen template, the generated ur_valddi.cpp, and the new e2e/unit tests. The design is sound: blockOnQueue correctly skips draining while a graph is capturing, only fires on UR_RESULT_SUCCESS, is excluded from UR_LAYER_FULL_VALIDATION, and the codegen exclusion list (verified against the generated file) exactly matches the four enqueue functions that legitimately have no blockOnQueue call (EnqueueEventsWait, EnqueueEventsWaitWithBarrier, EnqueueEventsWaitWithBarrierExt, EnqueueTimestampRecordingExp) — so there is no coverage gap. Test coverage is thorough (UR gtest for enable/disable/barrier/capture/failure paths, plus e2e tests across submission fast-paths, memory ops, graphs, and host-task interactions). SYCL_LAUNCH_BLOCKING's getenv usage is null-checked correctly. Two smaller issues below: an inaccurate justification comment in the codegen exclusion list, and a doc formatting change that risks breaking RST section parsing.
Report what the drain finds: blockOnQueue() now returns the urQueueFinish result and the generated wrappers make it the command's result, so a fault it detects is reported at the command that caused it rather than dropped. Covered by a new layer test with a failing drain. Block on urBindlessImagesSignalExternalSemaphoreExp too: it submits work to a queue. WaitExternalSemaphoreExp stays out, waiting for an external signal. State the real reason urEnqueueTimestampRecordingExp is left out - it does enqueue work, but takes its own `blocking` parameter, and blocking it would make SYCL's profiling tag synchronous only on the native path. Make the shared-queue test wait until the host task is actually blocking before submitting, and put the hang caveat back into EnvironmentVariables.md, with a blank line before the following section in both documents.
Fold the repeated "event is complete and the buffer holds the tag" assertions into two helpers, and run both queue kinds through one loop rather than two copies of the same calls. Drop the wait after the fill in the graph test: with only the blocking RUN line left, a memory operation has already completed. The wait after the replay loop stays, and now says why - the host task node splits the graph into pieces submitted around it, and host tasks are not made synchronous. Add a deliberately long kernel: the other event checks pass without the variable on a fast device, where a small kernel can finish before its event is queried, so without this the test only detected a regression by timing.
SYCL_LAUNCH_BLOCKING=1 makes the device work of a submission synchronous, so that a device fault is reported at the submission that caused it rather than at the next wait. This is the DPC++ counterpart ofCUDA_LAUNCH_BLOCKING, and is a debugging aid only - it serializes the application.