Skip to content

[SYCL][ABI-Break] Implement property interface for local_accessor & #6737

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Sep 15, 2022
Merged

[SYCL][ABI-Break] Implement property interface for local_accessor & #6737

merged 5 commits into from
Sep 15, 2022

Conversation

KornevNikita
Copy link
Contributor

usm_allocator

@KornevNikita KornevNikita requested a review from a team as a code owner September 8, 2022 16:34
@aelovikov-intel
Copy link
Contributor

Is there a test being added for this?

@KornevNikita
Copy link
Contributor Author

@aelovikov-intel not yet and I'm not sure if we can do this, as these classes don't have any properties yet

@KornevNikita
Copy link
Contributor Author

This is an additional patch for #6614

aelovikov-intel
aelovikov-intel previously approved these changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@aelovikov-intel aelovikov-intel left a comment

Choose a reason for hiding this comment

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

The change looks ok by itself but I lack experience to tell if something is missing or reason about the bigger picture. Please consider asking other people if you want to have a reviewer with deeper knowledge in the area :)

@aelovikov-intel
Copy link
Contributor

Also, can you increment the dev version in sycl/CMakeLists.txt ?

@KornevNikita
Copy link
Contributor Author

@steffenlarsen @AlexeySachkov could you take a look please?

steffenlarsen
steffenlarsen previously approved these changes Sep 8, 2022
View reviewed changes
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.

LGTM!

@KornevNikita
Copy link
Contributor Author

Re-trigger test

@KornevNikita KornevNikita reopened this Sep 9, 2022
@KornevNikita
Copy link
Contributor Author

I believe these fails are unrelated, since it was okay for the first run.

@KornevNikita
Copy link
Contributor Author

Re-trigger tests

@KornevNikita KornevNikita reopened this Sep 12, 2022
@KornevNikita
Copy link
Contributor Author

Re-trigger (sorry for spam, I believe this is the last attempt)

@KornevNikita KornevNikita reopened this Sep 12, 2022
@AlexeySachkov AlexeySachkov merged commit 1136b40 into intel:sycl Sep 15, 2022
@KornevNikita KornevNikita deleted the properties branch September 23, 2022 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steffenlarsen steffenlarsen steffenlarsen approved these changes

@aelovikov-intel aelovikov-intel aelovikov-intel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KornevNikita @aelovikov-intel @steffenlarsen @AlexeySachkov