[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT #1575

smaslov-intel · 2020-04-23T06:04:50Z

Add sycl-ls utility for listing devices discovered/selected by SYCL RT.
Add LIT testing (using sycl-ls) for the effect of SYCL_BE.

Change-Id: I9ce0f40d97aa6008c9b860342a8efd9c6e62fa3c
Signed-off-by: Sergey V Maslov [email protected]

sycl/tools/CMakeLists.txt

sycl/tools/lspi.cpp

sycl/test/plugins/lspi_gpu_cuda.cpp

sycl/tools/lspi.cpp

keryell · 2020-04-24T05:53:07Z

That looks like a good idea but the name lspi seems too short to avoid name conflict compared to the value. Perhaps sycl-lspi or dpc++-lspi or oneApi or... :-)

Ruyk

Few comments. The tool is called lspi but actually uses SYCL interfaces so doesn't need PI, why not sycl-ls ? will PI interfaces be used later?

sycl/tools/lspi.cpp

smaslov-intel · 2020-04-27T23:07:53Z

changed the name to sycl-ls per suggestions

bader

Could you add test case(s) for concise mode as well, please?

sycl/tools/sycl-ls/CMakeLists.txt

bader · 2020-04-28T11:42:30Z

sycl/tools/sycl-ls/sycl-ls.cpp

+// The "sycl-ls" utility lists all platforms/devices discovered by PI similar to
+// how lscl prints this for OpenCL devices.


Suggested change

// The "sycl-ls" utility lists all platforms/devices discovered by PI similar to

// how lscl prints this for OpenCL devices.

// The "sycl-ls" utility lists all platforms/devices discovered by SYCL similar to

// how clinfo prints this for OpenCL devices.

FYI: AFAIK, lscl is Intel internal tool. I suggest mentioning clinfo - a well-known open source analog.

I think you missed PI -> SYCL.

sycl/test/plugins/lspi_gpu_opencl.cpp

sycl/test/plugins/lspi_gpu_cuda.cpp

sycl/tools/sycl-ls/CMakeLists.txt

sycl/CMakeLists.txt

sycl/test/plugins/lspi_gpu_cuda.cpp

sycl/tools/sycl-ls/sycl-ls.cpp

smaslov-intel · 2020-04-30T00:38:17Z

the windows build failed, is this a known issue:

D:\BuildBot\worker_intel\sycl-win-x64-pr\llvm.src\sycl\include\CL/sycl/handler.hpp(136): error C2220: warning treated as error - no 'object' file generated
D:\BuildBot\worker_intel\sycl-win-x64-pr\llvm.src\sycl\include\CL/sycl/handler.hpp(136): warning C4273: 'cl::sycl::detail::getDeviceFromHandler': inconsistent dll linkage
D:\BuildBot\worker_intel\sycl-win-x64-pr\llvm.src\sycl\include\CL/sycl/handler.hpp(108): note: see previous definition of 'getDeviceFromHandler'

bader · 2020-04-30T11:13:44Z

@smaslov-intel, could you remove "executable" flags added by this patch to CMake script files, please?

bader · 2020-04-30T11:26:06Z

the windows build failed, is this a known issue:

D:\BuildBot\worker_intel\sycl-win-x64-pr\llvm.src\sycl\include\CL/sycl/handler.hpp(136): error C2220: warning treated as error - no 'object' file generated
D:\BuildBot\worker_intel\sycl-win-x64-pr\llvm.src\sycl\include\CL/sycl/handler.hpp(136): warning C4273: 'cl::sycl::detail::getDeviceFromHandler': inconsistent dll linkage
D:\BuildBot\worker_intel\sycl-win-x64-pr\llvm.src\sycl\include\CL/sycl/handler.hpp(108): note: see previous definition of 'getDeviceFromHandler'

This looks like an issue introduced by @v-klochkov by bb73d92.
After follow-up change abe533c we have two conflicting declarations of getDeviceFromHandler next to each other: https://github.com/intel/llvm/blob/sycl/sycl/include/CL/sycl/handler.hpp#L108.

Issues like this will be prevented once #1583 is merged.

bader · 2020-04-30T12:16:18Z

Other then two comments above, this LGTM.

sergey-semenov · 2020-04-30T12:29:54Z

As long as the other comments are addressed, this LGTM as well.

smaslov-intel · 2020-04-30T15:51:25Z

Other then two comments above, this LGTM.

I fixed the executable bits, what is the second one?

bader · 2020-04-30T15:57:17Z

Other then two comments above, this LGTM.

I fixed the executable bits, what is the second one?

This PR breaks the build on Windows.

v-klochkov · 2020-04-30T21:40:26Z

