[SYCL] Make ONEAPI_DEVICE_SELECTOR reject an empty string

[uditagarwal97]

ONEAPI_DEVICE_SELECTOR="" causes SYCL runtime to silently not select/initialize any device at all. This PR makes SYCL runtime to throw an error when input to ONEAPI_DEVICE_SELECTOR is null.

[aelovikov-intel]

What does documentation say? Should we update it as well? I think raising an exception would be fine too.

• @steffenlarsen

[gmlueck]

I also wonder if this is the right behavior. I think a strict reading of the BNF grammar in the documentation says that this should be an error. I think the grammar requires at least one term in the expression string.

I think you could also make an argument that the current behavior is correct. It does sort of make sense that setting ONEAPI_DEVICE_SELECTOR to an empty string would cause no devices to be selected. (I.e. calling device::get_devices would return an empty vector.)

I think both of these choices (error or select no devices) would make more sense than treating this the same as having ONEAPI_DEVICE_SELECTOR unset.

[RaviNarayanaswamy]

Currently OpenMP ignore ONEAPI_DEVICE_SELECTOR if it is set to null

[uditagarwal97]

I now think it would be better to throw a meaningful exception instead of not selecting any device at all.

@RaviNarayanaswamy if not a major change, could we update OpenMP to also throw an error on ONEAPI_DEVICE_SELECTOR=""? I could do the same in SYCL runtime.

[RaviNarayanaswamy]

Does DPC++ throw an error for other env variables if they are set to NULL?

[zettai-reido]

The behavior of OpenMP and SYCL in this case should be identical to avoid many problems.

Currently OpenMP ignore ONEAPI_DEVICE_SELECTOR if it is set to null

And what about space?

[uditagarwal97]

@zettai-reido Both OpenMP and DPC++ throws a parsing error on ONEAPI_DEVICE_SELECTOR=" " (space)

Target LEVEL_ZERO RTL --> Loaded library 'libomptarget.sycl.wrap.so':
terminate called after throwing an instance of 'sycl::_V1::exception'
  what():  ONEAPI_DEVICE_SELECTOR parsing error. Backend is required but missing from " "
Aborted (core dumped)

[RaviNarayanaswamy]

Openmp error is not during parsing. It is abort due to sycl not providing device list when asked for.

[uditagarwal97]

@RaviNarayanaswamy You are right. I looked at OpenMP's parsing of ONEAPI_DEVICE_SELECTOR and it does not throw when the value of ONEAPI_DEVICE_SELECTOR is illegal (like "", " ", "a:a", "a:a;a:a", etc), instead it just ignores it. This is different from how it's done in DPC++. OpenMP's parsing of ONEAPI_DEVICE_SELECTOR also doesn't take into account negated filters (like !opencl). Although, I'm not very familiar with the interactions between OpenMP plugins and SYCL runtime, IMO, we should align OpenMP's ONEAPI_DEVICE_SELECTOR parsing with that of DPC++, to prevent issues like this in future.

[RaviNarayanaswamy]

Yes OpenMP and DPC++ should have used the same code. Unfortunately OpenMP could not use DPC++ implementation as it was integrated into sycl. I will change OpenMP to report error if the ONEAPI_DEVICE_SELECTOR is not valid instead of silently ignoring it. Can you open a JIRA for this.
Hopefully the parsing of ONEAPI_DEVICE_SELECTOR will be done in UR when we switch to using it.

[uditagarwal97]

@intel/llvm-gatekeepers The PR is ready!