[SYCL][PI][OPENCL] Testing for eliminate usage of regex in opencl #11204
Conversation
sycl/plugins/opencl/pi_opencl.hpp
Outdated
| bool haveOCLPrefix = | ||
| std::equal(prefix.begin(), prefix.end(), version.begin()); | ||
|
|
||
| if (haveOCLPrefix && versionBegin != std::string::npos && |
There was a problem hiding this comment.
I wonder if there is any situation where this is expected to fail at runtime? If not, we could maybe turn this if condition into an assertion
There was a problem hiding this comment.
From what I found of how this function was used. Is that if an invalid string version was passed, it will keep the ocl_major - ocl_minor with a 0 value, which isValid function will return false in that case. and how I saw this propagating up. Is that it will return a relevant cl error code associated with the called function like CL_INVALID_PLATFORM in getPlatformVersion. I think the current way will give more information of where the faulty version string was passed from, but asserting will bail out the program without giving this information. however, I think this is more still of an assertion situation, but not sure if the assertion way could make some resources not being deleted appropriately or this cl error code are more of expected thing in the source code in another code place that depend on it.
There was a problem hiding this comment.
Sorry, I forgot to answer this. What you say makes sense. I think it's fine to keep it as is 👍
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
3 times, most recently
from
e250134
to
59d5409
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
from
59d5409
to
291b945
Compare
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
from
291b945
to
47bf07d
Compare
|
Moved the functionality to UR repo here |
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
from
47bf07d
to
b293909
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
changed the title
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
from
b293909
to
d325624
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
from
d325624
to
f579f35
Compare
omarahmed1111
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
omarahmed1111
force-pushed
the
eliminate-usage-of-regex-in-opencl
branch
from
f579f35
to
c2c1396
Compare
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
omarahmed1111
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
I'll be combining this change with some others from UR in a separate PR, closing. |
DO NOT MERGE