Skip to content

[SYCL][Graph] Support sycl::queue::ext_oneapi_get_graph with native recording#21869

Open
mmichel11 wants to merge 4 commits intointel:syclfrom
adamfidel:matt/sycl_get_native_graph_pr
Open

[SYCL][Graph] Support sycl::queue::ext_oneapi_get_graph with native recording#21869
mmichel11 wants to merge 4 commits intointel:syclfrom
adamfidel:matt/sycl_get_native_graph_pr

Conversation

@mmichel11
Copy link
Copy Markdown
Contributor

@mmichel11 mmichel11 commented Apr 24, 2026

  • Queries UR to fetch queue associated graph handle from native API in order to support queue transitions from direct backend command submissions (e.g. L0)
  • Introduces a map in context_impl linking UR graph handles to modifiable command graphs. queue::ext_oneapi_get_graph uses this map for lookup after calling urQueueGetGraphExp .
  • Two test cases are added for command buffer and native recording paths. For native recording, minimum driver versions are added to ensure support for underlying L0 API and L0 bugfixes for fork-join.

@mmichel11 mmichel11 force-pushed the matt/sycl_get_native_graph_pr branch from 879cbae to 98d65ba Compare April 24, 2026 21:44
@mmichel11 mmichel11 marked this pull request as ready for review April 24, 2026 22:37
@mmichel11 mmichel11 requested review from a team as code owners April 24, 2026 22:37
Comment thread sycl/source/detail/queue_impl.cpp Outdated
getAdapter().call_nocheck<UrApiKind::urQueueGetGraphExp>(
MQueue, &UrGraphHandle);

if (Result != UR_RESULT_SUCCESS || !UrGraphHandle) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we split the UR_RESULT_SUCCESS branch from the !UrGraphHandle branch? If the UR call fails (e.g. unsupported feature) the user will see an error saying saying "can only be called on recording queues," which might be a bit confusing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I switched the checks. The UR spec returns UR_RESULT_ERROR_INVALID_OPERATION if the queue is not recording (and sets the pointer to null pointer). I switched to check for this first to throw "can only be called on recording queues". If there is any other error code, then it is internal and should throw with errc::runtime instead of invalid.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It ends up being a bit more complicated to return the correct error codes due to backends that don't support graphs and driver versions where native get graph is unsupported but other graphs operations are.

I fixed this by adding a isNativeRecording() query. It lets us know which exception should be returned for drivers where get graph is not supported.

Copy link
Copy Markdown
Member

@adamfidel adamfidel left a comment

Choose a reason for hiding this comment

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

LGTM!

@mmichel11
Copy link
Copy Markdown
Contributor Author

@sergey-semenov @intel/llvm-reviewers-runtime Can you take a look?

@mmichel11
Copy link
Copy Markdown
Contributor Author

@intel/llvm-reviewers-runtime Friendly ping for review. There are some workloads I would like to get this in for.

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.

2 participants