-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add a unified SVM consistency check test #2174
base: cl_khr_unified_svm
Are you sure you want to change the base?
add a unified SVM consistency check test #2174
Conversation
Hmm, the CI build checks are going to fail unless we update the scripts to pull the headers from KhronosGroup/OpenCL-Headers#269. |
I've updated the test as we discussed. I also pulled in the CI changes so the unified SVM headers are used, and now the automated builds are working again. |
|
||
REGISTER_TEST(unified_svm_consistency) | ||
{ | ||
if (!is_extension_available(deviceID, "cl_khr_unified_svm")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not been following the unified svm test plan closely, but is the idea that the extension tests will live in test_conformance/SVM
rather than test_conformance/extensions/cl_khr_unified_svm
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I don't think we've discussed this. I'll create a new test executable if that's what the group prefers, but I think we've generally tried to minimize test executables, so I'm inclined to put the unified SVM tests here unless we have a good reason to do otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I may have found a few good reasons to create a new test executable:
-
The "SVM" tests run an
InitCL
function to decide whether to run the tests, which currently requires OpenCL 2.0 or newer, and requires that the query forCL_DEVICE_SVM_CAPABILITIES
returns a non-zero value. The first part is debatably OK, though the extension is currently written to support OpenCL 1.2 devices also. The second part is not OK though, because a device can support unified SVM without supporting SVM. -
The "SVM" tests run with
forceNoContextCreation
set totrue
, which means no context or queue are created. Not the end of the world, but I think the harness context and queue will be sufficient for most of the unified SVM tests, so it'd be nice to keep things simple and use them.
Adds a unified SVM consistency check test, as per the description in #2150.
Note that the target branch for this PR is the cl_khr_unified_svm branch, not the main branch.