tests: benchdnn: add experimental support for sycl graph #2295
Conversation
spalicki
force-pushed
the
spalicki/sycl_graph_test
branch
from
a7c4af4 to
541256b
Compare
github-actions
bot
added
the
documentation
| dnnl_status_t status = dnnl_runtime_error; | ||
| bool run_regular_exec = true; | ||
| #if DNNL_GPU_RUNTIME == DNNL_RUNTIME_DPCPP | ||
| while (use_sycl_graph && is_gpu(engine)) { |
There was a problem hiding this comment.
Why is while with a breaks preferred here instead of just ifs or a lambda?
There was a problem hiding this comment.
If the functionality can't be supported, the fallback to a regular execution must happen. Without while it's gonna be a more tangled connection between two or three ifs. Replacing this code with lambda would be identical to this while as we need objects from checking part and executing part, can't break it down into two without passing those objects around which doesn't help with readability. So to day, it's just a style preference.
| DNN_SAFE(dnnl_stream_wait(stream), CRIT); | ||
|
|
||
| auto exec = graph.finalize(); | ||
| queue.ext_oneapi_graph(exec).wait(); |
There was a problem hiding this comment.
Can you give a bit more context on the reasons behind this PR in general? I see it is adding a way to test the SYCL Graph extension with benchdnn. Is the intention to test for correctness and/or measure the impact on performance? Wouldn't it be useful to also be able to re-use the graph n times?
Also the PR says it is an experimental support. Is there a plan for the future of SYCL-Graph in benchdnn?
There was a problem hiding this comment.
The main goal is enabling functional validation of oneDNN primitives in SYCL Graph record/replay mode. While use of SYCL Graph does not require changes in oneDNN API it puts some limitations on what the implementation can do internally.
'Experimental' refers to support status of SYCL Graph extension support in the compiler.
|
General suggestions: for use in CI, it would be nice to be able to apply |
By "per layer" do you mean to run like this: and two problems will be executed, one with sycl-graph, another - not? |
spalicki
force-pushed
the
spalicki/sycl_graph_test
branch
from
541256b to
4f14837
Compare
The function is limited to GPU today due to L0 dependency, but in the end, it should be extended to CPU as well. Maybe then |
@dzarukin My main thought was that it would be nice to add a few targeted matmul test cases for SYCL graph (since there are some special SYCL graph code paths), but we don't need to test the whole CI test suite with SYCL graph. By making a per-layer knob we can add some graph test cases to the regular CI test suites instead of creating new test suites for SYCL graph.
That sounds good to me. |
Adds ability to run benchdnn tests using experimental SyCL graph execution by adding
--use-sycl-graph=true. It is limited to SyCL GPU engine executing on L0 backend.