src: cpu: aarch64: lowp_matmul: Make weights constant #2194
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Setting the weights as constant allows us to avoid redundant pretranspose and reduction operations in Arm Compute Library (ACL) every time execute is called (they are now run once and cached). This delives big speedups especially for relatively small matmuls. Note that this is a temp fix that needs to be handled carefully by primitive caches in frameworks, since the ACL object is now holding more state - i.e. we want to make sure that the cahce maps a layer with a specific set of weights to the oneDNN primitive storing those weights. We're currently working on the proper fix for this which involves making lowp_gemm stateless and fixed-format in ACL and oneDNN.
fadara01
added a commit
to fadara01/ideep
that referenced
this pull request
Oct 30, 2024
… lowp gemm This goes in hand with oneapi-src/oneDNN#2194 which sets the weights as constant for Arm Compute Library (ACL) lowp gemm objects. This is a temp fix which is needed because the lowp gemm ACL object in oneDNN is now holding more state. Hence, we need to make sure that the cahce maps a layer with a specific set of weights to the oneDNN primitive storing those weights. We're currently working on the proper fix for this which involves making lowp_gemm stateless and fixed-format in ACL and oneDNN.
mgouicem
reviewed
Nov 5, 2024
| @@ -121,7 +121,7 @@ status_t acl_lowp_matmul_t::pd_t::init(engine_t *engine) { | |||
| = arm_compute::TensorInfo(arm_compute::TensorShape(N(), K()), 1, | |||
| arm_compute::DataType::QASYMM8_SIGNED, | |||
| arm_compute::QuantizationInfo(1.0, 0, true)); | |||
| almc_.wei_tensor_info.set_are_values_constant(false); | |||
| almc_.wei_tensor_info.set_are_values_constant(true); | |||
There was a problem hiding this comment.
does ACL assumes the shapes are constant, or the actual values in the tensor are constant?
In general, oneDNN does not have a concept of constant memory objects, so assuming values in the memory object are constant is unsafe.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Setting the weights as constant allows us to avoid redundant pretranspose and reduction operations in Arm Compute Library (ACL) every time execute is called (they are now run once and cached). This delives big speedups especially for relatively small matmuls.
Note that this is a temp fix that needs to be handled carefully by primitive caches in frameworks, since the ACL object is now holding more state - i.e. we want to make sure that the cahce maps a layer with a specific set of weights to the oneDNN primitive storing those weights.
We're currently working on the proper fix for this which involves making lowp_gemm stateless and fixed-format in ACL and oneDNN.
Fixes # (github issue)
Checklist
General
make testandmake test_benchdnn_*) pass locally for each commit?Performance improvements
New features
Bug fixes
RFC PR