Skip to content

[SYCL][CUDA] Select only NVPTX64 device binaries #1223

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.

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

Merged
merged 3 commits into from
Mar 3, 2020

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Mar 1, 2020

Change the behaviour of the program manager to always ask the native runtime to choose a device image, even if only one is available; this should prevent selecting an image that is not compatible with the current device.

Fix the behaviour of the CUDA plugin to search through the available binary images and select the first one compatible with a PTX target, or return PI_INVALID_BINARY if there are no compatible images.

Define the PI_DEVICE_BINARY_TARGET_NVPTX64 target identifier as "nvptx64" for NVIDIA PTX devices.

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 1, 2020

Fixes #1194 in my test case.

fwyzard added 2 commits March 1, 2020 12:14
Add the binary target identifier "nvptx64" for NVIDIA PTX devices.

Signed-off-by: Andrea Bocci <[email protected]>
Search through the available binary images and select the first one for
the PI_DEVICE_BINARY_TARGET_NVPTX64 ("nvptx64") target.

Return PI_INVALID_BINARY if no "nvptx64" image is available.

Signed-off-by: Andrea Bocci <[email protected]>
@bader
Copy link
Contributor

bader commented Mar 1, 2020

@fwyzard, would you mind to adding a simple regression test to check that problem #1194 is fixed, please?
It will help us breaking this functionality in the future.

@bader bader requested a review from sergey-semenov March 1, 2020 11:21
@bader bader added the cuda CUDA back-end label Mar 1, 2020
@bader bader linked an issue Mar 1, 2020 that may be closed by this pull request
@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 1, 2020

@fwyzard, would you mind to adding a simple regression test to check that problem #1194 is fixed, please?

I don't mind, but I don't know where to get started.
If you can point me to some existing tests for SYCL or the plugin interface, I can probably take it from there...

@bader
Copy link
Contributor

bader commented Mar 1, 2020

We use LIT infrastructure to check end-to-end behavior and tests are located here: https://github.com/intel/llvm/blob/sycl/sycl/test/. Alternative approach is to use Google Test framework to validate specific parts of the SYCL runtime instead of building a full application.
These tests are located here: https://github.com/intel/llvm/tree/sycl/sycl/unittests

Looking at your changes it seems that CUDA agnostic part might be already covered by @sergey-semenov in 9095749, so need to validate only CUDA plugin changes. It seems to me that Codeplay team is using mostly Google test framework for this: https://github.com/intel/llvm/tree/sycl/sycl/unittests/pi/cuda.
One caveat with CUDA tests is they are all disabled today due to lack of machines in our CI system. We are still working on adding them.
I think it's still useful the add them, but they will be validated post-commit.

@romanovvlad, @Ruyk, does it make sense to you?

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 1, 2020

I'm trying to run the existing tests, but I must be missing something.

make check-sycl seems to get started fine, until it gets to the point

                                                                                                     -- Testing: 219 tests, 16 workers --                                                                                                    
 47% [======================================================================================================------------------------------------------------------------------------------------------------------------------] ETA: 00:00:34

However, most of the tests fail due to

/data/user/fwyzard/sycl/build-sycl-4b5d25b2f97/tools/sycl/test/sub_group/Output/scan.cpp.tmp.out: error while loading shared libraries: libsycl.so: cannot open shared object file: No such file or directory

error: command failed with exit status: 127

Do I need to set up the LD_LIBRARY_PATH ? Should't the test machinery take care of it ?

After setting up the environment, I still get

Expected Passes : 186
Unsupported Tests : 6
Unexpected Failures: 27

Most of the failures look like

$ "env" "SYCL_DEVICE_TYPE=HOST" "/data/user/fwyzard/sycl/build-sycl-4b5d25b2f97/tools/sycl/test/usm/Output/prefetch.cpp.tmp1.out"
note: command had no output on stdout or stderr
error: command failed with exit status: -11

and running them by hand I get

Segmentation fault (core dumped)

Am I missing something in my test setup ?

@romanovvlad
Copy link
Contributor

I'm trying to run the existing tests, but I must be missing something.

make check-sycl seems to get started fine, until it gets to the point

                                                                                                     -- Testing: 219 tests, 16 workers --                                                                                                    
 47% [======================================================================================================------------------------------------------------------------------------------------------------------------------] ETA: 00:00:34

However, most of the tests fail due to

/data/user/fwyzard/sycl/build-sycl-4b5d25b2f97/tools/sycl/test/sub_group/Output/scan.cpp.tmp.out: error while loading shared libraries: libsycl.so: cannot open shared object file: No such file or directory

error: command failed with exit status: 127

Do I need to set up the LD_LIBRARY_PATH ? Should't the test machinery take care of it ?

After setting up the environment, I still get

Expected Passes : 186
Unsupported Tests : 6
Unexpected Failures: 27

