Skip to content
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

[Testing] Improve "program" testing to better match the DPC++ e2e tests #1503

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

RossBrunton
Copy link
Contributor

This adds a number of conformance tests to test things that are required for the "program" DPC++ e2e tests.

Adapter testing infrastructure for level zero has been added, and a single test has been added to check their specific handling of linker errors (they write it to the build log of the output program).

Other than that, test additions are as follows:

  • A few specialization constant tests to test usage of a kernel with multiple specialization constants defined.
  • Testing of the default specialization values.
  • KernelGetInfo outputs the correct kernel name and context pointer.
  • Compiling an (valid or invalid) program produces a valid (though unspecified) build log.
  • The "num devices" program info is sensible.

@RossBrunton RossBrunton force-pushed the ross/e2e_program branch 11 times, most recently from 8a560e3 to f9883fd Compare April 10, 2024 12:26
@kbenzie kbenzie added the conformance Conformance test suite issues. label Apr 10, 2024
@RossBrunton RossBrunton force-pushed the ross/e2e_program branch 18 times, most recently from 2b1e40d to 5594569 Compare April 11, 2024 12:27
@RossBrunton RossBrunton marked this pull request as ready for review April 17, 2024 17:04
@RossBrunton RossBrunton requested a review from a team as a code owner April 17, 2024 17:04
test/conformance/kernel/urKernelGetInfo.cpp Outdated Show resolved Hide resolved
@@ -1,3 +1,4 @@
urProgramCreateWithILTest.BuildInvalidProgram/Intel_R__OpenCL___{{.*}}_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this based on an e2e test that passes on any platforms? if not it might not be worth testing, none of the adapters that support IL pass and realistically we aren't going to write something to validate IR for each of those adapters just to pass a cts test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it did, but apparently the error was coming from urProgramBuild rather than create. I've added a quick test case for that.

Despite not passing in CI, this test does pass for me locally, so some devices handle it correctly.

If we aren't going to validate IR, should we amend the spec to explicitly call out that it's not required for implementations to validate that the IR is correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it would be good to note that validation doesn't necessarily happen at the ProgramCreate step

test/conformance/program/urProgramGetInfo.cpp Outdated Show resolved Hide resolved
@RossBrunton RossBrunton force-pushed the ross/e2e_program branch 7 times, most recently from 097ec9f to bb2dec8 Compare April 25, 2024 13:58
@github-actions github-actions bot added loader Loader related feature/bug specification Changes or additions to the specification labels Apr 25, 2024
@RossBrunton RossBrunton marked this pull request as draft April 25, 2024 15:40
@RossBrunton RossBrunton force-pushed the ross/e2e_program branch 8 times, most recently from 59731cf to 1b80a22 Compare April 29, 2024 12:22
This adds a number of conformance tests to test things that are
required for the "program" DPC++ e2e tests. Note that these tests
don't all pass.

Adapter testing infrastructure for level zero has been added, and
a single test has been added to check their specific handling of
linker errors (they write it to the build log of the output
program).

Other than that, test additions are as follows:
* A few specialization constant tests to test usage of a kernel
  with multiple specialization constants defined.
* Testing of the default specialization values.
* KernelGetInfo outputs the correct kernel name and context pointer.
* Compiling an (valid or invalid) program produces a valid (though
  unspecified) build log.
* The "num devices" program info is sensible.
@RossBrunton RossBrunton marked this pull request as ready for review April 29, 2024 14:20
@RossBrunton RossBrunton added the ready to merge Added to PR's which are ready to merge label Apr 29, 2024
@RossBrunton RossBrunton merged commit 24f7d66 into oneapi-src:main Apr 29, 2024
51 checks passed
@RossBrunton RossBrunton deleted the ross/e2e_program branch November 6, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/cd Continuous integration/devliery conformance Conformance test suite issues. loader Loader related feature/bug ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants