[SYCL] Implement new env var SYCL_FORCE_PI and extend device_selector::select_device #2214

bso-intel · 2020-07-29T23:39:09Z

This PR responds to several requests for user controls to loading only specific plugins into SYCL RT, and a way to request a specific device regardless of SYCL_BE setting.
For the first request, this PR introduced a new env var SYCL_FORCE_PI. The possible values are any combination of {opencl, level0, cuda}. Only the specified plugins will be loaded into SYCL RT.
For the second request, this PR extends device_selector::select_device() to take optional parameters device_type and backend.
In current implementation, device availability is heavily depending on the env var SYCL_BE and SYCL_DEVICE_TYPE. The user wanted a control in API to pin down specific device regardless of SYCL_BE setting.
Note that SYCL_BE=PI_LEVEL0 prevents users from using CPU and ACC devices.

Extended select_device to take optional parameters device_type and backend. The current implementation of select_device returns different devices depending on the env var such as SYCL_BE and SYCL_DEVICE_TYPE, and uses heuristic. This extension of select_device will allow users to set the desired device type and possibly also backend if multiple devices are the same device type. Signed-off-by: Byoungro So <[email protected]>

Signed-off-by: Byoungro So <[email protected]>

Extended select_device to take optional parameters device_type and backend. The current implementation of select_device returns different devices depending on the env var such as SYCL_BE and SYCL_DEVICE_TYPE, and uses heuristic. This extension of select_device will allow users to set the desired device type and possibly also backend if multiple devices are the same device type. Signed-off-by: Byoungro So <[email protected]>

Signed-off-by: Byoungro So <[email protected]>

…o select-device

Extended select_device to take optional parameters device_type and backend. The current implementation of select_device returns different devices depending on the env var such as SYCL_BE and SYCL_DEVICE_TYPE, and uses heuristic. This extension of select_device will allow users to set the desired device type and possibly also backend if multiple devices are the same device type. Signed-off-by: Byoungro So <[email protected]>

Signed-off-by: Byoungro So <[email protected]>

…o select-device

Signed-off-by: Byoungro So <[email protected]>

This reverts commit a43dcc2.

alexbatashev

How is it going to affect user-defined selectors? Why don't users use custom selectors? If someone needs to control device selection with environment variables, why don't they create a special device selector?

alexbatashev · 2020-07-30T07:49:39Z

sycl/include/CL/sycl/device_selector.hpp

+  // deviceType is an optional parameter to set the desired device
+  // info::device_type::all means a heuristic is used to select a device with
+  // highest score
+  device select_device(info::device_type deviceType = info::device_type::all,
+                       backend BE = backend::level0) const;


Can we implement it as a non-breaking change?

This change won't affect existing abi. The existing code will continue to work as before based on a heuristic since no parameters are passed.

alexbatashev · 2020-07-30T08:04:32Z

sycl/source/device_selector.cpp

