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

generic: sycl: Inner Product FWD #2214

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AD2605
Copy link

@AD2605 AD2605 commented Nov 14, 2024

Description

Implements the forward pass of the inner product for the generic sycl backend with support for different post-ops and data types

@AD2605 AD2605 requested review from a team as code owners November 14, 2024 15:03
@AD2605 AD2605 changed the title [Generic]] [Generic][SYCL] Inner Product FWD Nov 14, 2024
@AD2605 AD2605 changed the title [Generic][SYCL] Inner Product FWD generic: sycl: Inner Product FWD Nov 14, 2024
@github-actions github-actions bot added platform:gpu-nvidia Codeowner: @oneapi-src/onednn-gpu-nvidia platform:gpu-generic Codeowner: @oneapi-src/onednn-gpu-generic labels Nov 14, 2024
src/gpu/nvidia/cudnn_matmul_impl.hpp Outdated Show resolved Hide resolved
src/gpu/nvidia/cudnn_matmul_lt_impl.hpp Show resolved Hide resolved
@@ -392,16 +392,16 @@ struct cudnn_matmul_lt_exec_t final : public cudnn_matmul_lt_base_exec_t {
memory_tracking::names::key_matmul_dst_in_acc_dt)
: xpu::sycl::interop_memory_arg_t<
::sycl::access::mode::read_write>();
auto arg_block_a_scratch = params->source_size_ != 0
Copy link

Choose a reason for hiding this comment

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

This looks like the changes in this file are fixing bugs? If so, should we consider adding more tests to catch them?

Copy link
Author

@AD2605 AD2605 Nov 18, 2024

Choose a reason for hiding this comment

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

Yes.

If so, should we consider adding more tests to catch them?

From what I can tell, this file should be covering all the possible combinations, and I do see the case for which this was failing (i.e A = S8, ab format, B = S8, ba format, and C = S32, ab format), but maybe others with a better understanding could chime in, maybe @ShanoToni ?

Edit: Link to the aforementioned file - https://github.com/oneapi-src/oneDNN/blob/main/tests/benchdnn/inputs/matmul/test_matmul_int8

@sgeor255
Copy link
Contributor

@AD2605 you should probably update https://github.com/oneapi-src/oneDNN/blob/main/src/gpu/generic/sycl/README.md to mention inner product is supported :)

@AD2605 AD2605 requested a review from a team as a code owner November 19, 2024 15:36
@AD2605
Copy link
Author

AD2605 commented Nov 19, 2024

@sgeor255 , updated the documentation

@github-actions github-actions bot added the documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc platform:gpu-generic Codeowner: @oneapi-src/onednn-gpu-generic platform:gpu-nvidia Codeowner: @oneapi-src/onednn-gpu-nvidia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants