cpu: aarch64: add brgemm bwd data support for block size 8 and 16 #2865
Conversation
github-actions
bot
added
the
platform:cpu-aarch64
src/cpu/aarch64/jit_brgemm_conv.cpp
Outdated
| template struct brgemm_convolution_fwd_t<sve_256>; | ||
| template struct brgemm_convolution_fwd_t<sve_256, true>; |
There was a problem hiding this comment.
use_inversion was moved to convolution op_desc some time ago. I would highly appreciate if you replace a templated argument of the implementation with a code relying on a member of the op_desc.
There was a problem hiding this comment.
@dzarukin Thank you for the suggestion. I haven’t worked much on this file yet, so I’d prefer to keep the current implementation as is for now since the PR works without modifying use_inversion. Also, updating use_inversion would require changes in other files, which we can address as a future task. Let me know if that works!
There was a problem hiding this comment.
Also, updating use_inversion would require changes in other files
I don't see it used in other files. The further it delayed the harder it's going to remove it. I don't see why would it take too long to make the code better. Thanks.
There was a problem hiding this comment.
@dzarukin Thank you for the feedback. I've made the changes now — you can review them at your convenience. Let me know if you have any further suggestions.
rpushkarr
force-pushed
the
jit_aarch64_brgemm_bwd_data
branch
from
cb520db to
92ce79e
Compare
|
|
||
| brgemm_convolution_bwd_t(const pd_t *apd) : primitive_t(apd) {}; | ||
|
|
||
| ~brgemm_convolution_bwd_t() = default; |
There was a problem hiding this comment.
| ~brgemm_convolution_bwd_t() = default; | |
| ~brgemm_convolution_bwd_t() override = default; |
| @@ -0,0 +1,80 @@ | |||
| /******************************************************************************* | |||
| * Copyright 2022-2024 Intel Corporation | |||
There was a problem hiding this comment.
Intel copyright is not needed here and for the new file above.
| *******************************************************************************/ | ||
|
|
||
| #include "common/c_types_map.hpp" | ||
| #include "common/compiler_workarounds.hpp" |
There was a problem hiding this comment.
This one shouldn't be needed here.
rpushkarr
force-pushed
the
jit_aarch64_brgemm_bwd_data
branch
from
0b7d56a to
a892566
Compare
|
@dzarukin Thank you for the approval! I've updated the files/code based on your suggestion |
|
@Radu2k Could you please review this PR at your earliest convenience? |
|
@dzarukin Thanks for the approval! I've made the updates based on your suggestions. Could you please re-review and approve the PR? |
| #include "cpu/cpu_convolution_pd.hpp" | ||
|
|
||
| #include "cpu/aarch64/jit_brgemm_1x1_conv.hpp" | ||
| #include "cpu/aarch64/jit_brgemm_conv.hpp" |
There was a problem hiding this comment.
Nit: seems these two can move to cpp-file instead.
| *******************************************************************************/ | ||
|
|
||
| #ifndef CPU_X64_JIT_BRGEMM_CONV_BWD_HPP | ||
| #define CPU_X64_JIT_BRGEMM_CONV_BWD_HPP |
There was a problem hiding this comment.
| #define CPU_X64_JIT_BRGEMM_CONV_BWD_HPP | |
| #define CPU_AARCH64_JIT_BRGEMM_CONV_BWD_HPP |
| break; // non-1x1 implementation found | ||
| } | ||
| } | ||
| if (it == it.end()) { return status::unimplemented; } |
There was a problem hiding this comment.
| if (it == it.end()) { return status::unimplemented; } | |
| VDISPATCH_CONV(it != it.end(), "Implementation wasn't found"); |
rpushkarr
force-pushed
the
jit_aarch64_brgemm_bwd_data
branch
from
53de57f to
3e6e67b
Compare
|
@dzarukin I've updated all the changes as requested. Please review and proceed with the merge if there are no further issues. Thank you! |
Description
This pull request adds support for BRGEMM backward data (BWD) on CPU for the AArch64 architecture, specifically for block sizes 8 and 16. The following changes have been made to enable this support:
New Files Added:
jit_brgemm_bwd_data.cpp – Implements the JIT kernel for BRGEMM backward data.
jit_brgemm_bwd_data.hpp – Defines the interface and data structures for the BRGEMM backward data kernel.
Template Changes:
The brgemm_convolution_fwd_t template has been updated to support inversion by adding a boolean parameter.
Checklist
General
make testandmake test_benchdnn_*) pass locally for each commit?