oneapi-src
[SYCLomatic] Block Load headers core
In Progress PR for Load/Store header functions for Block API (related later to #1305 ) cc @yihanwg @danhoeflinger @mmichel11
Thinking about the design in more depth, I do not think these APIs should be implemented as free functions. Rather, I think they should be implemented as classes which overload the function call operator or have a member function that invokes the load operation.
My reasoning for this is that certain applications during migration may need to pass a "load" object to a separate function / routine in a kernel. If they are implemented as classes, then an object can be passed to the function where it could then be used:
load_striped<...> obj;
my_func(obj);
However, if they are implemented as free functions, then they would need to be passed as function pointers:
auto func_obj = &load_striped<...>;
my_func(func_obj);
which is not supported in the SYCL programming model. To make this work, load_striped would have to be called directly from my_func which I think may be difficult from the migration perspective.
What I think we should do is have two classes work_group_load and sub_group_load and then have some enum type dictating the strategy (ex: striped, blocked, etc). We could then have have specializations of this class to implement each strategy and maybe have some helpers to reduce code duplication.
Do you have a matching test branch / PR for this that you are working with? If so, please ensure that it has appropriate coverage and post it. It would help to understand how this is meant to be used and to ensure correctness.
Yes it is in PR https://github.com/oneapi-src/SYCLomatic-test/pull/619 . This needs to be updated with the changes in this PR.
Yes made necessary changes, for now if looks good, we can go ahead with this PR , and I will add to #1784 following other apis. PR test https://github.com/oneapi-src/SYCLomatic-test/pull/619 needs some discussion and checks.
This PR looks to be in good shape for me. However, I think the outstanding comments in https://github.com/oneapi-src/SYCLomatic-test/pull/619 need to be addressed, so we can have confidence in the implementation's correctness.
This PR is now tested with https://github.com/oneapi-src/SYCLomatic-test/pull/619 and all tests are passing. Tested on application side as well, working as expected.