Skip to content
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

removed waits around enqueue_primitive in verbose mode #2973

Closed

Conversation

lslusarczyk
Copy link

Description

When running llama.cpp with SYCL Graph feature enabled by GGML_SYCL_GRAPH variable and with verbose logging on oneDNN, the wait call is executed on the sycl stream. This is illegal when recording graph, and wait cannot be called for a queue which is recording to a command graph exception is thrown by this line in llvm compiler.

I'd like to solve this wait problem in order to allow testing llama.cpp with SYCL and with oneDNN verbose logs, as active work on enabling fast graphs in llama.cpp is currently in progress.

What is the best solution

The simplest solution is just remove wait calls, as proposed in this PR, but in this case measurement of kernel execution time will be wrongly calculated.

Other solutions that I see are:

  1. Use "before" and "after" host tasks and log time from the "after" task.
  2. Use "before" and "after" regular tasks and print time directly from kernel code in the "after" task using SYCL stream.
  3. Use SYCL timers with its get_profiling_info function. This measures just kernel execution time, not data transfers and kernel launch time.
  4. Remove time measurements, as this can be observed by calculated by profiling tools

Is there a perfect solution to this problem already designed, or is there any other solution possible, that avoids waits but still computes and logs kernel execution time correctly?

@karturov
Copy link
Contributor

This issue will be resolved under MFDNN-12088 (internal tracker), and a separate PR will be created to address it.

@karturov karturov closed this Mar 28, 2025
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