MKLDNN layout: Support for convolution operator #11099
Conversation
mozga-intel
force-pushed
the
mozga-intel/Conv_mkldnn_layout
branch
from
ae9b5a5
to
9908d3c
Compare
|
@luotao1 The code is prepared to the code-review process. |
| if (memory::primitive_desc(conv_pd->src_primitive_desc()) != | ||
| user_src_memory.get_primitive_desc()) { | ||
| src_memory = memory(conv_pd->src_primitive_desc()); | ||
| reorder_src = reorder(user_src_memory, src_memory); |
There was a problem hiding this comment.
Is this inplace reorder according to L108?
The result would be correct?
There was a problem hiding this comment.
Yes, this is in-place reorder. During the tests I did't see any problems with the result. The all tests worked correctly.
There was a problem hiding this comment.
Actually I think that is not inplace reorder since you write src_memory = memory(conv_pd->src_primitive_desc()); . That will allocate new memory.
There was a problem hiding this comment.
@tensor-tang Oh yes this is not in place. I can not find information about it. All of tests work correctly. therefore I think that this code is correct. Should be out-of-place?.
| if (memory::primitive_desc(conv_pd->weights_primitive_desc()) != | ||
| user_weights_memory.get_primitive_desc()) { | ||
| weights_memory = memory(conv_pd->weights_primitive_desc()); | ||
| reorder_weights = reorder(user_weights_memory, weights_memory); |
| if (memory::primitive_desc(conv_bwd_weights_pd.src_primitive_desc()) != | ||
| user_src_memory.get_primitive_desc()) { | ||
| src_memory = memory(conv_bwd_weights_pd.src_primitive_desc()); | ||
| reorder_src = reorder(user_src_memory, src_memory); |
| @@ -75,9 +75,8 @@ void ConvOp::InferShape(framework::InferShapeContext* ctx) const { | |||
| framework::OpKernelType ConvOp::GetExpectedKernelType( | |||
| const framework::ExecutionContext& ctx) const { | |||
| framework::LibraryType library{framework::LibraryType::kPlain}; | |||
|
|
|||
| std::string data_format = ctx.Attr<std::string>("data_format"); | |||
| // TODO(pzelazko-intel): enable MKLDNN layout when it's ready | |||
There was a problem hiding this comment.
This comment shoud be removed?
Waiting for supports of the mkldnn’s layout.
Please have a look at this convolution 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