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

Remove usage of __kernel_name_generator as not required for non-compiled Kernel's #1935

Merged
merged 8 commits into from
Jan 15, 2025

Conversation

SergeyKopienko
Copy link
Contributor

@SergeyKopienko SergeyKopienko commented Nov 14, 2024

In this PR we remove usage of __kernel_name_generator from __parallel_find_or as not required.
We should avoid to use __kernel_name_generator for not-compiler Kernels (when absent the call of __kernel_compiler in the code) due a lot of char codes in template names.
For example we may see these names with char codes from VTune.

Attention :

  • when you will check the differences for this PR, please Hide whitespace.
    Otherwise you will see a lot of unsignificant changes due the formatting has been changed.
    Thanks.

@SergeyKopienko SergeyKopienko requested review from rarutyun and removed request for rarutyun November 14, 2024 13:25
… usage of __kernel_name_generator as not required

Signed-off-by: Sergey Kopienko <sergey.kopienko@intel.com>
@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/fix_kernel_name_generator_usage branch from acc4f20 to c1da480 Compare December 20, 2024 11:24
@SergeyKopienko SergeyKopienko added this to the 2022.8.0 milestone Dec 20, 2024
@SergeyKopienko SergeyKopienko marked this pull request as ready for review December 20, 2024 11:27
@SergeyKopienko SergeyKopienko changed the title Remove usage of __kernel_name_generator as not required Remove usage of __kernel_name_generator as not required for non-compiled Kernel's Dec 20, 2024
danhoeflinger
danhoeflinger previously approved these changes Jan 2, 2025
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable change. I agree that the __kernel_name_generator should not be necessary here because we have no need to pre-compile the kernel and interrogate characteristics of the compiled kernel, and therefore shouldn't be used.

I'm OK with removing the device_backend_tag argument as well, but I want to draw attention to it and make sure we consider that, as it isn't in the original goal of the PR.

It would be good to get another approval here, but I think it looks good.

…view comment: restore __device_backend_tag in __parallel_find_or_impl_one_wg::operator() and __parallel_find_or_impl_multiple_wgs::operator()
@SergeyKopienko
Copy link
Contributor Author

Filed spelling check will fixed in separate PR #1998

danhoeflinger
danhoeflinger previously approved these changes Jan 13, 2025
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

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

LGTM

…ernelName template parameter to last position
…view comment: rename _KernelName to __find_or_kernel_name
…view comment: rename _KernelName to __find_or_one_wg_kernel_name / __find_or_kernel_name
Copy link
Contributor

@rarutyun rarutyun left a comment

Choose a reason for hiding this comment

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

I hope I didn't miss anything, but the PR looks solid to me

@SergeyKopienko SergeyKopienko merged commit ea87681 into main Jan 15, 2025
22 checks passed
@SergeyKopienko SergeyKopienko deleted the dev/skopienko/fix_kernel_name_generator_usage branch January 15, 2025 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants