[SYCL] Fix SYCL_DEVICE_FILTER to affect get_devices()/get_platforms() #3397

bso-intel · 2021-03-23T02:57:27Z

SYCL_DEVICE_FILTER will now throw an exception if no matched device is found
regardless of the device_selector type.
Host device is not automatically available if SYCL_DEVICE_FILTER is set.

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

SYCL_DEVICE_FILTER will now throw an exception if no matched device is found regardless of the device_selector type. Host device is not automatically available if SYCL_DEVICE_FILTER is set. Signed-off-by: Byoungro So <[email protected]>

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

sycl/test/Unit/lit.cfg.py

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

bso-intel · 2021-04-06T22:38:15Z

sycl/doc/EnvironmentVariables.md

-| SYCL_DEVICE_FILTER (tentative name) | {backend:device_type:device_num} | Limits the SYCL RT to use only a subset of the system's devices. Setting this environment variable affects all of the device query functions and all of the device selectors. The value of this environment variable is a comma separated list of filters, where each filter is a triple of the form "backend:device_type:device_num" (without the quotes). Each element of the triple is optional, but each filter must have at least one value. Possible values of "backend" are "host", "level_zero", "opencl", "cuda", or "\*". Possible values of "device_type" are "host", "cpu", "gpu", "acc", or "\*". Device_num is an integer that indexes the enumeration of devices from the sycl-ls utility tool, where the first device in that enumeration has index zero. Assuming a filter has all three elements of the triple, it selects only those devices that come from the given backend, have the specified device type, AND have the given device index. If more than one filter is specified, the RT is restricted to the union of devices selected by all filters. The RT always includes the "host" backend and the host device regardless of the filter because the SYCL language requires this device to always be present. Therefore, one can specify 'host' to enforce SYCL to use the host device. Note that the standard selectors like gpu_selector or cpu_selector will throw an exception if the filtered list of devices does not include a device that satisfies the selector. In particular, limiting the devices to only those supported by the "level_zero" backend will cause the cpu_selector to throw an exception since that backend does not support any CPU devices at this time. This environment variable can be used to limit loading only specified plugins into the SYCL RT. |
+| SYCL_BE (deprecated) | PI_OPENCL, PI_LEVEL_ZERO, PI_CUDA | Force SYCL RT to consider only devices of the specified backend during the device selection. We are planning to deprecate SYCL_BE environment variable in the future. The specific grace period is not decided yet. Please use the new env var SYCL_DEVICE_FILTER instead. |
+| SYCL_DEVICE_TYPE (deprecated) | One of: CPU, GPU, ACC, HOST | Force SYCL to use the specified device type. If unset, default selection rules are applied. If set to any unlisted value, this control has no effect. If the requested device type is not found, a `cl::sycl::runtime_error` exception is thrown. If a non-default device selector is used, a device must satisfy both the selector and this control to be chosen. This control only has effect on devices created with a selector. We are planning to deprecate SYCL_DEVICE_TYPE environment variable in the future. The specific grace period is not decided yet. Please use the new env var SYCL_DEVICE_FILTER instead. |
+| SYCL_DEVICE_FILTER | backend:device_type:device_num | See Section [SYCL_DEVIC_EFILTER](#sycl_device_filter)  below. |


The only change here is the "deprecated" tag for SYCL_BE and SYCL_DEVICE_FILTER.
And SYCL_DEVICE_FILTER is taken to a separate section because it is too long to be included in a table.

vladimirlaz · 2021-04-07T08:51:06Z

@vladimirlaz , it seems I address everything you wanted to change. Did I miss anything?

I have no anything else. LGTM

bso-intel · 2021-04-07T16:42:11Z

@vladimirlaz , it seems I address everything you wanted to change. Did I miss anything?

I have no anything else. LGTM

Hmm.. I still see "Changes requested" marked by you below. Could you clear this?

sycl/source/detail/device_filter.cpp

sycl/source/detail/platform_impl.cpp

sycl/source/device.cpp

sycl/unittests/scheduler/AllocaLinking.cpp

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

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

sycl/source/detail/device_filter.cpp

sycl/source/detail/platform_impl.cpp

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

sycl/doc/EnvironmentVariables.md

Co-authored-by: kbobrovs <[email protected]>

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

kbobrovs

Doc LGTM (minor typo)

kbobrovs · 2021-04-14T00:26:52Z

sycl/doc/EnvironmentVariables.md

+
+Device_num is an integer that indexes the enumeration of devices from the sycl-ls utility tool, where the first device in that enumeration has index zero in each backend. For example, SYCL_DEVICE_FILTER=2 will return all devices with index '2' from all different backends. If multiple devices satisfy this device number (e.g., GPU and CPU devices can be assigned device number '2'), then default_selector will choose the device with the highest heuristic point. 
+
+Assuming a filter has all three elements of the triple, it selects only those devices that come from the given backend, have the specified device type, AND have the given device index. If more than one filter is specified, the RT is restricted to the union of devices selected by all filters. The RT does not includes the "host" backend and the host device automatically unless one of the filters explicitly specifies the "host" device type. Therefore, SYCL_DEVICE_FILTER=host should be set to enforce SYCL to use the host device only. 


Suggested change

Assuming a filter has all three elements of the triple, it selects only those devices that come from the given backend, have the specified device type, AND have the given device index. If more than one filter is specified, the RT is restricted to the union of devices selected by all filters. The RT does not includes the "host" backend and the host device automatically unless one of the filters explicitly specifies the "host" device type. Therefore, SYCL_DEVICE_FILTER=host should be set to enforce SYCL to use the host device only.

Assuming a filter has all three elements of the triple, it selects only those devices that come from the given backend, have the specified device type, AND have the given device index. If more than one filter is specified, the RT is restricted to the union of devices selected by all filters. The RT does not include the "host" backend and the host device automatically unless one of the filters explicitly specifies the "host" device type. Therefore, SYCL_DEVICE_FILTER=host should be set to enforce SYCL to use the host device only.

Thanks. Fixed the typo.

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

kbobrovs

Doc LGTM

romanovvlad · 2021-04-14T10:49:59Z

sycl/source/detail/platform_impl.cpp

+    if (PiDevType == PI_DEVICE_TYPE_ACC)
+      DeviceType = info::device_type::accelerator;
+    else if (PiDevType == PI_DEVICE_TYPE_CPU)
+      DeviceType = info::device_type::cpu;
+    else if (PiDevType == PI_DEVICE_TYPE_GPU)
+      DeviceType = info::device_type::gpu;


NIT: device_type enum values are 1:1 with values of PiDeviceType(see sycl/include/CL/sycl/info/info_desc.hpp:147), so we could just cast device_type to PiDeviceType here.

Thanks. Changed as you suggested.

pvchupin · 2021-04-14T18:44:53Z

sycl/doc/EnvironmentVariables.md

-| SYCL_DEVICE_FILTER (tentative name) | {backend:device_type:device_num} | Limits the SYCL RT to use only a subset of the system's devices. Setting this environment variable affects all of the device query functions and all of the device selectors. The value of this environment variable is a comma separated list of filters, where each filter is a triple of the form "backend:device_type:device_num" (without the quotes). Each element of the triple is optional, but each filter must have at least one value. Possible values of "backend" are "host", "level_zero", "opencl", "cuda", or "\*". Possible values of "device_type" are "host", "cpu", "gpu", "acc", or "\*". Device_num is an integer that indexes the enumeration of devices from the sycl-ls utility tool, where the first device in that enumeration has index zero. Assuming a filter has all three elements of the triple, it selects only those devices that come from the given backend, have the specified device type, AND have the given device index. If more than one filter is specified, the RT is restricted to the union of devices selected by all filters. The RT always includes the "host" backend and the host device regardless of the filter because the SYCL language requires this device to always be present. Therefore, one can specify 'host' to enforce SYCL to use the host device. Note that the standard selectors like gpu_selector or cpu_selector will throw an exception if the filtered list of devices does not include a device that satisfies the selector. In particular, limiting the devices to only those supported by the "level_zero" backend will cause the cpu_selector to throw an exception since that backend does not support any CPU devices at this time. This environment variable can be used to limit loading only specified plugins into the SYCL RT. |
+| SYCL_BE (deprecated) | PI_OPENCL, PI_LEVEL_ZERO, PI_CUDA | Force SYCL RT to consider only devices of the specified backend during the device selection. We are planning to deprecate SYCL_BE environment variable in the future. The specific grace period is not decided yet. Please use the new env var SYCL_DEVICE_FILTER instead. |
+| SYCL_DEVICE_TYPE (deprecated) | CPU, GPU, ACC, HOST | Force SYCL to use the specified device type. If unset, default selection rules are applied. If set to any unlisted value, this control has no effect. If the requested device type is not found, a `cl::sycl::runtime_error` exception is thrown. If a non-default device selector is used, a device must satisfy both the selector and this control to be chosen. This control only has effect on devices created with a selector. We are planning to deprecate SYCL_DEVICE_TYPE environment variable in the future. The specific grace period is not decided yet. Please use the new env var SYCL_DEVICE_FILTER instead. |
+| SYCL_DEVICE_FILTER | backend:device_type:device_num | See Section [SYCL_DEVIC_EFILTER](#sycl_device_filter)  below. |


Thanks. Fixed the typo.

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

bso-intel added 2 commits March 22, 2021 19:54

fix the test config files to support host

dcbf1d3

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

bso-intel mentioned this pull request Mar 24, 2021

[SYCL] Fix lit.cfg.py to set SYCL_DEVICE_FILTER correctly. intel/llvm-test-suite#190

Merged

bso-intel added 2 commits March 23, 2021 19:39

added host for RUN_PLACEHOLDERs

3565259

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

add back SYCL_DEVICE_FILTER in lit.cfg.py

eefcb92

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

vladimirlaz suggested changes Mar 25, 2021

View reviewed changes

sycl/test/Unit/lit.cfg.py Outdated Show resolved Hide resolved

vladimirlaz and others added 7 commits March 25, 2021 14:19

Update sycl/test/Unit/lit.cfg.py

37693ef

try to fix ubuntu failure

c023e1d

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

revert the trial fix

bcaa341

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

add back Vladimir's fix

4e504bd

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

Merge remote-tracking branch 'upstream/sycl' into filter

477f731

fix DeviceNum error

be82970

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

remove debugging print

46f7d00

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

bso-intel mentioned this pull request Mar 30, 2021

[SYCL] Fix SYCL_DEVICE_FILTER setting intel/llvm-test-suite#203

Merged

Merge remote-tracking branch 'upstream/sycl' into filter

566d6c2

bso-intel mentioned this pull request Mar 31, 2021

[SYCL] Fix SYCL_DEVICE_FILTER setting intel/llvm-test-suite#208

Merged

bso-intel added 8 commits March 31, 2021 10:08

fix the mock ordering issue

b4e598d

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

fixed doc

5a2771a

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

format error

e842438

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

more edit

4f89a4f

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

more edit

451f795

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

more edit

cb5094b

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

Merge remote-tracking branch 'upstream/sycl' into filter

d670ef7

cleanup

8b6bb32

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

bso-intel commented Apr 6, 2021

View reviewed changes

bso-intel marked this pull request as ready for review April 6, 2021 22:40

bso-intel requested review from kbobrovs, pvchupin and a team as code owners April 6, 2021 22:40

bso-intel requested a review from romanovvlad April 6, 2021 22:40

vladimirlaz approved these changes Apr 7, 2021

View reviewed changes

vladimirlaz self-requested a review April 7, 2021 17:21

vladimirlaz previously approved these changes Apr 8, 2021

View reviewed changes

romanovvlad requested changes Apr 11, 2021

View reviewed changes

bso-intel dismissed vladimirlaz’s stale review via fc05373 April 12, 2021 16:39

bso-intel and others added 5 commits April 12, 2021 09:39

Update sycl/source/detail/platform_impl.cpp

fc05373

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

Update sycl/source/device.cpp

fd06069

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

Merge remote-tracking branch 'upstream/sycl' into filter

46cd056

respond to feedback

bd9b64b

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

fix host selector

2e095e8

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

romanovvlad reviewed Apr 13, 2021

View reviewed changes

sycl/source/detail/device_filter.cpp Outdated Show resolved Hide resolved

sycl/source/detail/platform_impl.cpp Outdated Show resolved Hide resolved

piDeviceType to determin device_type

f0a7a01

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

kbobrovs reviewed Apr 13, 2021

View reviewed changes

sycl/doc/EnvironmentVariables.md Outdated Show resolved Hide resolved

sycl/doc/EnvironmentVariables.md Outdated Show resolved Hide resolved

bso-intel and others added 2 commits April 13, 2021 16:17

Update sycl/doc/EnvironmentVariables.md

cdd8c49

Co-authored-by: kbobrovs <[email protected]>

restructure the description of env var

de98a6b

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

kbobrovs previously approved these changes Apr 14, 2021

View reviewed changes

typo

d9fd2c6

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

bso-intel dismissed kbobrovs’s stale review via d9fd2c6 April 14, 2021 00:47

kbobrovs previously approved these changes Apr 14, 2021

View reviewed changes

romanovvlad previously approved these changes Apr 14, 2021

View reviewed changes

pvchupin reviewed Apr 14, 2021

View reviewed changes

bso-intel added 2 commits April 14, 2021 12:01

fix typo

fbf6ca4

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

simplified cast

c81f196

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

bso-intel dismissed stale reviews from romanovvlad and kbobrovs via c81f196 April 14, 2021 19:35

pvchupin approved these changes Apr 14, 2021

View reviewed changes

pvchupin merged commit ef4e6dd into intel:sycl Apr 14, 2021

bso-intel deleted the filter branch April 23, 2021 22:31


		Device_num is an integer that indexes the enumeration of devices from the sycl-ls utility tool, where the first device in that enumeration has index zero in each backend. For example, SYCL_DEVICE_FILTER=2 will return all devices with index '2' from all different backends. If multiple devices satisfy this device number (e.g., GPU and CPU devices can be assigned device number '2'), then default_selector will choose the device with the highest heuristic point.

		Assuming a filter has all three elements of the triple, it selects only those devices that come from the given backend, have the specified device type, AND have the given device index. If more than one filter is specified, the RT is restricted to the union of devices selected by all filters. The RT does not includes the "host" backend and the host device automatically unless one of the filters explicitly specifies the "host" device type. Therefore, SYCL_DEVICE_FILTER=host should be set to enforce SYCL to use the host device only.

	\| SYCL_DEVICE_FILTER \| backend:device_type:device_num \| See Section [SYCL_DEVIC_EFILTER](#sycl_device_filter) below. \|
	\| SYCL_DEVICE_FILTER \| backend:device_type:device_num \| See Section [SYCL_DEVICE_FILTER](#sycl_device_filter) below. \|

[SYCL] Fix SYCL_DEVICE_FILTER to affect get_devices()/get_platforms() #3397

[SYCL] Fix SYCL_DEVICE_FILTER to affect get_devices()/get_platforms() #3397

Uh oh!

Conversation

bso-intel commented Mar 23, 2021

Uh oh!

Uh oh!

bso-intel Apr 6, 2021

Choose a reason for hiding this comment

Uh oh!

vladimirlaz commented Apr 7, 2021

Uh oh!

bso-intel commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kbobrovs left a comment

Choose a reason for hiding this comment

Uh oh!

kbobrovs Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

bso-intel Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

kbobrovs left a comment

Choose a reason for hiding this comment

Uh oh!

romanovvlad Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

bso-intel Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

pvchupin Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

bso-intel Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!