Skip to content

[SYCL] Clean up excessive kernel name string copying #17340

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

Merged
merged 8 commits into from
Apr 14, 2025

Conversation

sergey-semenov
Copy link
Contributor

@sergey-semenov sergey-semenov commented Mar 6, 2025

On the application side, kernel names can be retrieved as a const char* from the integration header or built-ins. On the library side, they are retrieved from the offload entries. With the recent introduction of the __sycl_unregister_lib implementation, there shouldn't be any need to store copies of those strings anymore.

@sarnex
Copy link
Contributor

sarnex commented Mar 7, 2025

Seems like this is hanging on BMG Windows testing so I cancelled it

@sarnex
Copy link
Contributor

sarnex commented Mar 11, 2025

Seems this is also causing a hang on PVC, please fix it locally before running CI again

@sarnex
Copy link
Contributor

sarnex commented Mar 11, 2025

Also on Linux BMG

@sergey-semenov
Copy link
Contributor Author

Interesting, sorry about that. I've yet to reproduce this locally, will look into it.

Copy link
Contributor

@fabiomestre fabiomestre left a comment

Choose a reason for hiding this comment

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

Sycl Graphs changes: LGTM

Copy link
Contributor

@sommerlukas sommerlukas left a comment

Choose a reason for hiding this comment

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

JIT compiler changes LGTM.

@sergey-semenov
Copy link
Contributor Author

@againull @cperkinsintel Could you please have a look?

@againull againull merged commit 8a5462d into intel:sycl Apr 14, 2025
25 checks passed
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.

7 participants