Skip to content

[SYCL][RTC] Rename property to registered_names #17266

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

Conversation

jopperm
Copy link
Contributor

@jopperm jopperm commented Mar 3, 2025

Renames the registered_kernel_names property to just registered_names. The rationale is to prepare the kernel_compiler extension to support device globals in the future, for which we'll need help from the frontend to resolve source code names to mangled symbol names, in the same way we currently handle kernel names.

Note: This PR only changes the user-facing property, whereas the implementation continues to use the similarly-named but independent __sycl_detail__::__registered_kernels__ attribute, !sycl_registered_kernels named metdata and [SYCL/registered kernels] property set internally.

@jopperm jopperm self-assigned this Mar 3, 2025
@jopperm
Copy link
Contributor Author

jopperm commented Mar 3, 2025

Context: gmlueck#2 (comment)

Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
@jopperm jopperm force-pushed the rtc-rename-registered-kernels-lite branch from 3dc2353 to 0717026 Compare March 3, 2025 10:11
@jopperm jopperm temporarily deployed to WindowsCILock March 3, 2025 10:12 — with GitHub Actions Inactive
@jopperm jopperm marked this pull request as ready for review March 3, 2025 10:12
@jopperm jopperm requested review from a team as code owners March 3, 2025 10:12
@jopperm jopperm requested review from cperkinsintel and a team March 3, 2025 10:12
@jopperm jopperm temporarily deployed to WindowsCILock March 3, 2025 10:40 — with GitHub Actions Inactive
Copy link
Contributor

@Pennycook Pennycook left a comment

Choose a reason for hiding this comment

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

@gmlueck is on sabbatical, but the specification change here seems to be in keeping with comments he's made elsewhere, and looks good to me, so I'll approve.

Comment on lines 503 to 506
This property is useful when the source language represents names differently in
the source code and the generated code.
For example, {cpp} function names in the generated code are "mangled" in an
implementation-defined way.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: It might be useful to give another example here of names that are mangled, to make it clear that this behavior is not unique to functions. Something as simple as "{cpp} function names and the names of static variables at global scope", maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that; added.

Signed-off-by: Julian Oppermann <julian.oppermann@codeplay.com>
@jopperm
Copy link
Contributor Author

jopperm commented Mar 4, 2025

@intel/llvm-gatekeepers Please merge, thanks!

@uditagarwal97 uditagarwal97 merged commit 042b307 into intel:sycl Mar 4, 2025
22 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.

5 participants