-
Notifications
You must be signed in to change notification settings - Fork 788
[SYCL][CUDA] Allow joint_matrix to be loaded from const T #6532
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
Conversation
Signed-off-by: JackAKirk <[email protected]>
Signed-off-by: JackAKirk <[email protected]>
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.
LGTM
btw the CUDA test failure is the now XFAIL'ed known flaky test Assert/assert_in_simultaneously_multiple_tus.cpp |
ping @v-klochkov |
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.
Looks good to me. Thank you.
Before proceeding to merge. Please add a test verifying the test case being fixed by this PR.
If there is an existing test, but it is XFAIL due to being flaky, please add a simpler test or a compile-only test.
/verify with intel/llvm-test-suite#1280 |
/verify with intel/llvm-test-suite#1280 |
Thanks. I've added all new possible use cases in the test here: intel/llvm-test-suite#1280 |
/verify with intel/llvm-test-suite#1280 |
Can this be merge now? |
This tests new cases made possible in intel/llvm#6532, such that A,B, Accumulator accessors that are loaded to registers in joint_matrix can be of type const T.
…st-suite#1280) This tests new cases made possible in intel#6532, such that A,B, Accumulator accessors that are loaded to registers in joint_matrix can be of type const T.
Fixes a bug where if
joint_matrix_load
attempts to loadjoint_matrix
from an array ofconst T
incorrect behaviour will occur or an error will be thrown. To fix this we make use ofstd::remove_const_t<T>
in appropriate places. This is important functionality for integrating joint_matrix with existing SYCL-DNN routines.I think that similar problems might occur in the intel backends for their existing impl: I have not made corresponding changes because I do not have the hardware to test it.
Signed-off-by: JackAKirk [email protected]