Add hipSYCL scope_handle and host_task

[sbalint98]

Description

This PR adds hipSYCL support to oneMKL by providing a specific scope_handler. As discussed here hipSYCL has the half datatype in the cl::sycl:: namespace; Therore to compile successfully the following patch needs to be applied: half.txt

Checklist

All Submissions

• Do all unit tests pass locally? Attach a log.
• Have you formatted the code using clang-format?
  hipsycl_cuda_host.log
  dpcpp-hand-test.log

[mmeterel]

What is the purpose of half.txt? That seems like a global search and replace of all "half" texts in the library.

[sbalint98]

Half.txt is only necessary as a temporary solution until half is replaced by cl::sycl::half in the library. This is necessary since in hipSYCL half (according to the SYCL standard) is only defined in the cl::sycl namespace. It is only there so that the library can be built with hipSYCL and the tests can be run with hipSYCL.

EDIT:
The attached patch is mainly a global replacement indeed, but it needed some tweaking therefore I thought it might be useful for anyone who wants to verify this PR.

[mmeterel]

Half.txt is only necessary as a temporary solution until half is replaced by cl::sycl::half in the library. This is necessary since in hipSYCL half (according to the SYCL standard) is only defined in the cl::sycl namespace. It is only there so that the library can be built with hipSYCL and the tests can be run with hipSYCL.

EDIT:
The attached patch is mainly a global replacement indeed, but it needed some tweaking therefore I thought it might be useful for anyone who wants to verify this PR.

So, we need to apply the half.txt patch to be able to test this PR? If yes, I would suggest first submitting the PR needed for updating the half namespace. Once it is reviewed and merged, I suggest submitting the following PRs.
Regarding the half namespace: Instead of changing everything globally in oneMKL, is there any minimal workaround that can be applied to hipSYCL?

[sbalint98]

So, we need to apply the half.txt patch to be able to test this PR?

You should be able to test this PR with dpc++ without applying the half.txt, it should only be necessary for testing with hipSYCL.

If yes, I would suggest first submitting the PR needed for updating the half namespace.

That was the original purpose of the #105 I believe you mentioned in the conversation that you would like to change the namespace yourself. Do you have any information on when this change will approximately get merged?

Regarding the half namespace: Instead of changing everything globally in oneMKL, is there any minimal workaround that can be applied to hipSYCL?

Pinging @illuhad for information on that

[illuhad]

Regarding the half namespace: Instead of changing everything globally in oneMKL, is there any minimal workaround that can be applied to hipSYCL?

Of couse you could just have something like

#ifdef __HIPSYCL__
using ::sycl::half;
#endif

in some commonly used oneMKL header to import half into the global namespace when compiling with hipSYCL. I suspect this is the cleanest solution here to move forward.

I'd prefer not to apply such a workaround to hipSYCL itself, as hipSYCL follows the SYCL standard by putting half into the sycl namespace, and I think that unnecessary pollution of the global namespace should be avoided if possible.

[sbalint98]

Sorry for the long silence. @mmeterel would you prefer the solution proposed by @illuhad or would you rather have a different PR replacing the half with sycl::half

[mmeterel]

Sorry for the long silence. @mmeterel would you prefer the solution proposed by @illuhad or would you rather have a different PR replacing the half with sycl::half

Hi @sbalint98 we also would like to adopt SYCL2020 going forward, however, the current oneMKL version supported this repo is 2021.4 base tool kit, and i am worried sycl::half might not be supported in this version yet.
I would suggest going with the solution proposed by @illuhad right now, and when ready, we can remove that workaround and switch to sycl::half everywhere. What do you think?

[sbalint98]

I agree completely, I have added the change proposed by @illuhad to the types.hpp and to the test_helper.hpp. Now I can compile and run the tests with hipSYCL without applying any patch.

[sbalint98]

Hi @mmeterel , I think this PR is ready, could you have a look?

[mmeterel]

Hi @mmeterel , I think this PR is ready, could you have a look?

Hi @sbalint98 thanks for the PR. I am currently doing some local tests on the PR. Will get back to you soon.

[mmeterel]

@sbalint98 Could you please rebase your branch with latest develop and test with latest llvm compiler? Your branch still has sycl::vector_class calls which are not supported in recent llvm compilers.

[mmeterel]

@sbalint98 Could you please rebase your branch with latest develop and test with latest llvm compiler? Your branch still has sycl::vector_class calls which are not supported in recent llvm compilers.

@sbalint98 Please let me know when you complete this. I would like to do another round of tests after this.

[sbalint98]

It has been rebased to develop already, let me know if something doesn't seem right!

[mmeterel]

It has been rebased to develop already, let me know if something doesn't seem right!

Oh ok, looks like I missed the rebase commit. Thanks.

[mmeterel]

Thanks for the PR and all the updates.

[mkrainiuk]

Looks good to me, thank you!