Skip to content

[SYCL][RTC] Add device library E2E test #17131

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 7 commits into from
Mar 5, 2025

Conversation

jopperm
Copy link
Contributor

@jopperm jopperm commented Feb 24, 2025

New E2E test to exercise use of device libraries in an RTC kernel. Currently, the libraries are JIT-linked, i.e. their use is encoded in the SYCL/devicelib req mask property set, and the program manager loads and links the library.

@jopperm jopperm requested a review from a team February 24, 2025 08:27
@jopperm jopperm self-assigned this Feb 24, 2025
@jopperm jopperm requested a review from a team as a code owner February 24, 2025 08:27
@jopperm
Copy link
Contributor Author

jopperm commented Feb 24, 2025

I plan to add tests for the cl_intel_devicelib_imf_bf16 and cl_intel_devicelib_bfloat16 libraries after #16729 lands and we have adapted the RTC pipeline to follow the new approach.

// Only test the fp32 variants of complex, math and imf to keep this test
// device-agnostic.

// cl_intel_devicelib_math
Copy link
Contributor

Choose a reason for hiding this comment

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

since ptr is {1, 1, 1, 1} it'd be nice to leave a comment here for each stating the expected value ( 0.8 1.4, 0, 1.4 right?)

I mention this because the test is failing on Windows with an 0xc0000409 error code, which is an assert() failure. AFAICT, the only assert added by this PR is the none of the new values of ptr[] are 1.0f.

Not sure what's going on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I added a comment and now print out the values. Curious to see what happens on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the end this was just a compilation error, due to the specific flavour of complex data type not being supported on Windows. I changed the test to use std::complex instead. The reason why we didn't see this in the CI logs is that the default handler for uncaught exceptions doesn't seem to print anything on Windows 🤔

Signed-off-by: Julian Oppermann <[email protected]>
Signed-off-by: Julian Oppermann <[email protected]>
@jopperm
Copy link
Contributor Author

jopperm commented Mar 3, 2025

@cperkinsintel Could you have a second look, please?

@jopperm
Copy link
Contributor Author

jopperm commented Mar 5, 2025

@intel/llvm-gatekeepers Please merge, thanks!

@uditagarwal97 uditagarwal97 merged commit 7276c55 into intel:sycl Mar 5, 2025
21 checks passed
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.

4 participants