Skip to content

[SYCL] Indicate lack of direct OpenCL interop when not using OpenCL BE. #1242

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.

Sign up for GitHub

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 2 commits into from
Mar 13, 2020
Merged

[SYCL] Indicate lack of direct OpenCL interop when not using OpenCL BE. #1242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49aea89
[SYCL] Indicate lack of direct OpenCL interop when not using OpenCL BE.
rbegam Mar 4, 2020
051f200
[SYCL] removes duplicate requirement.
rbegam Mar 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sycl/test/basic_tests/buffer/buffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx %s -o %t1.out -lsycl
// RUN: env SYCL_DEVICE_TYPE=HOST %t1.out
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t2.out
Expand Down
4 changes: 2 additions & 2 deletions sycl/test/basic_tests/buffer/buffer_interop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

// REQUIRES: opencl

//==------------------- buffer.cpp - SYCL buffer basic test ----------------==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
Expand Down
4 changes: 2 additions & 2 deletions sycl/test/basic_tests/buffer/subbuffer_interop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

// REQUIRES: opencl

//==------------ subbuffer_interop.cpp - SYCL buffer basic test ------------==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/basic_tests/event.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
//==--------------- event.cpp - SYCL event test ----------------------------==//
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/basic_tests/image_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple -I %sycl_source_dir %s -o %t1.out
// RUN: %clangxx -I %sycl_source_dir %s -o %t3.out -lsycl
// RUN: env SYCL_DEVICE_TYPE=HOST %t1.out
Expand Down
4 changes: 2 additions & 2 deletions sycl/test/basic_tests/kernel_interop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

// REQUIRES: opencl

//==--------------- kernel_interop.cpp - SYCL kernel ocl interop test ------==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/basic_tests/sampler/sampler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove lines 9 and 10?

// TODO: Image support in CUDA backend
// XFAIL: cuda

Copy link
Contributor

Choose a reason for hiding this comment

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

It would not hurt though might be better as an extra PR to not block this one that re-enables LIT testing with CUDA?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay to resolve this comment in a separate pull request.
@bjoernknafla, will do this?
We need to clean-up following files from this PR:

basic_tests/buffer/buffer.cpp
basic_tests/sampler/sampler.cpp
fpga_tests/fpga_queue.cpp
ordered_queue/ordered_buffs.cpp
ordered_queue/ordered_dmemll.cpp
sub_group/common_ocl.cpp

I think all these files should have something like:

// REQUIRES: opencl
// UNSUPPORTED: 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.

If I remember correctly @bjoernknafla mentioned that XFAIL: cuda was there to indicate that the test fails for CUDA and needs to be fixed. Have all the TODOs been resolved for ^ tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

These are requiring OpenCL, so I assume that none of them can be "fixed on CUDA". Am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

sampler.cpp should have a UNSUPPORTED: cuda on it.
If the others already have REQUIRES: opencl nothing else should be required. However, I will look over this in an upcoming PR, so we could get LIT fixes of this PR in.

Expand Down
5 changes: 2 additions & 3 deletions sycl/test/basic_tests/set_arg_interop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL -O3
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out

// REQUIRES: opencl


#include <CL/sycl.hpp>

#include <cassert>
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/fpga_tests/fpga_queue.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/kernel-and-program/cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -I %sycl_source_dir %s -o %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
//==---------------- cache.cpp - SYCL kernel/program test ------------------==//
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/kernel-and-program/kernel-and-program.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
Expand Down
3 changes: 2 additions & 1 deletion sycl/test/linear_id/opencl-interop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// REQUIRES: opencl
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume it means that this test is supposed to be skipped if there is no OpenCL support in the system.
If so, the code which detects presence of OpenCL looks strange:

# PI API either supports OpenCL or CUDA.
opencl = False
if not cuda:
    opencl = True
    config.available_features.add('opencl')

Copy link
Contributor

Choose a reason for hiding this comment

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

The current test idea is to either run testing for OpenCL xor CUDA.