+device device_selector::select_device(info::device_type deviceType,
+                                      backend be) const {
+  // return if a requested deviceType is found
+  if (deviceType != info::device_type::all) {
+    if (deviceType == info::device_type::host) {
+      vector_class<device> devices;
+      devices.resize(1);
+      return devices[0];
+    }
+
+    const vector_class<detail::plugin> &plugins = RT::initialize();
+    for (unsigned int i = 0; i < plugins.size(); i++) {
+      pi_uint32 numPlatforms = 0;
+      plugins[i].call<detail::PiApiKind::piPlatformsGet>(0, nullptr,
+                                                         &numPlatforms);
+      if (numPlatforms) {
+        vector_class<RT::PiPlatform> piPlatforms(numPlatforms);
+        plugins[i].call<detail::PiApiKind::piPlatformsGet>(
+            numPlatforms, piPlatforms.data(), nullptr);
+        for (const auto &piPlatform : piPlatforms) {
+          platform pltf = detail::createSyclObjFromImpl<platform>(
+              std::make_shared<detail::platform_impl>(piPlatform, plugins[i]));
+          if (!pltf.is_host()) {
+            vector_class<device> devices = pltf.get_devices(deviceType);
+            for (uint32_t i = 0; i < devices.size(); i++) {
+              if (deviceType != info::device_type::gpu) {
+                return devices[i];
+              } else if (devices[i].is_gpu() && be == pltf.get_backend()) {
+                return devices[i];
+              }
+            }
+          }
+        }
+      }
+    }
+  }


This seem to violate the spirit of device_selector. Why not just give some boost to matching device scores?

The problem is that SYCL_BE=PI_LEVEL0 will hide CPU and ACC devices from available devices in the system. So giving bonus points won't work.
As I mentioned above, the existing calls to select_device() with no parameter specified will continue to work and does not violate the existing code.

alexbatashev · 2020-07-30T08:05:21Z

sycl/source/device_selector.cpp

+      vector_class<device> devices;
+      devices.resize(1);
+      return devices[0];


Suggested change

vector_class<device> devices;

devices.resize(1);

return devices[0];

return device{};

alexbatashev · 2020-07-30T08:05:33Z

sycl/source/device_selector.cpp

+    }
+
+    const vector_class<detail::plugin> &plugins = RT::initialize();
+    for (unsigned int i = 0; i < plugins.size(); i++) {


Range-based for?

alexbatashev · 2020-07-30T08:06:02Z

sycl/source/device_selector.cpp

+              if (deviceType != info::device_type::gpu) {
+                return devices[i];
+              } else if (devices[i].is_gpu() && be == pltf.get_backend()) {
+                return devices[i];
+              }


If we ever have a non-OpenCL based backend for other devices, this will be a bug.

As described in the comment, the second parameter 'be' is meant to designate specific GPU device backend, and not used for other device types.

alexbatashev · 2020-07-30T08:06:55Z

sycl/test/abi/sycl_symbols_linux.dump

@@ -3858,7 +3858,7 @@ _ZNK2cl4sycl14interop_handle12getNativeMemEPNS0_6detail16AccessorImplHostE
 _ZNK2cl4sycl14interop_handle14getNativeQueueEv
 _ZNK2cl4sycl14interop_handle15getNativeDeviceEv
 _ZNK2cl4sycl14interop_handle16getNativeContextEv
-_ZNK2cl4sycl15device_selector13select_deviceEv
+_ZNK2cl4sycl15device_selector13select_deviceENS0_4info11device_typeENS0_7backendE


If you still want to break ABI, please, uplift DEV_ABI version.

alexbatashev · 2020-07-30T08:10:53Z

sycl/source/detail/pi.cpp

+  // to be loaded. Note that SYCL_BE=PI_LEVEL0 prevents from other OCL CPU
+  // devices to be discovered.
+
+  const char *envVal = std::getenv("SYCL_FORCE_PI");


SYCLConfig?

Enables partial specialization of atomic_ref for pointer types. Implementation assumes that both host and device pointers can be stored in a uintptr_t, but uses compare_exchange to implement pointer arithmetic rather than make assumptions about how pointers will be represented on different devices. Signed-off-by: John Pennycook <[email protected]>

sycl/test/basic_tests/select_device.cpp

romanovvlad · 2020-07-30T11:01:20Z

This PR responds to several requests for user controls to loading only specific plugins into SYCL RT, and a way to request a specific device regardless of SYCL_BE setting.
For the first request, this PR introduced a new env var SYCL_FORCE_PI. The possible values are any combination of {opencl, level0, cuda}. Only the specified plugins will be loaded into SYCL RT.
For the second request, this PR extends device_selector::select_device() to take optional parameters device_type and backend.
In current implementation, device availability is heavily depending on the env var SYCL_BE and SYCL_DEVICE_TYPE. The user wanted a control in API to pin down specific device regardless of SYCL_BE setting.
Note that SYCL_BE=PI_LEVEL0 prevents users from using CPU and ACC devices.

Do I understand correctly that SYCL_FORCE_PI is the same as SYCL_BE but support several plugins specified?
If so, why we need two variables? Can't we just extend SYCL_BE to support multiple plugins?

…xtension (intel#2221) Signed-off-by: Michael Kinsner <[email protected]>

…tel#2025) PI_LEVEL0 -> PI_LEVEL_ZERO. PI_LEVEL0 is still accepted and handled correctly. Signed-off-by: Gail Lyons <[email protected]>

This version supports side-by-side installation of OpenCL RT for Intel CPU and OpenCL RT for Intel FPGA emulation.

bso-intel · 2020-07-30T17:55:09Z

It seems I have to clarify the difference between SYCL_BE and SYCL_FORCE_PI.
SYCL_BE is designed to tell SYCL RT the choice of GPU backend. It has a side effect of hiding other device types such as CPU and ACC.
SYCL_FORCE_PI is limit SYCL RT to load only designated plugins.
Here is some information to help you understand why this makes a difference.
SYCL RT look for all possible plugins in the system, and load them all. And then, it provides platforms and devices supported from these plugins. SYCL_BE can affect the device discovery at this time.
SYCL_FORCE_PI limits SYCL RT to only look for specified plugins at the beginning. Unspecified plugins won't even be loaded to SYCL RT.
There was a request for this user control to avoid some problems on the user side.

Another clarification may be needed between select_device() vs specific device_selector.
Specific device_selectors are disabled when SYCL_BE=PI_LEVEL0 is set. For example, cpu_selector won't return the available device. Customers use SYCL_BE to switch between different GPU devices and they forget they set it once before, and they found CPU and ACC devices are not working.
Customers asked a way to select a particular device by themselves instead of relying on the heuristic point.
Select_device() still continue to work as before based on the heuristic if 2 optional parameters are not passed.
These optional parameters of select_device(device_type, backend) will allow the user to "select" their favorite device without worrying about env vars such as SYCL_BE or SYCL_DEVICE_TYPE.

bso-intel · 2020-07-30T18:43:17Z

How is it going to affect user-defined selectors? Why don't users use custom selectors? If someone needs to control device selection with environment variables, why don't they create a special device selector?

All different kinds of device selectors are governed by SYCL_BE and SYCL_DEVICE_TYPE. For example, CPU and ACC devices are not available as soon as the user sets SYCL_BE=PI_LEVEL0 for GPU. There was a request for customer request to "select" their favorite device regardless of these env vars.

Co-authored-by: Romanov Vlad <[email protected]>

bso-intel · 2020-07-30T18:58:49Z

This PR responds to several requests for user controls to loading only specific plugins into SYCL RT, and a way to request a specific device regardless of SYCL_BE setting.
For the first request, this PR introduced a new env var SYCL_FORCE_PI. The possible values are any combination of {opencl, level0, cuda}. Only the specified plugins will be loaded into SYCL RT.
For the second request, this PR extends device_selector::select_device() to take optional parameters device_type and backend.
In current implementation, device availability is heavily depending on the env var SYCL_BE and SYCL_DEVICE_TYPE. The user wanted a control in API to pin down specific device regardless of SYCL_BE setting.
Note that SYCL_BE=PI_LEVEL0 prevents users from using CPU and ACC devices.

Do I understand correctly that SYCL_FORCE_PI is the same as SYCL_BE but support several plugins specified?
If so, why we need two variables? Can't we just extend SYCL_BE to support multiple plugins?

I explained the difference between SYCL_FORCE_PI and SYCL_BE. I hope I clarified your questions.

Extended select_device to take optional parameters device_type and backend. The current implementation of select_device returns different devices depending on the env var such as SYCL_BE and SYCL_DEVICE_TYPE, and uses heuristic. This extension of select_device will allow users to set the desired device type and possibly also backend if multiple devices are the same device type. Signed-off-by: Byoungro So <[email protected]>

Signed-off-by: Byoungro So <[email protected]>

Extended select_device to take optional parameters device_type and backend. The current implementation of select_device returns different devices depending on the env var such as SYCL_BE and SYCL_DEVICE_TYPE, and uses heuristic. This extension of select_device will allow users to set the desired device type and possibly also backend if multiple devices are the same device type. Signed-off-by: Byoungro So <[email protected]>

Signed-off-by: Byoungro So <[email protected]>

Co-authored-by: Romanov Vlad <[email protected]>

…o select-device Signed-off-by: Byoungro So <[email protected]>

Signed-off-by: Byoungro So <[email protected]>

bso-intel · 2020-07-31T20:15:13Z

I am closing this PR and will create a new one based on our discussion today.

This PR aims to introduce entities related to OpCooperativeMatrixApplyFunctionINTEL in llvm-spirv translator, according to https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_joint_matrix.asciidoc. Co-authored-by: Sidorov, Dmitry <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@467edf9

bso-intel added 16 commits July 26, 2020 22:13

added SYCL_FORCE_PI env var to pin down plugins to load

7b5467f

Signed-off-by: Byoungro So <[email protected]>

Added description of SYCL_FORCE_PI env var.

2787d08

Signed-off-by: Byoungro So <[email protected]>

added SYCL_FORCE_PI env var to pin down plugins to load

eaa881a

Signed-off-by: Byoungro So <[email protected]>

Added description of SYCL_FORCE_PI env var.

4c85986

Signed-off-by: Byoungro So <[email protected]>

clang-format

f2eb166

Signed-off-by: Byoungro So <[email protected]>

Merge branch 'select-device' of https://github.com/bso-intel/llvm int…

5ada30b

…o select-device

added SYCL_FORCE_PI env var to pin down plugins to load

bbeb7ba

Signed-off-by: Byoungro So <[email protected]>

Added description of SYCL_FORCE_PI env var.

0250948

Signed-off-by: Byoungro So <[email protected]>

added SYCL_FORCE_PI env var to pin down plugins to load

7f6a37d

Signed-off-by: Byoungro So <[email protected]>

clang-format

8507621

Signed-off-by: Byoungro So <[email protected]>

fix error

fe4f3de

Signed-off-by: Byoungro So <[email protected]>

reverted

fc98abc

Signed-off-by: Byoungro So <[email protected]>

Merge branch 'select-device' of https://github.com/bso-intel/llvm int…

9f30db1

…o select-device

bso-intel requested review from kbobrovs, pvchupin, smaslov-intel and a team as code owners July 29, 2020 23:39

bso-intel and others added 4 commits July 29, 2020 16:43

fix merge error

0c3b2f5

Signed-off-by: Byoungro So <[email protected]>

reverted

ec3bee3

Signed-off-by: Byoungro So <[email protected]>

disable accelerator on windows

8938388

Signed-off-by: Byoungro So <[email protected]>

Revert "[SYCL] Disallow mutable lambdas (intel#1785)" (intel#2213)

8ac87a3

This reverts commit a43dcc2.

alexbatashev suggested changes Jul 30, 2020

View reviewed changes

alexbatashev requested a review from romanovvlad July 30, 2020 08:11

romanovvlad reviewed Jul 30, 2020

View reviewed changes

sycl/test/basic_tests/select_device.cpp Show resolved Hide resolved

romanovvlad reviewed Jul 30, 2020

View reviewed changes

sycl/test/basic_tests/select_device.cpp Show resolved Hide resolved

Mike Kinsner and others added 3 commits July 30, 2020 17:04

[SYCL][Doc] Fix typo in directory name of ParallelForSimplification e…

0bc2441

…xtension (intel#2221) Signed-off-by: Michael Kinsner <[email protected]>

[SYCL] Changed sycl::backend::level0 to sycl::backend::level_zero (in…

215f591

…tel#2025) PI_LEVEL0 -> PI_LEVEL_ZERO. PI_LEVEL0 is still accepted and handled correctly. Signed-off-by: Gail Lyons <[email protected]>

[NFC] Update OpenCL installation process (intel#2201)

4c57d4d

This version supports side-by-side installation of OpenCL RT for Intel CPU and OpenCL RT for Intel FPGA emulation.

Update sycl/test/basic_tests/select_device.cpp

9b6ca2c

Co-authored-by: Romanov Vlad <[email protected]>

bso-intel and others added 13 commits July 30, 2020 13:14

added SYCL_FORCE_PI env var to pin down plugins to load

292a7d2

Signed-off-by: Byoungro So <[email protected]>

Added description of SYCL_FORCE_PI env var.

a6a12af

Signed-off-by: Byoungro So <[email protected]>

added SYCL_FORCE_PI env var to pin down plugins to load

b2ae042

Signed-off-by: Byoungro So <[email protected]>

clang-format

1d766f1

Signed-off-by: Byoungro So <[email protected]>

clang-format

68abe10

Signed-off-by: Byoungro So <[email protected]>

fix error

837b0c9

Signed-off-by: Byoungro So <[email protected]>

reverted

50e922a

Signed-off-by: Byoungro So <[email protected]>

disable accelerator on windows

c56d88e

Signed-off-by: Byoungro So <[email protected]>

Update sycl/test/basic_tests/select_device.cpp

64987dc

Co-authored-by: Romanov Vlad <[email protected]>

Merge branch 'select-device' of https://github.com/bso-intel/llvm int…

3629976

…o select-device Signed-off-by: Byoungro So <[email protected]>

accommodated review feedback

063a8e4

Signed-off-by: Byoungro So <[email protected]>

bso-intel requested review from bader, elizabethandrews, Fznamznon, jbrodman, mkinsner and premanandrao as code owners July 31, 2020 02:14

bso-intel closed this Jul 31, 2020

bso-intel deleted the select-device branch August 4, 2020 20:11

[SYCL] Implement new env var SYCL_FORCE_PI and extend device_selector::select_device #2214

[SYCL] Implement new env var SYCL_FORCE_PI and extend device_selector::select_device #2214

Uh oh!

Conversation

bso-intel commented Jul 29, 2020

Uh oh!

alexbatashev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

romanovvlad commented Jul 30, 2020

Uh oh!

bso-intel commented Jul 30, 2020

Uh oh!

bso-intel commented Jul 30, 2020

Uh oh!

bso-intel commented Jul 30, 2020

Uh oh!

bso-intel commented Jul 31, 2020

Uh oh!

Uh oh!