[SYCL][Matrix spec] keep deletion of assign op and copy ctor but change signature of joint_matrix_mad

[dkhaldi]

@gmlueck, in #7964, we explicitly deleted the copy ctor and assign op because we added joint_matrix_copy that is used to actually copy matrix data.
However, when deleting them in implementation, failures occur as joint_matrix_mad uses them.
This PR proposes to change the signature of joint_matrix_mad so these ctors are not used.

[dkhaldi]

gentle ping @gmlueck

[gmlueck]

Why do we need to say that the copy constructor and assignment operator don't copy the matrix content? How would the application even be able to tell whether the matrix content was copied? Isn't the content of the matrix hidden behind this abstract joint_matrix type?

I'm thinking now that we could replace joint_matrix_copy with an assignment operator that allows the user to copy from a matrix with different Use and T (type). That way, matrix copy operations will always be done via the assignment operator. That seems clearer than having both an assignment operator and a joint_matrix_copy function.

[dkhaldi]

@gmlueck, if we have the assignment op do the actual data copy, = joint_matrix_mad that uses assignment op would do extra unnecessary copy of the data (

llvm/sycl/include/sycl/ext/oneapi/matrix/matrix-unified.hpp

Line 418 in faec54c

joint_matrix_mad(

)

[gmlueck]

It seems like neither joint_matrix_copy nor the assignment operator should copy anything to/from memory. The only memory operations are joint_matrix_load and joint_matrix_store. It seems like both joint_matrix_copy and the assignment operator should both just copy some "handle" that represents the register(s) that hold the matrix content?

In your implementation today, what is the difference between the implementation of joint_matrix_copy and the copy operator?

Here's another way to think about it. Consider a code snippet like this:

joint_matrix<sub_group, int8_t, use::a, Rows, Cols, layout::row_major> m1;
joint_matrix<sub_group, int8_t, use::a, Rows, Cols, layout::row_major> m2;
joint_matrix_load(sg, m1, ptr1, Rows);  // Load "m1" with data from "ptr1"
m2 = m1;
joint_matrix_load(sg, m1, ptr2, Rows);  // Load "m1" with data from "ptr2"

I think a user would expect the matrix m2 to still contain the data loaded from ptr1 after the end of that code snippet. Isn't this the same semantic as joint_matrix_copy?

[dkhaldi]

@gmlueck, can you please review?
This capture what we decided to do:

• keep deletion of copy ctor and assign op
• change signature of joint_matrix_mad by having D as input

I also did some cleanup: replace deprecated multi_ptr() with address_space_cast, replace one instance of get_wi_data (left by accident) by joint_matrix_apply.

[gmlueck]

The changes look good, but the description of the PR is outdated:

@gmlueck, in #7964, we explicitly deleted the copy ctor and assign op because we added joint_matrix_copy that is used to actually copy matrix data.
However, when deleting them in implementation, failures occur as joint_matrix_mad uses them.
This PR proposes to keep them as implemented currently in https://github.com/intel/llvm/blob/sycl/sycl/include/sycl/ext/oneapi/matrix/matrix-unified.hpp#L68
and add clarification that they don't copy matrix data.

Can you update the description, so the merged commit accurately describes what happened?

[dkhaldi]

The changes look good, but the description of the PR is outdated:

@gmlueck, in #7964, we explicitly deleted the copy ctor and assign op because we added joint_matrix_copy that is used to actually copy matrix data.
However, when deleting them in implementation, failures occur as joint_matrix_mad uses them.
This PR proposes to keep them as implemented currently in https://github.com/intel/llvm/blob/sycl/sycl/include/sycl/ext/oneapi/matrix/matrix-unified.hpp#L68
and add clarification that they don't copy matrix data.

Can you update the description, so the merged commit accurately describes what happened?

Done

[dkhaldi]

@intel/llvm-gatekeepers, can you please merge this?