Skip to content

[SYCL][JM] Support different types for the store#21398

Draft
dkhaldi wants to merge 4 commits intointel:syclfrom
dkhaldi:store-types
Draft

[SYCL][JM] Support different types for the store#21398
dkhaldi wants to merge 4 commits intointel:syclfrom
dkhaldi:store-types

Conversation

@dkhaldi
Copy link
Copy Markdown
Contributor

@dkhaldi dkhaldi commented Feb 27, 2026

Complete the implementation of joint_matrix_store as in the spec, types of pointer and matrix can be different. See https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/experimental/sycl_ext_matrix/sycl_ext_oneapi_matrix.asciidoc#store
But in current implementation, we only support one type for both.
This PR adds this distinction.

@dkhaldi dkhaldi marked this pull request as ready for review March 10, 2026 20:59
@dkhaldi dkhaldi requested a review from a team as a code owner March 10, 2026 20:59
Copy link
Copy Markdown
Contributor

@YixingZhang007 YixingZhang007 Mar 11, 2026

Choose a reason for hiding this comment

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

I am not sure if we should cast A to be float here, because A is Tp*, after we cast it, pA will be Tjm* (since Tjm is float). Then, later in the test, the matrix being passed into joint_matrix_store, as well as the pointer will have the same type which are both float* (but I thought they should have different types).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch.
It turns out the store from float to half is not working properly. That cast that I missed was masking the bug.
I am setting the test to XFAIL

@YixingZhang007
Copy link
Copy Markdown
Contributor

Thank you so much for making the change! I have added some suggestion :)

Copy link
Copy Markdown
Contributor

@YixingZhang007 YixingZhang007 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the PR!

@dkhaldi dkhaldi marked this pull request as draft March 24, 2026 18:43
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.

2 participants