[SYCL][UR][OpenCL] Fix profiling tag ordering on OpenCL GPU#22641
Open
crystarm wants to merge 1 commit into
Open
[SYCL][UR][OpenCL] Fix profiling tag ordering on OpenCL GPU#22641crystarm wants to merge 1 commit into
crystarm wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect profiling-tag timestamp ordering on OpenCL GPUs by changing the Unified Runtime (UR) OpenCL adapter to record timestamps using a lightweight real device command (buffer fill) instead of synchronization-only commands whose profiling timestamps can be misordered on some drivers. This helps ensure profiling data ordering invariants hold and un-masks previously disabled SYCL ProfilingTag E2E coverage for opencl:gpu.
Changes:
- Implement
urEnqueueTimestampRecordingExpfor OpenCL usingclEnqueueFillBufferon a lazily created per-context internal buffer (requires a profiling-enabled queue; otherwise reports unsupported). - Update SYCL scheduler ProfilingTag enqueue to attempt native timestamp recording via
call_nocheckand fall back to a barrier onUR_RESULT_ERROR_UNSUPPORTED_FEATURE; updatesubmit_profiling_tag()to route non-aspect + profiling-enabled queues through the internal profiling-tag CG path. - Split/adjust unit tests for both “timestamp recording works” and “timestamp recording unsupported → barrier fallback”, and re-enable OpenCL GPU E2E tests by removing the UNSUPPORTED lines.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| unified-runtime/source/adapters/opencl/event.cpp | Implements OpenCL timestamp recording via a small buffer fill and requires queue profiling. |
| unified-runtime/source/adapters/opencl/context.hpp | Adds lazy, thread-safe internal buffer storage to support the OpenCL timestamp emulation. |
| sycl/source/detail/scheduler/commands.cpp | Uses non-throwing UR calls for timestamp recording with an explicit unsupported-feature fallback to a barrier. |
| sycl/include/sycl/ext/oneapi/experimental/profiling_tag.hpp | Submits an internal profiling-tag CG when queue profiling is enabled even without the device aspect. |
| sycl/unittests/Extensions/ProfilingTag.cpp | Expands unit tests to cover native timestamp path vs unsupported-feature barrier fallback. |
| sycl/test-e2e/ProfilingTag/profiling_queue.cpp | Re-enables OpenCL GPU E2E coverage by removing the UNSUPPORTED directive. |
| sycl/test-e2e/ProfilingTag/in_order_profiling_queue.cpp | Re-enables OpenCL GPU E2E coverage by removing the UNSUPPORTED directive. |
Comment on lines
+340
to
+349
| std::vector<cl_event> CLWaitEvents(numEventsInWaitList); | ||
| for (uint32_t I = 0; I < numEventsInWaitList; I++) | ||
| CLWaitEvents[I] = cast(phEventWaitList[I])->CLEvent; | ||
|
|
||
| const cl_uint Pattern = 0; | ||
| cl_event Event = nullptr; | ||
| CL_RETURN_ON_FAILURE(clEnqueueFillBuffer( | ||
| Queue->CLQueue, Buffer, &Pattern, sizeof(Pattern), /*offset=*/0, | ||
| /*size=*/sizeof(Pattern), numEventsInWaitList, CLWaitEvents.data(), | ||
| ifUrEvent(phEvent, Event))); |
Comment on lines
75
to
83
| for (uint32_t i = 0; i < DeviceCount; i++) { | ||
| ur::opencl::urDeviceRelease(cast(Devices[i])); | ||
| } | ||
| if (TimestampRecordingBuffer) { | ||
| clReleaseMemObject(TimestampRecordingBuffer); | ||
| } | ||
| if (IsNativeHandleOwned) { | ||
| clReleaseContext(CLContext); | ||
| } |
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.
Related to #22229.
submit_profiling_tagreturned timestamps in the wrong order onopencl:gpu: the end tag'scommand_endcould be earlier than the completion of work submitted before it, soE2End <= EndTagEndfailed and the e2e tests were masked withUNSUPPORTED: opencl && gpu.Why
OpenCL devices don't advertise
ext_oneapi_queue_profiling_tag(the adapter hard-codesUR_DEVICE_INFO_TIMESTAMP_RECORDING_SUPPORT_EXPtofalse), so the tag fell back toext_oneapi_submit_barrier()and read its profiling info straight off the barrier event.The NEO driver timestamps
clEnqueueBarrierWithWaitList/clEnqueueMarkerWithWaitListwhen the command enters the pipeline, not when the preceding work completes. So the barrier'sCL_PROFILING_COMMAND_ENDcan be earlier than the end of a kernel enqueued before it.I confirmed this with a small pure-OpenCL reproducer on Intel Iris Xe (NEO 26.05.037020): any synchronization-only tag (barrier/marker, empty or explicit wait list) fails 100% of the time, while any real device command (kernel or buffer fill) gives correct timestamps - including a fill into a
dedicated buffer, which is what this PR does.
Changes
unified-runtime/.../opencl/event.cpp: implement urEnqueueTimestampRecordingExpusingclEnqueueFillBufferon a small internal buffer, whose timestamps do reflect completion of prior work. It needs command profiling, so if the queue isn't profiling-enabled we returnUR_RESULT_ERROR_UNSUPPORTED_FEATURE`.unified-runtime/.../opencl/context.hpp: lazily-created, thread-safe 4-byte buffer per context, released with the context. It's never read - only the fill command's timestamps are used.sycl/.../scheduler/commands.cpp(CGType::ProfilingTag): issue the recording withcall_nocheckand fall back to a barrier onUR_RESULT_ERROR_UNSUPPORTED_FEATURE. Also addresses the existingTODOhere.sycl/.../experimental/profiling_tag.hpp: when the device lacks the aspect but the queue has profiling enabled, submit a profiling-tag command group (reusing the scheduler's in-order/out-of-order marker+barrier handling) instead of a bare barrier.The device aspect stays
falsefor OpenCL: the emulation needs queue profiling, but the aspect promises the tag works without it. Enabling the aspect (e.g. via an internal profiling queue) is a possible follow-up.Level Zero / CUDA / HIP are unaffected - they advertise the aspect and already implement the recording, so they never take the fallback branch. The scheduler change only adds the fallback branch and turns a throwing call into a checked one; the success path is unchanged.
Testing
Unit tests (
sycl/unittests/Extensions/ProfilingTag.cpp): split the old fallback test into two - native recording used on a profiling queue without the aspect, and the barrier fallback when the backend reports the recording unsupported.E2E: dropped
UNSUPPORTED: opencl && gpufromProfilingTag/in_order_profiling_queue.cppandProfilingTag profiling_queue.cpp.The reproducer's dedicated-buffer fill (matching this implementation) passes 200/200 iterations on Intel Iris Xe, vs. the old barrier fallback failing 200/200.
The NEO timestamping behavior is arguably a driver bug worth reporting separately; this is a runtime workaround that doesn't touch the public API or other backends.
P.S. I may have gone a little overboard with the code comments. If anyone else feels the same way, I will clean them up.