LGTM, but it requires a conflict resolution for handler.hpp before it can be merged.
Please re-base the patch.

…YCL RT Change-Id: I9ce0f40d97aa6008c9b860342a8efd9c6e62fa3c Signed-off-by: Sergey V Maslov <[email protected]>

Signed-off-by: Sergey V Maslov <[email protected]>

sycl/include/CL/sycl/handler.hpp

Signed-off-by: Sergey V Maslov <[email protected]>

bader · 2020-05-01T11:10:40Z

getDeviceFromHandler is declared in three files:
https://github.com/intel/llvm/search?q=getDeviceFromHandler&unscoped_q=getDeviceFromHandler

@alexbatashev, how it happened that we have __SYCL_EXPORT only for forward declaration in handler header, but miss this marker in accessor.*pp files where this function is defined?

alexbatashev · 2020-05-01T11:13:35Z

getDeviceFromHandler is declared in three files:
https://github.com/intel/llvm/search?q=getDeviceFromHandler&unscoped_q=getDeviceFromHandler

@alexbatashev, how it happened that we have __SYCL_EXPORT only for forward declaration in handler header, but miss this marker in accessor.*pp files where this function is defined?

Human error + lack of LITs that would indicate this mistake.

bader · 2020-05-01T11:18:35Z

@alexbatashev, would you mind fixing this issue in a separate PR (+ the bug with double declaration fixed by @smaslov-intel in this PR), please?
I think it shouldn't go with this PR.

alexbatashev · 2020-05-01T11:20:23Z

@alexbatashev, would you mind fixing this issue in a separate PR (+ the bug with double declaration fixed by @smaslov-intel in this PR), please?
I think it shouldn't go with this PR.

Sure

