[SYCL] Add sycl-ls utility for listing devices discovered/selected by SYCL RT by smaslov-intel · Pull Request #1575 · intel/llvm

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 sergey.v.maslov@intel.com

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

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 <sergey.v.maslov@intel.com>

Signed-off-by: Sergey V Maslov <sergey.v.maslov@intel.com>

sycl/include/CL/sycl/handler.hpp

Signed-off-by: Sergey V Maslov <sergey.v.maslov@intel.com>

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 bader and sergey-semenov 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 Outdated Show resolved Hide resolved

vladimirlaz reviewed Apr 28, 2020

View reviewed changes

sycl/CMakeLists.txt Outdated 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 sergey-semenov and vladimirlaz 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 <sergey.v.maslov@intel.com>

[SYCL] address review comments

2452093

Signed-off-by: Sergey V Maslov <sergey.v.maslov@intel.com>

[SYCL] address review comments #2

9b6c8e2

Signed-off-by: Sergey V Maslov <sergey.v.maslov@intel.com>

[SYCL] address review comments intel#3

2e503e9

Signed-off-by: Sergey V Maslov <sergey.v.maslov@intel.com>

[SYCL] address review comments intel#4

9255a4d

Signed-off-by: Sergey V Maslov <sergey.v.maslov@intel.com>

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 <sergey.v.maslov@intel.com>

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

-// 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.

Conversation

smaslov-intel commented Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keryell commented Apr 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ruyk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smaslov-intel commented Apr 27, 2020

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bader Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

smaslov-intel Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

bader Apr 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smaslov-intel commented Apr 30, 2020

Uh oh!

bader commented Apr 30, 2020

Uh oh!

bader commented Apr 30, 2020

Uh oh!

bader commented Apr 30, 2020

Uh oh!

sergey-semenov commented Apr 30, 2020

Uh oh!

smaslov-intel commented Apr 30, 2020

Uh oh!

bader commented Apr 30, 2020

Uh oh!

v-klochkov commented Apr 30, 2020

Uh oh!

Uh oh!

bader commented May 1, 2020

Uh oh!

alexbatashev commented May 1, 2020

Uh oh!

bader commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexbatashev commented May 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

smaslov-intel commented Apr 23, 2020 •

edited

Loading

keryell commented Apr 24, 2020 •

edited

Loading

bader commented May 1, 2020 •

edited

Loading