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 7 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
Comment on lines -462 to -464
~cudnn_matmul_impl_t() {
if (matmul_params_) { matmul_params_->cleanup(); }
}
Copy link

Choose a reason for hiding this comment

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

Could you explain what are the consequences of this change? Are we sure matmul_params_ is cleaned up elsewhere or does it not need to?

@@ -717,7 +717,7 @@ struct cudnn_matmul_lt_impl_t {
}
if (!params->w_blocked_) {
transform_matrix(lt_handle, params, a_layout, a,
blocked_a_layout, block_a_scratch, !params->trans_a_,
blocked_a_layout, block_a_scratch, params->trans_a_,
Copy link

Choose a reason for hiding this comment

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

What is the impact of this change? Was there a bug?

@@ -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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants