Skip to content

[SYCL][Graph] Optimize graph enqueue for in-order queues #18792

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 26 commits into from
Jun 18, 2025

Conversation

fabiomestre
Copy link
Contributor

@fabiomestre fabiomestre commented Jun 3, 2025

Optimizes the enqueue() function of sycl graphs to bypass the scheduler whenever possible and avoid creating events when not needed.

  • Refactors the executable graph enqueue() to have different paths depending on workload:
    • The direct path will be used when there are no host-tasks or accessor requirements in the graph and the execution dependencies are considered safe to bypass the scheduler.
    • The scheduler path will be used when there are requirements in the graph but no host-tasks or, if the execution dependencies require using the scheduler.
    • The multiple partitions path will be used when the graph contains host-tasks which requires scheduling multiple graph partitions. The implementation was also changed to avoid adding unnecessary event dependencies to partition executions and avoiding copying CGData when possible.
  • Extends the changes in [SYCL] Do not store last event for in-order queues #18277 to sycl graphs. This means that no implicit events will be created when using in-order queues and graphs without host-tasks. Also updates the handler to only request events from the graph enqueue() when they are needed.

@fabiomestre fabiomestre force-pushed the fabio/eventless_graph_enqueue branch 2 times, most recently from 4c130dc to ad6ed0e Compare June 3, 2025 16:23
@fabiomestre fabiomestre force-pushed the fabio/eventless_graph_enqueue branch from ad6ed0e to e8ee455 Compare June 3, 2025 16:45
@fabiomestre fabiomestre force-pushed the fabio/eventless_graph_enqueue branch from e8ee455 to 8393ea8 Compare June 3, 2025 16:54
@fabiomestre fabiomestre force-pushed the fabio/eventless_graph_enqueue branch 2 times, most recently from b00774f to 130688f Compare June 5, 2025 13:39
@fabiomestre
Copy link
Contributor Author

fabiomestre commented Jun 17, 2025

@intel/llvm-gatekeepers Can this PR be merged? The existing failures seem to be CI issues. This PR was green for those jobs before:

Intel-Arc jobs:

Cuda UR Job:

There were only unrelated changes to the HIP UR adapter made since then.

Edit: Nevermind, recently merged commits broke this PR, needs to be rebased.

@fabiomestre
Copy link
Contributor Author

@intel/llvm-gatekeepers This PR is ready to merge. The CI failure on PVC is unrelated (I have seen it in other PR's).

@uditagarwal97
Copy link
Contributor

@intel/llvm-gatekeepers This PR is ready to merge. The CI failure on PVC is unrelated (I have seen it in other PR's).

Failing tests are unrelated and tracked by #18932

@uditagarwal97 uditagarwal97 merged commit b643b8b into intel:sycl Jun 18, 2025
48 of 51 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.

9 participants