Added object comparability verification for CL_DEVICE_PLATFORM query #2225
Conversation
| error = clGetDeviceIDs(platforms[p], CL_DEVICE_TYPE_ALL, 0, nullptr, | ||
| &num_devices); | ||
| test_error(error, "clGetDeviceIDs failed"); | ||
|
|
||
| std::vector<cl_device_id> devices(num_devices); | ||
| error = clGetDeviceIDs(platforms[p], CL_DEVICE_TYPE_ALL, num_devices, | ||
| devices.data(), nullptr); | ||
| test_error(error, "clGetDeviceIDs failed"); |
There was a problem hiding this comment.
I think we should ignore platforms where these queries return an error vs. failing the test. I know this is atypical and not the pattern used by most tests, but I think it is the right thing to do here for two reasons:
- If we fail the test when these queries return an error, it means that a buggy platform that just so happens to be installed on the system alongside the platform being tested could cause the test to fail, even if the platform being tested is correct and conformant.
clGetDeviceIDsis a little weird in that it is specified to return an error when no devices are found rather than returningnum_deviceequal to zero. This means that a conformant platform could return an error, which would also cause the test to fail. Linking in: remove unavoidable error conditions OpenCL-Docs#1269
Since this test is run through the harness, we could also consider using the CL_PLATFORM_INDEX environment variable to choose the platform to test against, though this loop is fine too, and it has the added benefit of testing that clGetDeviceIDs returns the same set of device IDs if it is called multiple times.
| devices.data(), nullptr); | ||
| test_error(error, "clGetDeviceIDs failed"); | ||
|
|
||
| // try to find invalid device |
There was a problem hiding this comment.
Nitpick: I don't think this is an "invalid device" that we are looking for.
| comp_platform = platforms[p]; | ||
| break; |
There was a problem hiding this comment.
Should we fail the test if no comp_platform is found, basically if we get out of this loop and comp_platform is still nullptr?
Fixes #1485 according to work plan, point CL_DEVICE_PLATFORM