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

Enable CM kernels for GPU #27857

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

jkasprza
Copy link

@jkasprza jkasprza commented Dec 2, 2024

Details:

  • Add CM implementation type
  • Reuse OCL logic with kernel selector, kernel cache and batch compilation for CM primitive implementations
  • Adjust primitive db and codegen logic to store CM sources separately from OCL C
  • Add example CM print kernel for fully_connected primitive with unit test

Tickets:

@jkasprza jkasprza requested review from a team as code owners December 2, 2024 12:47
@github-actions github-actions bot added category: GPU OpenVINO GPU plugin category: build OpenVINO cmake script / infra labels Dec 2, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalIntelPR External contributor from Intel label Dec 2, 2024
@@ -154,7 +154,11 @@ void kernels_cache::get_program_source(const kernels_code& kernels_source_code,
std::string options = kernel_string->options;
bool batch_compilation = kernel_string->batch_compilation;

if (batch_compilation) {
bool is_cm = options.find("-cmc") != std::string::npos;
Copy link

Choose a reason for hiding this comment

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

Is it the only way to check that CM compiler is invoked?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, OpenCL runtime will use CM compiler only if the -cmc is present in options provided to clBuildProgram. This is the most straightforward way to check this. Alternatively, we can add is_cm flag to kernel_string (src/plugins/intel_gpu/include/intel_gpu/runtime/kernel_args.hpp) and set it for CM primitives during creation.

Copy link

Choose a reason for hiding this comment

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

Usually search in a string being inside a loop is not the most effective way from performance perspective. But overall impact for the app should not be visible I believe. So leaving this for OV guys to suggest.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we may add enum like KernelLanguage {OCLC, CM} and make it a part of KernelCode structure

Copy link
Author

Choose a reason for hiding this comment

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

I added the kernel_language enum to kernel_string in 472a46e. Because KernelCode struct from kernel_selector is not propagated to kernels_cache.

@p-durandin
Copy link
Contributor

build_jenkins

@@ -154,7 +154,11 @@ void kernels_cache::get_program_source(const kernels_code& kernels_source_code,
std::string options = kernel_string->options;
bool batch_compilation = kernel_string->batch_compilation;

if (batch_compilation) {
bool is_cm = options.find("-cmc") != std::string::npos;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we may add enum like KernelLanguage {OCLC, CM} and make it a part of KernelCode structure

@p-durandin
Copy link
Contributor

build_jenkins

@p-durandin
Copy link
Contributor

build_jenkins

@p-durandin
Copy link
Contributor

build_jenkins

Copy link
Contributor

@vladimir-paramuzov vladimir-paramuzov left a comment

Choose a reason for hiding this comment

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

Overall, LGTM

@@ -121,6 +121,47 @@ bool query_local_block_io_supported(engine& e, const ExecutionConfig& config) {

namespace cldnn {

bool check_cm_jit_support(cldnn::engine& e, const cldnn::ExecutionConfig& config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: please align the indentation with other code -- we don't use indentation for the content of namespace

Copy link
Author

Choose a reason for hiding this comment

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

@p-durandin
Copy link
Contributor

build_jenkins

@p-durandin
Copy link
Contributor

build_jenkins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: build OpenVINO cmake script / infra category: GPU OpenVINO GPU plugin ExternalIntelPR External contributor from Intel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants