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

[SYCL] [Joint Matrix] Modularized tests #13730

Merged
merged 7 commits into from May 14, 2024

Conversation

artemrad
Copy link
Contributor

@artemrad artemrad commented May 9, 2024

This is part 1 of 2 to improve reuse of common functions in the Joint Matrix test set. This changes focuses on removing functions, aliases and types from individual tests and relying on them being present in common.hpp.

@artemrad artemrad requested a review from a team as a code owner May 9, 2024 17:06
Copy link
Contributor

@YuriPlyakhin YuriPlyakhin left a comment

Choose a reason for hiding this comment

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

I noticed that tests in XMX8 folder were not changed, while some .hpp files which are used for XMX8 were changed. Most likely it will cause compilation failures for XMX8 tests. Please, update tests in that folder as well accordingly.

Also other comments need to be addressed.

Copy link
Contributor

@YuriPlyakhin YuriPlyakhin left a comment

Choose a reason for hiding this comment

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

2 tests specific for CPU are not running in pre-check-in testing. Could you run them locally to verify your changes or remove these 2 tests from PR?

@dkhaldi
Copy link
Contributor

dkhaldi commented May 13, 2024

Why did you remove the changes from the CPU tests?
Remember, at the end of the enhancements you are making to the tests, there will be no CPU vs GPU tests really as the runtime query will take care of the right dispatch. So these tests should not really be treated differently.

@artemrad
Copy link
Contributor Author

@dkhaldi - I removed the changes to the CPU tests because it was not trivial to convert the CPU centric matrix_multiply_ref() to common.hpp:matrix_multiply_ref(). These tests can be updated and aligned with common.hpp with a future change

@artemrad
Copy link
Contributor Author

@intel/llvm-gatekeepers Ready to merge

@steffenlarsen steffenlarsen merged commit 9beb70b into intel:sycl May 14, 2024
13 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.

None yet

4 participants