-
Notifications
You must be signed in to change notification settings - Fork 787
[Joint Matrix] Enable different accumulator and output types in spirv. Add tests to cover bfloat16 and half floating-point sizes. #17502
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
base: sycl
Are you sure you want to change the base?
Conversation
f5e46ba
to
3a5de5a
Compare
3a5de5a
to
c5be2c5
Compare
c5be2c5
to
29c35ac
Compare
29c35ac
to
8116625
Compare
8116625
to
fc7eaf7
Compare
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.
Overall LGTM. Note few comments from Yuri that were resolved, but seem to be not applied (at least in the PR)
@@ -54,7 +54,7 @@ | |||
// tests to match the required format and in that case you should just update | |||
// (i.e. reduce) the number and the list below. | |||
// | |||
// NUMBER-OF-UNSUPPORTED-WITHOUT-INFO: 248 | |||
// NUMBER-OF-UNSUPPORTED-WITHOUT-INFO: 249 |
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.
nit: Can we add sycl/test-e2e/Matrix/joint_matrix_half.cpp to the unsupported tests below?
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.
sycl/test-e2e/Matrix/joint_matrix_half.cpp There is patch https://github.com/intel-innersource/drivers.gpu.unified/pull/195521 where support for half and bfloat16 accumulator/output is added to IGC. Do we want to mark it as unsupported untill it is merged or just xfail?
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.
Well, may be leaving as is is fine, @KornevNikita WDYT?
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 posted a suggestion below.
Co-authored-by: Nikita Kornev <nikita.kornev@intel.com>
Add JIRA trackers
Add JIRA trackers
Remove tests from unsupported without info check file
Updated matrix_compare and matrix_multiply_ref functions to better match bfloat16 calculations on device.