[NFCI][SYCL] Refactor device selection in platform_impl.cpp #12288
Conversation
…d == backend::all)"
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Filter.DeviceNum.value()" checks
aelovikov-intel
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
aelovikov-intel
changed the title
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
aelovikov-intel
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| (target.DeviceType.value() != info::device_type::all))) { | ||
| // This device was specifically requested and yet is not | ||
| // partitionable. | ||
| std::cout << "device is not partitionable: " << target << std::endl; |
There was a problem hiding this comment.
I understand these and other prints are actually diagnosing errors (e.g. trying to partition a device that is not partitionable). Is my understanding correct? If so, I wonder if just printing and keep going as if nothing happened is good enough as a diagnostic.
There was a problem hiding this comment.
They were here before me, any changes in the behavior should go into a separate PR :)
There was a problem hiding this comment.
Yes, I saw they were already there in the old version, and I agree that it should be a separate PR if we change that, but I wanted to bring up the debate. What do you think? Should we keep diagnostics as they are now? Or should we stop if we find an error?
I'm approving this PR since we agreed this should go into a separate PR, if we do anything about it.
There was a problem hiding this comment.
I honestly would expect an exception thrown, but maybe @cperkinsintel had his reasons to implement it this way (I assume it was added as part of his major work in this area).
The very last commit in that PR (intel@f9a1377) accidentally removed one condition needed to add the device into allowed devices list. Restore it here.
Mostly "early continue" and use the same idioms for similar things.