intel / llvm

Intel staging area for llvm.org contribution. Home for Intel LLVM-based projects.
Other
1.25k stars 738 forks source link

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

Open bader opened 1 year ago

bader commented 1 year ago

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.

npmiller commented 1 year ago

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 commented 1 year ago

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

Sounds good to me.

npmiller commented 9 months ago

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

bader commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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.