…_docs * origin/sycl: (6482 commits) [SYCL][NFC] Clean formatting in Markdown documents (intel#1635) [SYCL][Doc] Remove obsolete parens from README (intel#1637) [SYCL] Fix failing ABI tests when LLVM_LIBDIR_SUFFIX is set (intel#1605) [SYCL] Fix warnings in libdevice (intel#1630) [SYCL][CUDA] Triage and clean LIT (intel#1620) [SYCL][NFC] Fix GCC 8 compilation warnings (intel#1631) [SYCL] Minor fixes in LowerWGScope [SYCL] PI: correct default interoperability plugin selection [SYCL] Add faster reduction implementations using atomic or/and intel::reduce() (intel#1615) [SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT (intel#1575) [SYCL] Fix getDeviceFromHandler declarations (intel#1626) [SPIR-V] Correct/improve declaration of SPIR-V builtins (intel#1519) [SYCL][USM] Improve USM allocator test and fix improper behavior. (intel#1538) [SYCL] Fix failing ABI LITs (intel#1622) [SYCL] Add support for MSVC internal math functions in device library (intel#1441) [SYCL] Add runtime library versioning (intel#1604) [SYCL] Check weak symbols in ABI dumps (intel#1609) [NFC][SYCL] Improve kernel metadata test (intel#1610) Revert "[SYCL] XFAIL LIT test due to duplicate diagnostic" (intel#1460) [SYCL] Move the reduction command group funcs out of handler.hpp (intel#1602) ...

smaslov-intel requested a review from a team as a code owner April 23, 2020 06:04

smaslov-intel requested review from sergey-semenov and bader April 23, 2020 06:04

smaslov-intel force-pushed the lspi branch from c62dd6f to bce9216 Compare April 23, 2020 06:12

bader reviewed Apr 23, 2020

View reviewed changes

sycl/tools/CMakeLists.txt Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sergey-semenov reviewed Apr 23, 2020

View reviewed changes

Ruyk reviewed Apr 27, 2020

View reviewed changes

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

sycl/tools/lspi.cpp Outdated Show resolved Hide resolved

bader reviewed Apr 28, 2020

View reviewed changes

sycl/test/plugins/lspi_gpu_cuda.cpp Outdated Show resolved Hide resolved

vladimirlaz reviewed Apr 28, 2020

View reviewed changes

sycl/tools/sycl-ls/CMakeLists.txt Show resolved Hide resolved

vladimirlaz reviewed Apr 28, 2020

View reviewed changes

sycl/CMakeLists.txt Show resolved Hide resolved

vladimirlaz reviewed Apr 28, 2020

View reviewed changes

sycl/test/plugins/lspi_gpu_cuda.cpp Outdated Show resolved Hide resolved

smaslov-intel force-pushed the lspi branch 2 times, most recently from be12b85 to a4b2814 Compare April 29, 2020 05:43

Ruyk reviewed Apr 29, 2020

View reviewed changes

sycl/tools/sycl-ls/sycl-ls.cpp Outdated Show resolved Hide resolved

bader changed the title ~~[SYCL]: Add lspi utility for listing devices discovered/selected by SYCL RT~~ [SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT Apr 29, 2020

smaslov-intel force-pushed the lspi branch from a4b2814 to 6f1301a Compare April 30, 2020 00:42

bader requested review from vladimirlaz and sergey-semenov April 30, 2020 12:15

vladimirlaz previously approved these changes Apr 30, 2020

View reviewed changes

smaslov-intel dismissed vladimirlaz’s stale review via b37a4cc April 30, 2020 15:50

smaslov-intel added 5 commits April 30, 2020 14:51

[SYCL]: Add lspi utility for listing devices discovered/selected by S…

4a4a5e0

…YCL RT Change-Id: I9ce0f40d97aa6008c9b860342a8efd9c6e62fa3c Signed-off-by: Sergey V Maslov <[email protected]>

[SYCL] address review comments

2452093

Signed-off-by: Sergey V Maslov <[email protected]>

[SYCL] address review comments #2

9b6c8e2

Signed-off-by: Sergey V Maslov <[email protected]>

[SYCL] address review comments intel#3

2e503e9

Signed-off-by: Sergey V Maslov <[email protected]>

[SYCL] address review comments intel#4

9255a4d

Signed-off-by: Sergey V Maslov <[email protected]>

smaslov-intel force-pushed the lspi branch from 55758b0 to b0c42ff Compare April 30, 2020 21:55

v-klochkov previously approved these changes May 1, 2020

View reviewed changes

v-klochkov reviewed May 1, 2020

View reviewed changes

sycl/include/CL/sycl/handler.hpp Outdated Show resolved Hide resolved

smaslov-intel dismissed v-klochkov’s stale review via 5b4d0d1 May 1, 2020 04:10

smaslov-intel force-pushed the lspi branch from b0c42ff to 5b4d0d1 Compare May 1, 2020 04:10

[SYCL] address review comments intel#5

89ad5b0

Signed-off-by: Sergey V Maslov <[email protected]>

smaslov-intel force-pushed the lspi branch from 5b4d0d1 to 89ad5b0 Compare May 1, 2020 04:12

alexbatashev mentioned this pull request May 1, 2020

[SYCL] Fix getDeviceFromHandler declarations #1626

Merged

bader requested review from v-klochkov and vladimirlaz May 1, 2020 17:00

v-klochkov approved these changes May 1, 2020

View reviewed changes

bader merged commit cc0c33b into intel:sycl May 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT #1575

[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT #1575

smaslov-intel commented Apr 23, 2020 •

edited

Loading

keryell commented Apr 24, 2020 •

edited

Loading

Ruyk left a comment

smaslov-intel commented Apr 27, 2020

bader left a comment

bader Apr 28, 2020

smaslov-intel Apr 28, 2020

bader Apr 29, 2020

smaslov-intel commented Apr 30, 2020

bader commented Apr 30, 2020

bader commented Apr 30, 2020

bader commented Apr 30, 2020

sergey-semenov commented Apr 30, 2020

smaslov-intel commented Apr 30, 2020

bader commented Apr 30, 2020

v-klochkov commented Apr 30, 2020

bader commented May 1, 2020

alexbatashev commented May 1, 2020

bader commented May 1, 2020 •

edited

Loading

alexbatashev commented May 1, 2020

		// The "sycl-ls" utility lists all platforms/devices discovered by PI similar to
		// how lscl prints this for OpenCL devices.

[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT #1575

[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT #1575

Conversation

smaslov-intel commented Apr 23, 2020 • edited Loading

keryell commented Apr 24, 2020 • edited Loading

Ruyk left a comment

Choose a reason for hiding this comment

smaslov-intel commented Apr 27, 2020

bader left a comment

Choose a reason for hiding this comment

bader Apr 28, 2020

Choose a reason for hiding this comment

smaslov-intel Apr 28, 2020

Choose a reason for hiding this comment

bader Apr 29, 2020

Choose a reason for hiding this comment

smaslov-intel commented Apr 30, 2020

bader commented Apr 30, 2020

bader commented Apr 30, 2020

bader commented Apr 30, 2020

sergey-semenov commented Apr 30, 2020

smaslov-intel commented Apr 30, 2020

bader commented Apr 30, 2020

v-klochkov commented Apr 30, 2020

bader commented May 1, 2020

alexbatashev commented May 1, 2020

bader commented May 1, 2020 • edited Loading

alexbatashev commented May 1, 2020

smaslov-intel commented Apr 23, 2020 •

edited

Loading

keryell commented Apr 24, 2020 •

edited

Loading

bader commented May 1, 2020 •

edited

Loading