Skip to content

[SYCL][UR][OpenCL] allow passing events from multiple contexts to urEventWait #18711

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.

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

Merged
merged 8 commits into from
Jun 3, 2025

Conversation

igchor
Copy link
Member

@igchor igchor commented May 28, 2025

SYCL allows creating cross-context dependencies. It automatically handles cases where contexts are from different adapters. However, events from a single adapter are always passed directly to urEventWait (even if they are from different contexts). This causes problems with opencl CPU (it returns CL_INVALID_CONTEXT from clWaitForEvents). In general, OpenCL spec does not guarantee that passing events from different contexts works for any function. The same problem exists for all enqueue functions but in practice SYCL does not mix events in the waitList - it will always use urEventWait for synchronization.

@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 18:17 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 18:47 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 18:47 — with GitHub Actions Inactive
@igchor igchor force-pushed the opencl_wait_m_context branch from 11e0f73 to 6c5ee12 Compare May 28, 2025 20:34
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 20:35 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 21:10 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 21:10 — with GitHub Actions Inactive
@igchor igchor force-pushed the opencl_wait_m_context branch from 6c5ee12 to b22212e Compare May 28, 2025 21:19
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 21:19 — with GitHub Actions Inactive
@igchor igchor marked this pull request as ready for review May 28, 2025 21:21
@igchor igchor requested review from a team as code owners May 28, 2025 21:21
@igchor igchor requested a review from slawekptak May 28, 2025 21:21
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 21:39 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 28, 2025 21:39 — with GitHub Actions Inactive
Copy link
Contributor

@slawekptak slawekptak left a comment

Choose a reason for hiding this comment

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

SYCL changes LGTM

@igchor igchor force-pushed the opencl_wait_m_context branch from d6ef618 to f605383 Compare May 29, 2025 16:50
@igchor igchor force-pushed the opencl_wait_m_context branch from f605383 to b399dea Compare May 29, 2025 16:59
@igchor igchor temporarily deployed to WindowsCILock May 29, 2025 17:00 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 29, 2025 17:40 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 29, 2025 17:40 — with GitHub Actions Inactive
@igchor
Copy link
Member Author

igchor commented May 29, 2025

The failure is unrelated - v2 adapter is failing

@igchor igchor temporarily deployed to WindowsCILock May 30, 2025 15:01 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 30, 2025 15:24 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock May 30, 2025 15:24 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock June 2, 2025 17:57 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock June 2, 2025 18:18 — with GitHub Actions Inactive
@igchor igchor temporarily deployed to WindowsCILock June 2, 2025 18:18 — with GitHub Actions Inactive
@igchor
Copy link
Member Author

igchor commented Jun 3, 2025

@intel/llvm-gatekeepers this is ready to be merged

@kbenzie kbenzie merged commit 85a3861 into intel:sycl Jun 3, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants