-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL][Doc] Add test plan for sycl_ext_intel_kernel_queries extension #17593
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| # Test plan for [`sycl ext intel kernel queries`][spec-link] extension | ||
|
|
||
| ## Testing scope | ||
|
|
||
| ### Device coverage | ||
|
|
||
| The unit tests should be launched on every supported device configuration we | ||
| have. | ||
|
|
||
| ### Type coverage | ||
|
|
||
| Each query is templated on a single template type argument `Param` | ||
| with some queries being template overloads of each other. | ||
| Each query has a restriction on the value of `Param` defined | ||
| in the relevant section of the spec. | ||
| Each `Param` defines a related sycl aspect that signals whether the device | ||
| supports it. | ||
|
|
||
| Param must be one of the following types defined in | ||
| `sycl::ext::intel::info::kernel_device_specific` namespace: | ||
| - `spill_memory_size` | ||
|
|
||
| The tests should cover all of these types. | ||
|
|
||
| ## Tests | ||
|
|
||
| ### Unit tests | ||
|
|
||
| #### Interface tests | ||
|
|
||
| These tests are intended to check that all classes and methods defined by the | ||
| extension have correct implementation, i.e.: right signatures, right return | ||
| types, all necessary constraints are checked/enforced, etc. | ||
|
|
||
| These tests should check the following: | ||
|
|
||
| - that each query is not available when the template argument `Param` has | ||
| a value different than the one in the spec. | ||
| - that each query can be called with the appropriate value for `Param` and the | ||
| appropriate argument types as defined by its signature. | ||
| - the return types of all queries match the spec. | ||
|
|
||
| Tests in this category may not perform some useful actions to exercise the | ||
| extension functionality in full, but instead they are focused on making sure | ||
| that all APIs are consistent with respect to other APIs. | ||
|
|
||
| #### Check behavior in the case of unsupported aspects | ||
|
|
||
| Verigy that a synchronous `exception` with the error code | ||
| `errc::feature_not_supported` is thrown if an aspect is not supported by the | ||
| device. | ||
|
|
||
| [spec-link]: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/supported/sycl_ext_intel_kernel_queries.asciidoc | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Sorry for delayed review. Test plan indicates that unit tests are going to be added (i.e. https://github.com/intel/llvm/tree/sycl/sycl/unittests) but corresponding PR - #17594 - actually adds e2e lit test, not a unit test.
So, please change the wording in the test plan.
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 recently discovered that that test won't be of any use. And if I add the requirement on the aspect the test would be actually dropped. I'll try to add a unittest this week that'd use mock ur.
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.
That sounds good, in that case this test plan looks good, but should it be moved to unittests directory?
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.
Sure, done