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

cpu: aarch64: matmul: Move allocation of temporary tensors to scratchpad in acl_matmul #1935

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

annop-w
Copy link
Contributor

@annop-w annop-w commented May 29, 2024

Description

Introduce 3 new scrathpad memory key names.

Checklist

General

  • Do all unit and benchdnn tests (make test and make test_benchdnn_*) pass locally for each commit?
  • Have you formatted the code using clang-format?

src/cpu/aarch64/matmul/acl_matmul_utils.hpp Outdated Show resolved Hide resolved
@mgouicem
Copy link
Contributor

tagging @snadampal as this relates to #1470

@vpirogov
Copy link
Member

+@snadampal, could you please help reviewing these changes?

@annop-w, please resolve merge conflict.

…pad in acl_matmul

Introduce 3 new scrathpad memory key names.
@annop-w
Copy link
Contributor Author

annop-w commented Jun 24, 2024

@annop-w, please resolve merge conflict.

Done.

@snadampal
Copy link
Contributor

glad to see this change finally coming.
Hi @annop-w to understand the change a bit in detail, the scratchpad buffers for src and weights are the same buffers allocated in the framework, right . for example, by ideep in PyTorch and by mkldnn wrapper in TensorFlow?

thanks @mgouicem and @vpirogov for tagging me here.

@annop-w
Copy link
Contributor Author

annop-w commented Jun 24, 2024

@snadampal I am not sure how the scratchpads are currently managed in ideep (or Tensorflow), but the idea here is to allow for users (i.e. PyTorch or TF) a chance to decide that, isn't it ? For example, PyTorch can now choose to allocate the same buffer for both src and wei, if sensible, which was not possible before. Does this help ?

@snadampal
Copy link
Contributor

snadampal commented Jun 24, 2024

i mean reusing the buffer allocated in PT or TF via oneDNN user mode scratchpad, you clarified it, thanks.
this change LGTM.

@annop-w
Copy link
Contributor Author

annop-w commented Jun 24, 2024

@snadampal Ah, yes, in that case, you're absolutely right. Thanks for the review.

@vpirogov vpirogov merged commit 6f14365 into oneapi-src:main Jun 25, 2024
6 of 10 checks passed
@vpirogov
Copy link
Member

Awesome. Thanks, @snadampal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants