-
Notifications
You must be signed in to change notification settings - Fork 1k
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
graph: backend: dnnl: encode mem address into constant cache key #2312
base: main
Are you sure you want to change the base?
Conversation
f4499e2
to
ac702db
Compare
ac702db
to
89d402e
Compare
make test |
@@ -43,6 +43,24 @@ bool kernel_base_t::enabled_constant_cache() const { | |||
return enabled; | |||
} | |||
|
|||
size_t kernel_base_t::encode_constant_cache_key( | |||
const std::vector<tensor_t> &inputs, size_t cache_key) const { | |||
// Encode the constant memory address into cache key for differentiation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this, the original constant_key_
is not a cache key anymore. I would suggest to rename these variables as well as the function name of generate_constant_cache_key
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! Rename to const_md_hash_
and generate_constant_md_hash
accordingly, please review again.
470f190
to
28606ed
Compare
make test |
Description
Same compiled partitions may have different constant weights, potential accuracy issue may happens on future op direct optimization integration solutions.
This PR aims to enhance the library constant cache key for better cache or differentiation.
For the future direction of API design, please refer to #2280 for details. We will revisit this RFC once the real user scenario and request pops out.
Performance impact