Skip to content

Conversation

bjoernknafla
Copy link
Contributor

Build the get_device_count_by_type tool with the required preprocessor
definitions to enable CUDA, and link with CUDA.

Passed correct backend name to the get_device_count_by_type tool to
is required to run tests with PI OpenCL instead of PI CUDA.

Clean up the get_device_count_by_type tool code to make it harder to
call it with the wrong backend and get unexpected query results back.

Signed-off-by: Bjoern Knafla [email protected]

@bjoernknafla
Copy link
Contributor Author

bjoernknafla commented Mar 12, 2020

WARNING This PR fixes CUDA LIT testing and multiple LIT tests will unexpectedly pass while more will fail. A follow up PR will fix this as I didn't want to overload this PR.

ninja check-sycl-cuda
...
Unexpected Passing Tests (2):
    SYCL :: basic_tests/boolean.cpp
    SYCL :: hier_par/hier_par_basic.cpp
    SYCL :: kernel_from_file/hw.cpp

********************
Failing Tests (7):
    SYCL :: backend/cuda/primary_context.cpp

    SYCL :: basic_tests/handler/handler_copy_with_offset.cpp
    SYCL :: basic_tests/handler/handler_mem_op.cpp

    SYCL :: group-algorithm/all_of.cpp
    SYCL :: group-algorithm/broadcast.cpp
    SYCL :: group-algorithm/any_of.cpp
    SYCL :: group-algorithm/exclusive_scan.cpp
    SYCL :: group-algorithm/leader.cpp
    SYCL :: group-algorithm/inclusive_scan.cpp
    SYCL :: group-algorithm/none_of.cpp
    SYCL :: group-algorithm/reduce.cpp
    

    SYCL :: program_manager/env_vars.cpp
    SYCL :: multi_ptr/multi_ptr.cpp

    SYCL :: scheduler/DataMovement.cpp
    SYCL :: scheduler/MultipleDevices.cpp

    SYCL :: usm/allocator_vector.cpp
    SYCL :: usm/allocatorll.cpp
    SYCL :: usm/allocator_vector_fail.cpp
    SYCL :: usm/badmalloc.cpp
    SYCL :: usm/depends_on.cpp
    SYCL :: usm/dmemll.cpp
    SYCL :: usm/hmemll.cpp
    SYCL :: usm/memadvise.cpp
    SYCL :: usm/memcpy.cpp
    SYCL :: usm/memset.cpp
    SYCL :: usm/mixed.cpp
    SYCL :: usm/mixed2.cpp
    SYCL :: usm/mixed_queue.cpp
    SYCL :: usm/mixed2template.cpp
    SYCL :: usm/queue_wait.cpp
    SYCL :: usm/smemll.cpp

And the following two tests hang when run for CUDA:

basic_tests/device_event.cpp
regression/group.cpp

Follow up PR to rework: #1303

@bjoernknafla bjoernknafla force-pushed the bjoern/fix-get-device-count-by-type-tool branch from 75d856f to 1a33ab8 Compare March 12, 2020 19:25
@bader bader added the cuda CUDA back-end label Mar 12, 2020
fwyzard added a commit to fwyzard/llvm that referenced this pull request Mar 14, 2020
@bader bader requested a review from vladimirlaz March 16, 2020 10:20
fwyzard added a commit to cms-patatrack/llvm that referenced this pull request Mar 21, 2020
@romanovvlad
Copy link
Contributor

@bjoernknafla ping

@bjoernknafla bjoernknafla force-pushed the bjoern/fix-get-device-count-by-type-tool branch from 1a33ab8 to 556d275 Compare March 27, 2020 20:20
@bjoernknafla
Copy link
Contributor Author

I have updated the list of failing and now also hanging tests. I will first rework the PR that fixes LIT tests before this PR can go in.

@bader
Copy link
Contributor

bader commented Apr 9, 2020

@bjoernknafla, could you take a look at merge conflicts, please?

@bjoernknafla
Copy link
Contributor Author

@bader Will do.

@bjoernknafla bjoernknafla force-pushed the bjoern/fix-get-device-count-by-type-tool branch from 556d275 to 01a0e92 Compare April 9, 2020 16:57
@bjoernknafla bjoernknafla requested a review from bader as a code owner April 9, 2020 16:57
@bader bader requested a review from smaslov-intel April 9, 2020 17:00
@@ -119,11 +120,11 @@ def getDeviceCount(device_type):
if getDeviceCount("cpu")[0]:
found_at_least_one_device = True
lit_config.note("Found available CPU device")
cpu_run_substitute = "env SYCL_DEVICE_TYPE=CPU "
cpu_run_substitute = "env SYCL_DEVICE_TYPE=CPU SYCL_BE={SYCL_BE} ".format(SYCL_BE=backend)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On a fail LIT prints out this information and eases running the test by hand. Though the user still needs to add the LD_LIBRARY_PATH to find the SYCL library, but that is not part of this change.

@@ -5,45 +5,29 @@ set(CMAKE_CXX_EXTENSIONS OFF)
add_executable(get_device_count_by_type get_device_count_by_type.cpp)
add_dependencies(get_device_count_by_type ocl-headers ocl-icd)