Most of the failures look like

$ "env" "SYCL_DEVICE_TYPE=HOST" "/data/user/fwyzard/sycl/build-sycl-4b5d25b2f97/tools/sycl/test/usm/Output/prefetch.cpp.tmp1.out"
note: command had no output on stdout or stderr
error: command failed with exit status: -11

and running them by hand I get

Segmentation fault (core dumped)

Am I missing something in my test setup ?

I'm trying to run the existing tests, but I must be missing something.

make check-sycl seems to get started fine, until it gets to the point

                                                                                                     -- Testing: 219 tests, 16 workers --                                                                                                    
 47% [======================================================================================================------------------------------------------------------------------------------------------------------------------] ETA: 00:00:34

However, most of the tests fail due to

/data/user/fwyzard/sycl/build-sycl-4b5d25b2f97/tools/sycl/test/sub_group/Output/scan.cpp.tmp.out: error while loading shared libraries: libsycl.so: cannot open shared object file: No such file or directory

error: command failed with exit status: 127

Do I need to set up the LD_LIBRARY_PATH ? Should't the test machinery take care of it ?

After setting up the environment, I still get

Expected Passes : 186
Unsupported Tests : 6
Unexpected Failures: 27

Most of the failures look like

$ "env" "SYCL_DEVICE_TYPE=HOST" "/data/user/fwyzard/sycl/build-sycl-4b5d25b2f97/tools/sycl/test/usm/Output/prefetch.cpp.tmp1.out"
note: command had no output on stdout or stderr
error: command failed with exit status: -11

and running them by hand I get

Segmentation fault (core dumped)

Am I missing something in my test setup ?

No, libsycl.so should be automatically added to LD_LIBRARY_PATH by lit.cfg.py.
Could you please create an issue with information of your system and all steps from having empty dir and clear env to seeing fails when running check-sycl?
Backtrace for segfault would be also helpful.

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 2, 2020

OK, will do.

I the meantime I think I've managed to prepare a test for #1194, that shows how it is fixed by these changes:

********************
FAIL: SYCL :: plugins/sycl-targets-order.cpp (135 of 219)
******************** TEST 'SYCL :: plugins/sycl-targets-order.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';    /data/user/fwyzard/sycl/build/bin/clang++ -fsycl -fsycl-targets=spir64-unknown-unknown-sycldevice,nvptx64-unknown-unknown-sycldevice /data/user/fwyzard/sycl/llvm/sycl/test/plugins/sycl-targets-order.cpp -o /data/us
er/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-spir64-nvptx64.out
: 'RUN: at line 2';   env SYCL_BE=PI_OPENCL /data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-spir64-nvptx64.out
: 'RUN: at line 3';   env SYCL_BE=PI_CUDA   /data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-spir64-nvptx64.out
: 'RUN: at line 4';    /data/user/fwyzard/sycl/build/bin/clang++ -fsycl -fsycl-targets=nvptx64-unknown-unknown-sycldevice,spir64-unknown-unknown-sycldevice /data/user/fwyzard/sycl/llvm/sycl/test/plugins/sycl-targets-order.cpp -o /data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-nvptx64-spir64.out
: 'RUN: at line 5';   env SYCL_BE=PI_OPENCL /data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-nvptx64-spir64.out
: 'RUN: at line 6';   env SYCL_BE=PI_CUDA   /data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-nvptx64-spir64.out
--
Exit Code: -6

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "/data/user/fwyzard/sycl/build/bin/clang++" "-fsycl" "-fsycl-targets=spir64-unknown-unknown-sycldevice,nvptx64-unknown-unknown-sycldevice" "/data/user/fwyzard/sycl/llvm/sycl/test/plugins/sycl-targets-order.cpp" "-o" "/data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-spir64-nvptx64.out"
$ ":" "RUN: at line 2" 
$ "env" "SYCL_BE=PI_OPENCL" "/data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-spir64-nvptx64.out"
# command output:
Running on SYCL device Intel(R) Gen9 HD Graphics NEO, driver version 20.07.15711
The results are correct!

$ ":" "RUN: at line 3" 
$ "env" "SYCL_BE=PI_CUDA" "/data/user/fwyzard/sycl/build/tools/sycl/test/plugins/Output/sycl-targets-order.cpp.tmp-spir64-nvptx64.out"
# command output:
Running on SYCL device Tesla K40c, driver version CUDA 10.20
  
# command stderr:
terminate called after throwing an instance of 'cl::sycl::feature_not_supported'
  what():  Online compilation is not supported in this context 0 (CL_SUCCESS)

error: command failed with exit status: -6

With these changes there is no mention of this test in the output of make check-sycl, so I assume it was successful.

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 2, 2020

No, libsycl.so should be automatically added to LD_LIBRARY_PATH by lit.cfg.py.

