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

[SYCL][Doc] Update KernelProperties extension #5343

Merged
merged 6 commits into from Jan 27, 2022

Conversation

Pennycook
Copy link
Contributor

This commit makes several changes, to align with other extensions and
address feedback and concerns:

  • To align with changes to the base property list extension, the _v
    suffix has been dropped from property values.

  • The "property_list" class is now called "properties". Some text in the
    extension still refers to the concept of a "property list" because this
    is easier to read.

  • Using a member function in conjunction with a tag type prevents kernel
    functors from accidentally implementing the properties interface, and
    prevents naming conflicts with existing variables in user code.

Signed-off-by: John Pennycook john.pennycook@intel.com

This commit makes several changes, to align with other extensions and
address feedback and concerns:

- To align with changes to the base property list extension, the _v
suffix has been dropped from property values.

- The "property_list" class is now called "properties". Some text in the
extension still refers to the concept of a "property list" because this
is easier to read.

- Using a member function in conjunction with a tag type prevents kernel
functors from accidentally implementing the properties interface, and
prevents naming conflicts with existing variables in user code.

Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook Pennycook added the spec extension All issues/PRs related to extensions specifications label Jan 19, 2022
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook
Copy link
Contributor Author

I'll leave this as a draft until #5338 is merged, but I'd appreciate another pair of eyes on the renaming and feedback on the updated mechanism for embedding properties in a kernel.

Renamed to avoid confusion with similarly named feature test macros.

Signed-off-by: John Pennycook <john.pennycook@intel.com>
@Pennycook Pennycook marked this pull request as ready for review January 24, 2022 21:32
@Pennycook Pennycook requested a review from a team as a code owner January 24, 2022 21:32
@bader
Copy link
Contributor

bader commented Jan 27, 2022

@Pennycook, do you want me to merge it or wait for a feedback from @rolandschulz?
The same question for #5353.

@Pennycook
Copy link
Contributor Author

@Pennycook, do you want me to merge it or wait for a feedback from @rolandschulz? The same question for #5353.

Please wait until @steffenlarsen has reviewed #5338 and that's been merged, just in case I need to reflect any changes here.

@steffenlarsen
Copy link
Contributor

Please wait until @steffenlarsen has reviewed #5338 and that's been merged, just in case I need to reflect any changes here.

Sorry! I forgot to hit the approve-button. #5338 should be ready for merge now.

Note that #5338 does not move compile-time property list to the experimental namespace, but there will be another PR shortly after to move it, so the changes here should be correct.

@Pennycook
Copy link
Contributor Author

Thanks, @steffenlarsen. @bader, this can be merged now.

@bader bader merged commit b48be20 into intel:sycl Jan 27, 2022
@Pennycook Pennycook deleted the kernel-properties branch December 13, 2023 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec extension All issues/PRs related to extensions specifications
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants