[SYCL][Graph] Support sycl::queue::ext_oneapi_get_graph with native recording#21869
[SYCL][Graph] Support sycl::queue::ext_oneapi_get_graph with native recording#21869mmichel11 wants to merge 4 commits intointel:syclfrom
sycl::queue::ext_oneapi_get_graph with native recording#21869Conversation
879cbae to
98d65ba
Compare
| getAdapter().call_nocheck<UrApiKind::urQueueGetGraphExp>( | ||
| MQueue, &UrGraphHandle); | ||
|
|
||
| if (Result != UR_RESULT_SUCCESS || !UrGraphHandle) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
@sergey-semenov @intel/llvm-reviewers-runtime Can you take a look? |
|
@intel/llvm-reviewers-runtime Friendly ping for review. There are some workloads I would like to get this in for. |
queue::ext_oneapi_get_graphuses this map for lookup after callingurQueueGetGraphExp.