Skip to content

Conversation

@yhmtsai
Copy link
Contributor

@yhmtsai yhmtsai commented Aug 1, 2022

Description

From intel/llvm#6407, it moves almost all headers from CL/sycl to sycl
I followed #199 way
make the header can use sycl/* if they exist and allow the old intel llvm.
I also update the CL/sycl.hpp which are not changed before.

All Submissions

  • Do all unit tests pass locally? Attach a log. A: It is a compiling issue.
  • Have you formatted the code using clang-format?

Bug fixes

  • Have you added relevant regression tests? A: It is a compiling issue.
  • Have you included information on how to reproduce the issue (either in a
    GitHub issue or in this PR)?

Reproduce:
compile the latest intel llvm and this repo, it will not be able to compile due to missing headers.

@mmeterel
Copy link
Contributor

mmeterel commented Aug 1, 2022

@yhmtsai Thanks for the PR. Is the description from sycl/CL to CL correct? My understanding is all header files moved from CL/sycl to sycl

@yhmtsai
Copy link
Contributor Author

yhmtsai commented Aug 1, 2022

@mmeterel you are right. I write the wrong folder.
There are some header files still in sycl/CL. I guess some from opencl?
image

@mmeterel
Copy link
Contributor

mmeterel commented Aug 1, 2022

@mmeterel you are right. I write the wrong folder. There are some header files still in sycl/CL. I guess some from opencl? image

I think the description is still wrong. The change was moving header files from CL/sycl to sycl

@mmeterel
Copy link
Contributor

mmeterel commented Aug 1, 2022

Could you please provide the test results with new LLVM compiler? I understand this fixes a compilation issue, but we still want to monitor if there are any functional errors with new compiler version.

@yhmtsai
Copy link
Contributor Author

yhmtsai commented Aug 1, 2022

Compiling all cublas, cusolver, and curand with testing will lead the error

/var/tmp/oneMKL/include/oneapi/mkl/lapack/detail/cusolver/lapack_ct.hxx:2294:14: note: candidate function template not viable: no known conversion from 'backend_selector<oneapi::mkl::backend::cublas aka 2>' to 'backend_selector<backend::cusolver aka 3>' for 1st argument
std::int64_t gebrd_scratchpad_size(backend_selector<backend::cusolver> selector, std::int64_t m

mixed the backend in the tests.
after digging into issue, it #91 or #127 may be related.

oneapi-cuda-blas-test.txt
When only enabling blas, it can be compiled but many testings are failed.
I do not think it is from this pr.
I may need to try cuda 11.6 not cuda 11.4
One more question, does this repo set up any CI with corresponding devices for testing?
I can not set up hipSYCL and mkl recently

@yhmtsai yhmtsai changed the title Fix cuda backend location Fix cuda/hip backend location Aug 5, 2022
@mmeterel
Copy link
Contributor

mmeterel commented Aug 29, 2022

Compiling all cublas, cusolver, and curand with testing will lead the error

/var/tmp/oneMKL/include/oneapi/mkl/lapack/detail/cusolver/lapack_ct.hxx:2294:14: note: candidate function template not viable: no known conversion from 'backend_selector<oneapi::mkl::backend::cublas aka 2>' to 'backend_selector<backend::cusolver aka 3>' for 1st argument
std::int64_t gebrd_scratchpad_size(backend_selector<backend::cusolver> selector, std::int64_t m

mixed the backend in the tests. after digging into issue, it #91 or #127 may be related.

oneapi-cuda-blas-test.txt When only enabling blas, it can be compiled but many testings are failed. I do not think it is from this pr. I may need to try cuda 11.6 not cuda 11.4 One more question, does this repo set up any CI with corresponding devices for testing? I can not set up hipSYCL and mkl recently

@yhmtsai I see the similar errors in my local runs with latest LLVM compiler as well. From the failing tests, it looks like a missing synchronization at first look. Need to look into it deeper.

But I verified that, this PR fixes the build issue. Approved.

@mmeterel
Copy link
Contributor

@mkrainiuk I verified this build fix locally. Could you please take a look?

I opened another issue about the failures seen with latest LLVM compiler. #223

@mmeterel mmeterel requested a review from mkrainiuk August 29, 2022 20:19
Copy link
Contributor

@mkrainiuk mkrainiuk left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Looks good to me.
Thanks @mmeterel for verifying it.

@mkrainiuk mkrainiuk merged commit 50a2387 into uxlfoundation:develop Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants