-
Notifications
You must be signed in to change notification settings - Fork 75
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
Update tests for queue constructors #483
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but we need to fix the compilation with ComputeCPP before the merge.
@ProGTX, ComputeCPP build fails with:
Any fix suggestions? |
issue with compilation model, need to explicitly capture values into the kernel due to different layout between compiler and host |
} | ||
|
||
void check_in_order_functionality(sycl::queue &queue) { | ||
bool *data_changed = sycl::malloc_device<bool>(1, queue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if USM is not supported by the device?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If USM is not supported by the device, this part of test is skipped (checked on line 88) and check_in_order_functionality
is not called. Should we move this check into this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good. Perhaps it could be clearer if the test were here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved SKIP to check_in_order_functionality
for clarity.
docker fix needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
} | ||
|
||
void check_in_order_functionality(sycl::queue &queue) { | ||
bool *data_changed = sycl::malloc_device<bool>(1, queue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good. Perhaps it could be clearer if the test were here.
ComputeCpp Docker image was updated, could you please try running this again? |
@ProGTX It seems that it worked, thank you! |
Add missing tests for queue constructors according to test plan https://github.com/KhronosGroup/SYCL-CTS/blob/SYCL-2020/test_plans/queue_constructors.asciidoc