(My understanding is though that some tests do not go through the SYCL default selector and therefore won't react to the env var - this adds some nondeterminism to which backend they choose).

The selection happens as follows:

  • If the check-sycl target is called, then we want to test OpenCL.
  • If the check-sycl-cuda target is called, then we want to test CUDA.
  • By hand this would either be:
    ** ./bin/llvm-lit --param SYCL_BE=PI_OPENCL ./tools/sycl/test or
    ** ./bin/llvm-lit --param SYCL_BE=PI_CUDA ./tools/sycl/test

The Python script for the LIT SYCL tests uses the SYCL_BE environment variable to pass a backend parameter to the get_device_count_by_type tool which only reports the available devices belonging to the parameter. It won't report CUDA devices when looking for OpenCL and vice versa.

Therefore, we either set the cuda xor the opencl feature but not both together.

However, it is always possible to explicitly query for all devices of all backends by not using the default device selector.

Copy link
Contributor

@bader bader Mar 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't have neither OpenCL nor CUDA on my machine. What will happen to my check-sycl?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify, I expect my tests to run on the host device only, but I'm not sure that tests which explicitly requested OpenCL will be disabled.

Copy link
Contributor

@bjoernknafla bjoernknafla Mar 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Good question. I am not sure how to solve this cleanly right now, e.g., we would want to say that no backend is under test (to express that we test HOST)?

What will happen at the moment is:

  • OpenCL testing is activated.
  • No devices are found by the get-device-count-by-type tool which limits the RUN lines - so many test might skip while some fail.
  • Tests themselves still find all available devices at runtime.
  • Some tests explicitly check that they are not on host before calling into OpenCL.

If I run ninja check-sycl on the tip of sycl on a machine without OpenCL but with CUDA I get the following:

Testing Time: 158.27s
********************
Failing Tests (5):
    SYCL :: kernel_from_file/hw.cpp   <- unexpected interaction with CUDA
    SYCL :: scheduler/DataMovement.cpp   <- unexpected interaction with CUDA
    SYCL :: scheduler/MultipleDevices.cpp   <- unexpected interaction with CUDA
    SYCL :: separate-compile/test.cpp    <- unexpected interaction with CUDA

  Expected Passes    : 201
  Unsupported Tests  : 17
  Unexpected Failures: 5

On a machine without OpenCL and when not building the CUDA backend no failures happen.

Something to look into though not in this PR I recon.

Copy link
Contributor

Choose a reason for hiding this comment

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

NOTE: this file already has // REQUIRES: opencl at line 7.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bjoernknafla, I'm okay to resolve this separately. I need to go deeper into LIT config file to understand what is the best way to handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I missed the // REQUIRES: opencl at line 7. Let me check if I've missed any more.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the only case.


// RUN: %clangxx -fsycl %s -o %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
// RUN: %GPU_RUN_PLACEHOLDER %t.out
// RUN: %ACC_RUN_PLACEHOLDER %t.out
// REQUIRES: opencl
// UNSUPPORTED: cuda
//==---------------- opencl-interop.cpp - SYCL linear id test --------------==//
//
Expand Down
3 changes: 3 additions & 0 deletions sycl/test/lit.cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
if 'OCL_ICD_FILENAMES' in os.environ:
config.environment['OCL_ICD_FILENAMES'] = os.environ['OCL_ICD_FILENAMES']

if 'SYCL_DEVICE_WHITE_LIST' in os.environ:
config.environment['SYCL_DEVICE_WHITE_LIST'] = os.environ['SYCL_DEVICE_WHITE_LIST']

config.substitutions.append( ('%sycl_libs_dir', config.sycl_libs_dir ) )
config.substitutions.append( ('%sycl_include', config.sycl_include ) )
config.substitutions.append( ('%opencl_libs_dir', config.opencl_libs_dir) )
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/ordered_queue/ordered_buffs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl %s -o %t.out -L %opencl_libs_dir -lOpenCL
// RUN: %ACC_RUN_PLACEHOLDER %t.out
// RUN: %CPU_RUN_PLACEHOLDER %t.out
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/ordered_queue/ordered_dmemll.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clangxx -fsycl %s -o %t1.out -L %opencl_libs_dir -lOpenCL
// RUN: %CPU_RUN_PLACEHOLDER %t1.out
// RUN: %GPU_RUN_PLACEHOLDER %t1.out
Expand Down
2 changes: 2 additions & 0 deletions sycl/test/sub_group/common_ocl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// REQUIRES: opencl

// RUN: %clang_cc1 -x cl -cl-std=CL2.0 %S/sg.cl -triple spir64-unknown-unknown -emit-llvm-bc -o %T/kernel_ocl.bc -include opencl-c.h
// RUN: llvm-spirv %T/kernel_ocl.bc -o %T/kernel_ocl.spv
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out -L %opencl_libs_dir -lOpenCL
Expand Down