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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

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 <[email protected]>
@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
template <typename _ExecutionPolicy, typename _BrickTag, typename __FoundStateType, typename _Predicate,
typename... _Ranges>
__FoundStateType
operator()(_ExecutionPolicy&& __exec, _BrickTag __brick_tag, const std::size_t __rng_n,
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that we are removing the device backend tag here and below as a side effect of this change. I understand that it is not technically necessary as device backend is the only option at this point.
However, we decided to include it originally. Is it useful as a form of documentation still (this is my only guess as to why it was originally included)?

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.

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.

2 participants