Skip to content

[SYCL][Graph] Add E2E test for recording handlerless queue submissions #20420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Oct 23, 2025
Merged

[SYCL][Graph] Add E2E test for recording handlerless queue submissions #20420

merged 8 commits into from
Oct 23, 2025

Conversation

@mmichel11
Copy link
Contributor

  • Follow-up test case to the bug fixed in [SYCL] Update the kernel function pointer on function copy/move #20389 which was not caught by SYCL graph tests.
  • Exercises graph recording over queue submissions with handlerless APIs. Queue submissions are recorded over a non-inlined function call to provide coverage over now resolved kernel argument capture issues.

@mmichel11 mmichel11 requested a review from a team as a code owner October 21, 2025 15:57
@mmichel11 mmichel11 requested a review from reble October 21, 2025 15:57
@mmichel11 mmichel11 requested a review from slawekptak October 21, 2025 15:58
@mmichel11 mmichel11 marked this pull request as draft October 21, 2025 19:33
@mmichel11 mmichel11 marked this pull request as ready for review October 22, 2025 02:24
@mmichel11
Copy link
Contributor Author

Removed the handlerless ops not supported with command buffers in the OpenCL adapter (prefetch, memadvise), so we can still test this configuration.

reble
reble reviewed Oct 22, 2025
View reviewed changes
reble
reble reviewed Oct 23, 2025
View reviewed changes
sycl/test-e2e/Graph/RecordReplay/handlerless_enqueue_functions.cpp Outdated
Comment on lines 40 to 45
auto e1 = Q.memset(A, Pattern, N * sizeof(uint32_t));
auto e2 = Q.fill(D, FillValue, N);
Q.copy(D, E, N, e2);
auto e3 = Q.memcpy(B, A, N * sizeof(uint32_t), e1);
auto e4 = Q.parallel_for(KernelRange, e3, DoubleKernelLambda);
Q.single_task(e4, SingleTaskKernel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we have two independent streams of execution here. Would it make sense to reorder queue submissions to make this more obvious? or is there a specific reason why you picked that order ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no particular reason for the ordering. I reordered the submissions as suggested for clarity.

@mmichel11
Copy link
Contributor Author

@intel/llvm-gatekeepers Please consider merging. The failure is unrelated.

@sarnex sarnex merged commit e9248dd into intel:sycl Oct 23, 2025
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reble reble reble approved these changes

@slawekptak slawekptak slawekptak approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mmichel11 @reble @slawekptak @sarnex