-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
MKLDNN layout: Support for pool operator #11101
MKLDNN layout: Support for pool operator #11101
Conversation
c6bf91f
to
36031cb
Compare
@@ -94,16 +103,17 @@ class PoolMKLDNNOpKernel : public paddle::framework::OpKernel<T> { | |||
auto pool_p = | |||
std::static_pointer_cast<pooling_forward>(dev_ctx.GetBlob(key_pool_p)); | |||
if (pool_p == nullptr) { | |||
// TODO(pzelazko-intel): support more formats | |||
auto src_md = platform::MKLDNNMemDesc( | |||
src_tz, platform::MKLDNNGetDataType<T>(), input_format); |
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.
input->format()
.
input_format is only used once.
@@ -267,35 +302,74 @@ class PoolMKLDNNGradOpKernel : public paddle::framework::OpKernel<T> { | |||
auto pool_bwd_pd = mkldnn::pooling_backward::primitive_desc( | |||
pool_bwd_desc, mkldnn_engine, *pool_pd); | |||
|
|||
// reorder between user_diff_dst and pool diff_dst if needed | |||
diff_dst_memory = std::make_shared<memory>(user_diff_dst_memory); |
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.
We recommend unique_ptr
, maybe next time you can change all of them.
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.
LGTM
The comments can be addressed next time.
Waiting for supports of the mkldnn’s layout.
Please have a look at this pool operator supported by the MKLDNN’s layout and assess if we can keep this design of the code.
This code uses the implementation of layout which is available in the last pull-request. Therefore some of the function in this code are not available in this pull-request, but are available here
This version of code can be merged into the main branch when the pull-request with layout is accepted.
The concept of splits of the long code was proposed by @luotao1
Pull-request is related to #11040