Ah, I found the issue: sycl/test/CMakeLists.txt uses

set(LLVM_BUILD_LIBRARY_DIRS "${LLVM_BINARY_DIR}/lib/")

instead of

set(LLVM_BUILD_LIBRARY_DIRS "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/")

so the tests will fail if a LLVM_LIBDIR_SUFFIX is specified.

I've opened #1227 for that.

Copy link
Contributor

@Ruyk Ruyk left a comment

Choose a reason for hiding this comment

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

Few minor comments, thanks for the patch

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 2, 2020

2a787290e47 breaks sycl/test/kernel_from_file/hw.cpp, at least on my machine.
As it is not necessary t fix this issue, I would drop that commit, and possibly open an issue to keep track of it ?

…rgets

Add a LIT test to check that both backends (PI_OPENCL, PI_CUDA) work
irrespective of the order of the -fsycl-targets=... arguments.

Signed-off-by: Andrea Bocci <[email protected]>
@fwyzard fwyzard closed this Mar 2, 2020
@fwyzard fwyzard deleted the fix_issue_1194 branch March 2, 2020 19:38
@fwyzard fwyzard restored the fix_issue_1194 branch March 2, 2020 19:46
@fwyzard fwyzard reopened this Mar 2, 2020
@bader bader merged commit bcefeb3 into intel:sycl Mar 3, 2020
@bader
Copy link
Contributor

bader commented Mar 3, 2020

@fwyzard, thanks for working on this!

@fwyzard fwyzard deleted the fix_issue_1194 branch March 3, 2020 12:09
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Mar 4, 2020
…ctor_tests

* origin/sycl: (32 commits)
  [SYCL] Fix circular reference between events and queues (intel#1226)
  [CI][Doc] Use SSH to deploy GitHub Pages (intel#1232)
  [SYCL][CUDA][Test] Testing for use of CUDA primary context (intel#1174)
  [SYCL] allow underscore symbol in temporary directory name
  [SYCL] Reject zero length arrays (intel#1153)
  [SYCL] Fix static code analyzis concerns (intel#1189)
  [SYCL] Add more details about the -fintelfpga option (intel#1218)
  [SYCL][CUDA] Select only NVPTX64 device binaries (intel#1223)
  [SYCL] Reverse max work-group size order (intel#1177)
  [SYCL][Doc] Add GroupAlgorithms extension (intel#1079)
  [SYCL] Fix SYCL internal enumerators conflict with user defined macro (intel#1188)
  [SYCL][CUDA] Fixes context release and unnamed context scope (intel#1207)
  [SYCL][CUDA] Fix context creation property parsing
  [CUDA][PI] clang-format pi.h
  [SYCL][CUDA] Handle the case of not having any CUDA device (intel#1212)
  [SYCL] Fix check-sycl-deploy target problems (intel#1165)
  [SYCL] Disable tests which take more than 5 minutes (intel#1220)
  [SYCL] Make context constructors explicit to avoid unintended conversions (intel#1219)
  [SYCL][NFC] Add clang-format configuration file for SYCL LIT tests (intel#1224)
  [SYCL] Fix command cleanup invoked from multiple threads (intel#1214)
  ...
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Mar 5, 2020
…_accessor_refactor

* origin/sycl: (38 commits)
  [SYCL] Fix device::get_devices() with a non-host device type (intel#1235)
  [SYCL][PI][CUDA] Implement kernel and kernel-group information queries (intel#1180)
  [SYCL] Remove default error code value in exception (intel#1150)
  [SYCL] Fix devicelib assert LIT test (intel#1245)
  [SYCL] Set aux-target-cpu for SYCL offload device compilation (intel#1225)
  [SYCL] Remove fabs and ceil from the list of unsupported math functions (intel#1217)
  [SYCL] Fix circular reference between events and queues (intel#1226)
  [CI][Doc] Use SSH to deploy GitHub Pages (intel#1232)
  [SYCL][CUDA][Test] Testing for use of CUDA primary context (intel#1174)
  [SYCL] allow underscore symbol in temporary directory name
  [SYCL] Reject zero length arrays (intel#1153)
  [SYCL] Fix static code analyzis concerns (intel#1189)
  [SYCL] Add more details about the -fintelfpga option (intel#1218)
  [SYCL][CUDA] Select only NVPTX64 device binaries (intel#1223)
  [SYCL] Reverse max work-group size order (intel#1177)
  [SYCL][Doc] Add GroupAlgorithms extension (intel#1079)
  [SYCL] Fix SYCL internal enumerators conflict with user defined macro (intel#1188)
  [SYCL][CUDA] Fixes context release and unnamed context scope (intel#1207)
  [SYCL][CUDA] Fix context creation property parsing
  [CUDA][PI] clang-format pi.h
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda CUDA back-end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-fsycl-targets is order dependent
5 participants