if( SYCL_BUILD_PI_CUDA )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this to avoid duplication and instead use the cudadrv target set up in the sycl/plugin/cuda/CMakeLists.txt CMake file.

@bjoernknafla bjoernknafla force-pushed the bjoern/fix-get-device-count-by-type-tool branch from 01a0e92 to 379636c Compare April 9, 2020 17:06
Build the `get_device_count_by_type` tool with the required preprocessor
definitions to enable CUDA, and link with CUDA.

Pass correct backend name to the `get_device_count_by_type` tool that
is required to run tests with PI OpenCL instead of PI CUDA.

Clean up the `get_device_count_by_type` tool code to make it harder to
call it with the wrong backend and get unexpected query results back.

Signed-off-by: Bjoern Knafla <[email protected]>
@bjoernknafla bjoernknafla force-pushed the bjoern/fix-get-device-count-by-type-tool branch from 379636c to aee267f Compare April 9, 2020 17:10
Copy link
Contributor

@romanovvlad romanovvlad left a comment

Choose a reason for hiding this comment

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

LGTM

@bader bader requested a review from romanovvlad April 10, 2020 14:49
@bader
Copy link
Contributor

bader commented Apr 10, 2020

@romanovvlad, please, approve to unblock merge.

@bjoernknafla
Copy link
Contributor Author

There is an open change request that GitHub cannot identify as the commit (due to rebasing) is gone...

@bader
Copy link
Contributor

bader commented Apr 10, 2020

@bjoernknafla, basic_tests/buffer/buffer_dev_to_dev.cpp seems to hang. Do you know why?

@bjoernknafla
Copy link
Contributor Author

I haven’t seen buffer_dev_to_dev.cpp hanging when running it locally (otherwise I wouldn’t have updated the PR). I will take a look (though not today).

@bader
Copy link
Contributor

bader commented Apr 12, 2020

I've restarted the job and it passed.
I'll keep an eye on this check and if this issue repeats, I'll investigate.

@bader bader merged commit 6f7cd95 into intel:sycl Apr 12, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 15, 2020
…duler_docs

* origin/sycl:
  [SYCL][PI][CUDA] Implements get_native interoperability (intel#1332)
  [SYCL] Fix check-sycl test suite on systems w/o OpenCL (intel#1503)
  [SYCL][Doc] Update ExtendedAtomics documentation (intel#1487)
  [SYCL][CUDA] Expose context extended deleters on PI API (intel#1483)
  [SYCL][NFC] Remove a dropped environment variable from a test (intel#1506)
  [SYCL] Add opencl-aot to sycl-toolchain target (intel#1504)
  [SYCL] Allow to run deploy LIT tests from particular directory
  [SYCL][CUDA] Fix LIT testing with CUDA devices (intel#1300)
  [SYCL] Remove operator name keywords (intel#1501)
  [Driver][SYCL] Consider .lo files as static archives (intel#1500)
  [SYCL-PTX] Update the compiler design to describe the CUDA target (intel#1408)
  [SYCL] Fix library build on Windows (intel#1499)
  [SYCL][NFC] Refactor lit.cfg.py (intel#1452)
  [SYCL] Fixed sub-buffer memory allocation update (intel#1486)
  [SYCL] Ensure proper definition of spirv builtins for SYCL (intel#1393)
  [SYCL][CUDA] LIT XFAIL/UNSUPPORTED (intel#1303)
  [SYCL][Doc] Function-type kernel attribute extension (intel#1494)
@bjoernknafla bjoernknafla deleted the bjoern/fix-get-device-count-by-type-tool branch April 15, 2020 09:35
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 15, 2020
…c_abi_checks

* origin/sycl: (32 commits)
  [SYCL] Do not force LLVM_INCLUDE_TESTS variable (intel#1505)
  [SYCL][NFC] Align nd_item members with constructor initialization list (intel#1521)
  [SYCL] Move get_info_host implementation to header (intel#1514)
  [SYCL] Always use dynamic CRT for Unit tests (intel#1515)
  [SYCL][NFC] Temporarily disable sporadically failing test (intel#1526)
  [SYCL] Fix inline namespaces (intel#1525)
  [SYCL] Release notes for March'20 DPCPP implementation update (intel#1511)
  [SYCL][PI][CUDA] Implements get_native interoperability (intel#1332)
  [SYCL] Fix check-sycl test suite on systems w/o OpenCL (intel#1503)
  [SYCL][Doc] Update ExtendedAtomics documentation (intel#1487)
  [SYCL][CUDA] Expose context extended deleters on PI API (intel#1483)
  [SYCL][NFC] Remove a dropped environment variable from a test (intel#1506)
  [SYCL] Add opencl-aot to sycl-toolchain target (intel#1504)
  [SYCL] Allow to run deploy LIT tests from particular directory
  [SYCL][CUDA] Fix LIT testing with CUDA devices (intel#1300)
  [SYCL] Remove operator name keywords (intel#1501)
  [Driver][SYCL] Consider .lo files as static archives (intel#1500)
  [SYCL-PTX] Update the compiler design to describe the CUDA target (intel#1408)
  [SYCL] Fix library build on Windows (intel#1499)
  [SYCL][NFC] Refactor lit.cfg.py (intel#1452)
  ...
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.

3 participants