Reusing of softmax mkldnn primitives #10576
Merged
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
jczaja
force-pushed
the
prv-reuse-mkldnn-softmax-primitives
branch
4 times, most recently
from
b0a5d41
to
4b3b20a
Compare
|
@jczaja please merge the latest code to pass the TeamCity. Thanks very much! |
jczaja
force-pushed
the
prv-reuse-mkldnn-softmax-primitives
branch
2 times, most recently
from
27167c7
to
50e559b
Compare
- Added hash function inside of MKLDNN softmax op to be used as handle for primitives stroing in a context - Style fixes to softmax mkldnn op - Fixes after review - Coding style - Fix to style - style fixes - style fix - style fixes - Fix to cody style check - Rephrasing a comment
jczaja
force-pushed
the
prv-reuse-mkldnn-softmax-primitives
branch
from
50e559b
to
7bf00c3
Compare
|
@luotao1 Thanks very much for a suggesstion. It helped. |
tensor-tang
reviewed
May 14, 2018
| if (softmax_p == nullptr) { | ||
| // Currently only NC data format is supported | ||
| auto softmax_md = | ||
| MKLDNNMemDesc({softmax_tz}, memory::f32, memory::format::nc); |
There was a problem hiding this comment.
f32should depends on T, right?
Maybe this should be enhanced, or at least enforce as float.
There was a problem hiding this comment.
It should be enhanced in next PR, since I find:
BDSHYF000120887:operators luotao02$ grep "memory::f32" *.cc
activation_mkldnn_op.cc: ? platform::MKLDNNMemDesc(src_tz, mkldnn::memory::f32,
activation_mkldnn_op.cc: : platform::MKLDNNMemDesc(src_tz, mkldnn::memory::f32,
activation_mkldnn_op.cc: ? platform::MKLDNNMemDesc(src_tz, mkldnn::memory::f32,
activation_mkldnn_op.cc: : platform::MKLDNNMemDesc(src_tz, mkldnn::memory::f32,
pool_mkldnn_op.cc: auto src_md = platform::MKLDNNMemDesc(src_tz, mkldnn::memory::f32,
pool_mkldnn_op.cc: auto dst_md = platform::MKLDNNMemDesc(dst_tz, mkldnn::memory::f32,
pool_mkldnn_op.cc: {{}, mkldnn::memory::f32, mkldnn::memory::format::nchw},
pool_mkldnn_op.cc: auto diff_src_md = platform::MKLDNNMemDesc(diff_src_tz, mkldnn::memory::f32,
pool_mkldnn_op.cc: auto diff_dst_md = platform::MKLDNNMemDesc(diff_dst_tz, mkldnn::memory::f32,
softmax_mkldnn_op.cc: MKLDNNMemDesc({softmax_tz}, memory::f32, memory::format::nc);
luotao1
approved these changes
May 14, 2018
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.
Changes presented here are introducing concept of reusing once created MKLDNN primitives for softmax for specific scenario eg. when we have softmax MKLDNN op called for the second time with
same dims of input/output then we can reuse previously created objects rather than recreate them.
Intention of this change is to speedup Softmax MKLDNN operator (more MKLDNN ops will follow)