update support MKLDNN BN conditions #15870
Conversation
|
Thanks for your contribution. Could you add a test case to cover the specific size of 8? |
| @@ -420,8 +419,7 @@ void BatchNormGradComputeExCPU(const nnvm::NodeAttrs &attrs, | |||
|
|
|||
| mxnet::TShape shape = inputs[0].shape(); | |||
| // MKLDNN batchnorm only works well on the special MKLDNN layout. | |||
| if (SupportMKLDNNBN(inputs[0], param) | |||
| && (inputs[3].IsMKLDNNData() || inputs[0].IsMKLDNNData())) { | |||
| if (SupportMKLDNNBN(inputs[0], param)) { | |||
There was a problem hiding this comment.
why we don't need to check these two conditions?
There was a problem hiding this comment.
It's the same reason as line 399, as described in Performance part, when a model has fp32 bn along with other int8 ops, it triggers the IsMKLDNNData() check and cannot run int MKLDNN, which cause a slower speed.
| @@ -396,7 +395,7 @@ void BatchNormComputeExCPU(const nnvm::NodeAttrs &attrs, | |||
| CHECK_EQ(inputs.size(), 5U); | |||
| const BatchNormParam ¶m = nnvm::get<BatchNormParam>(attrs.parsed); | |||
| // MKLDNN batchnorm only works well on the special MKLDNN layout. | |||
| if (SupportMKLDNNBN(inputs[0], param) && inputs[0].IsMKLDNNData()) { | |||
| if (SupportMKLDNNBN(inputs[0], param)) { | |||
| std::vector<NDArray> in_data(inputs.begin(), inputs.begin() + batchnorm::kInMovingMean); | |||
There was a problem hiding this comment.
Please remove the comments in L397
|
Seems something wrong on CI or your code. Please rebase the code and try again. |
|
Seems something wrong on CI, retrigger it again. If it's still failed, will look into it. |
ElaineBao
force-pushed
the
bn-condition
branch
from
4b08a14
to
f0a5a7f
Compare
|
It's not easy to pass CI. Merging now and thanks for your contribution. |
Description
update BN conditions of when to run into MKLDNN.
Changes
Performance