Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

Conversation

@steffenlarsen
Copy link

Removes the assumption that level-zero is available and the default on the system, allowing the default GPU to be from either the OpenCL backend, the CUDA backend, or the L0 backend.

@steffenlarsen
Copy link
Author

The changes clang-format suggests will invalidate the test.

@smaslov-intel
Copy link

I am afraid this test doesn't actually check anything then. What are the SYCL RT changes that change the default? Should we somehow really test that if multiple platforms are discovered then something specific is selected?

@steffenlarsen
Copy link
Author

steffenlarsen commented Apr 7, 2021

On my system sycl-ls reports the following

0. CPU : Intel(R) OpenCL 2.1 [2020.9.1.0.18]
1. GPU : NVIDIA CUDA BACKEND 0.0 [CUDA 11.0]
2. HOST: SYCL host platform 1.2 [1.2]

so the test - as is - fails.

After these changes I suppose the only thing this test checks is that the default is a GPU that belongs to one of the DPC++ backends and not something bogus or unexpected. I agree, it is not a lot, but as is this test depends on it being on a system that has sycl-ls report a L0 GPU.

@smaslov-intel
Copy link

the test - as is - fails.

Understood. Let's then create multiple tests.
One that is checking that Level-Zero is the default when Level-Zero is available. For that you'd decorate it with:

// REQUIRES: level_zero

The other (new one) is doing what you did.

@steffenlarsen
Copy link
Author

Good call. I have done as suggested. Would you like me to rename sycl-ls-gpu-default.cpp to something that makes the distinction more clear? (sycl-ls-gpu-default-level-zero.cpp for example.)

@smaslov-intel
Copy link

Would you like me to rename sycl-ls-gpu-default.cpp to something that makes the distinction more clear? (sycl-ls-gpu-default-level-zero.cpp for example.)

Yes, I think this would be fair.

smaslov-intel
smaslov-intel previously approved these changes Apr 7, 2021
Copy link

@smaslov-intel smaslov-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@vladimirlaz
Copy link

@steffenlarsen could you please fix clang-format issues?

@steffenlarsen
Copy link
Author

@steffenlarsen could you please fix clang-format issues?

I would like to, but I don't think it's possible without invalidating the test. Clang-format wants the check to be in two lines, but from my tests I have concluded that it results in it ignoring the second line.

@vladimirlaz
Copy link

@steffenlarsen could you please fix clang-format issues?

I would like to, but I don't think it's possible without invalidating the test. Clang-format wants the check to be in two lines, but from my tests I have concluded that it results in it ignoring the second line.

You can try approach used in https://github.com/intel/llvm-test-suite/blob/intel/SYCL/Plugin/interop-level-zero.cpp#L8

@steffenlarsen steffenlarsen force-pushed the steffen/fix-sycl-ls-gpu-default branch from 33f3112 to 0a00e56 Compare April 12, 2021 11:19
@vladimirlaz vladimirlaz merged commit 4e86afd into intel:intel Apr 13, 2021
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants