Skip to content

[NFC][SYCL] Minor refactor around KernelPropertiesUnpacker #7125

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 4 commits into from
Oct 21, 2022

Conversation

aelovikov-intel
Copy link
Contributor

No description provided.

@aelovikov-intel aelovikov-intel requested a review from a team as a code owner October 19, 2022 21:37
@aelovikov-intel aelovikov-intel force-pushed the unpack-refactor branch 2 times, most recently from f152969 to d1995b1 Compare October 19, 2022 21:40
@aelovikov-intel aelovikov-intel changed the title [NFC][SYCL] Minor refactor for KernelPropertiesUnpacker [NFC][SYCL] Minor refactor around KernelPropertiesUnpacker Oct 20, 2022
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Looks good! Minor comment.


// Wrappers for kernel_*** functions above with and without support of
// additional kernel_handler argument.
// Wrappers for kernel_*** functions above supporting scenarios with and
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like "above" is a little vague now that it's not in the same class. Maybe it makes more sense to mention that they're in KernelPropertiesUnpackerImpl instead?

@aelovikov-intel
Copy link
Contributor Author

CUDA failure:

Failed Tests (1):
SYCL :: Basic/work_group_size_prop.cpp

is unrelated, addressed in intel/llvm-test-suite#1338 IIUC.

@intel/llvm-gatekeepers , OK to merge?

@pvchupin pvchupin merged commit 20dbc70 into intel:sycl Oct 21, 2022
@aelovikov-intel aelovikov-intel deleted the unpack-refactor branch November 8, 2022 20:44
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.

3 participants