[SYClomatic-test] Add help function for Block Load header#619
[SYClomatic-test] Add help function for Block Load header#619ShengchenJ merged 38 commits intooneapi-src:SYCLomaticfrom
Conversation
danhoeflinger
left a comment
danhoeflinger
left a comment
There was a problem hiding this comment.
Needs some more checks as described. Until this is improved, I don't think we can be confident in the PR.
mmichel11
left a comment
mmichel11
left a comment
There was a problem hiding this comment.
A few comments from my side. The most important I think are testing the remaining free functions and verifying the correctness of the data that has been loaded.
| q.wait_and_throw(); | ||
|
|
||
| sycl::host_accessor data_accessor(buffer, sycl::read_only); | ||
| const int *ptr = data_accessor.get_multi_ptr<sycl::access::decorated::yes>(); |
There was a problem hiding this comment.
Not a big deal from my side, but host_accessor overloads operator[], so we don't really need a pointer.
Co-authored-by: Yihan Wang <yihan.wang@intel.com>
Co-authored-by: Yihan Wang <yihan.wang@intel.com>
Co-authored-by: Yihan Wang <yihan.wang@intel.com>
|
Also requesting review from @ShengchenJ @yihanwg . thanks |
help_function/help_function.xml
Outdated
| <!-- <test testName="onedpl_test_transform" configFile="config/TEMPLATE_help_function_skip_cuda_backend.xml" /> --> | ||
| <test testName="onedpl_test_transform_output_iterator" configFile="config/TEMPLATE_help_function_skip_cuda_backend.xml" /> | ||
| <test testName="onedpl_test_group_sort" configFile="config/TEMPLATE_help_function.xml" /> | ||
| <test testName="util_group_load_test" configFile="config/TEMPLATE_help_function.xml" /> |
There was a problem hiding this comment.
The test name should be onedpl_test_group_load.
| @@ -0,0 +1,213 @@ | |||
| // ====------ util_group_load_test.cpp------------ *- C++ -* ----===// | |||
There was a problem hiding this comment.
Can we keep the filename same as other onedpl test?
There was a problem hiding this comment.
I think the other similar filenames for sort etc should also change when being migrated to the group_util.hpp file.
This was changed to util_group_load_test post @danhoeflinger 's review.
There was a problem hiding this comment.
This shouldn't be done in this PR.
There was a problem hiding this comment.
This is not testing oneDPL or oneDPL-like functionality. It is currently placed in the dpcpp extensions header with the intention to move it to a new header which is targeted at sycl compat.
This code tests a helper function called from within a kernel, which is a lower level than the rest of oneDPL or oneDPL compatibility headers.
If we must have the name as onedpl test until the code is moved, that is fine, but it is just more that needs to be changed later.
There was a problem hiding this comment.
Yes noted, will update the file paths when writing linking tests for oneapi-src/SYCLomatic#1784 .
danhoeflinger
left a comment
danhoeflinger
left a comment
There was a problem hiding this comment.
LGTM, thanks for all the work on this.
|
Thanks @danhoeflinger @mmichel11 @yihanwg for review . |
5 participants
Add block load test Following PR #1640
oneapi-src/SYCLomatic#1640 (Merge after 1640 is merged)
cc @yihanwg