Skip to content

add a unified SVM consistency check test#2174

Merged
bashbaug merged 4 commits into
KhronosGroup:cl_khr_unified_svmfrom
bashbaug:unified-svm-consistency
Feb 11, 2025
Merged

add a unified SVM consistency check test#2174
bashbaug merged 4 commits into
KhronosGroup:cl_khr_unified_svmfrom
bashbaug:unified-svm-consistency

Conversation

@bashbaug
Copy link
Copy Markdown
Contributor

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.

@bashbaug
Copy link
Copy Markdown
Contributor Author

Hmm, the CI build checks are going to fail unless we update the scripts to pull the headers from KhronosGroup/OpenCL-Headers#269.

@bashbaug
Copy link
Copy Markdown
Contributor Author

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"))
Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor Author

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:

  1. 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 for CL_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.

  2. The "SVM" tests run with forceNoContextCreation set to true, 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.

@bashbaug
Copy link
Copy Markdown
Contributor Author

Merging (to the cl_khr_unified_svm branch), as discussed in the February 11th memory TSG.

@bashbaug bashbaug merged commit 9123b05 into KhronosGroup:cl_khr_unified_svm Feb 11, 2025
@bashbaug bashbaug deleted the unified-svm-consistency branch February 11, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants