Skip to content
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.

Sign up for GitHub

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

Jump to bottom

DPC++ runtime plug-in unittests are broken. #10688

Open
bader opened this issue Aug 3, 2023 · 7 comments
Open

DPC++ runtime plug-in unittests are broken. #10688

bader opened this issue Aug 3, 2023 · 7 comments
Assignees
@npmiller @AerialMantis
Labels
bug Something isn't working cuda CUDA back-end hip Issues related to execution on HIP backend.

Comments

@bader
Copy link
Contributor

bader commented Aug 3, 2023 •
edited
Loading

I recently tried to build the sycl branch on the system with ROCM and NVIDIA SDK and found that unittests for cuda and hip plugins do not compile.
Considering that these components has moved to unified runtime, should we remove these tests?

NOTE: These tests are disabled in CI.
@bader bader added bug Something isn't working cuda CUDA back-end hip Issues related to execution on HIP backend. labels Aug 3, 2023
@npmiller
Copy link
Contributor

I'm hoping we can replace a lot of them with the Unified Runtime CTS.

Should we set SYCL_PI_TESTS to OFF by default while we figure out what to do with these?

cc @kbenzie

@bader
Copy link
Contributor Author

bader commented Aug 10, 2023

Should we set SYCL_PI_TESTS to OFF by default while we figure out what to do with these?

Sounds good to me.

@npmiller npmiller mentioned this issue Aug 10, 2023
Merged
bader pushed a commit that referenced this issue Aug 22, 2023
@npmiller
[SYCL] Disable PI unit tests by default (#10781)
cd3c40f 
These tests are currently disabled in the CI and ended up broken.

Since we're also currently moving to the Unified Runtime we might be
able to replace most of them with the Unified Runtime CTS, or they will
also need to be ported to the Unified Runtime.

So disable them by default until we figure out a proper course of action
of these.

Related ticket: #10688
@npmiller
Copy link
Contributor

Started the process of porting some of the tests to UR CTS:

@bader
Copy link
Contributor Author

bader commented Jan 22, 2024

Started the process of porting some of the tests to UR CTS:

@npmiller, that's great! Please, do not forget to remove these tests from intel/llvm repo.

@npmiller
Copy link
Contributor

npmiller commented Jan 22, 2024 •
edited
Loading

Yes!

I'm preparing a patch removing them as I'm going through but there's still a couple I need to port/replace, so it's not ready yet, but I've just put it in a draft PR for now:

@bader
Copy link
Contributor Author

bader commented Jan 22, 2024

Thanks!

I'm preparing a patch removing them as I'm going through but there's still a couple I need to port/replace, so it's not ready yet

To be honest, PI unit tests seem to be a "dead code" in intel/llvm, so I would be fine with removing all of them at once.
We can have an issue to track the migration status and put the permalink to the commit before removal (i.e. to the code state with all the tests).

I'm fine with keeping non-ported tests in intel/llvm too.

@npmiller
Copy link
Contributor

I've just marked the PR as ready for review:

There's still a little bit of work to do on the CUDA and HIP interop testing but I think we'll add that separately, so I think we're ready to delete these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@npmiller npmiller

@AerialMantis AerialMantis

Labels
bug Something isn't working cuda CUDA back-end hip Issues related to execution on HIP backend.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bader @npmiller @AerialMantis