[CPU_Cache]Enable Reorder node run time cache. #9490
Conversation
luweizhou2016
changed the title
| @@ -9,6 +9,8 @@ | |||
| #include "mkldnn_input_node.h" | |||
| #include "mkldnn_edge.h" | |||
| #include "mkldnn_node.h" | |||
| #include "cache/multi_cache.h" | |||
|
|
|||
There was a problem hiding this comment.
Right know cache feature coverage for reorder node seems to be not enough. This unit test handles only custom optimizedNcsp2Nspc() and optimizedNspc2Ncsp() impls which in fact are not cached.
Can we extend this test to handle generic Reorder from oneDNN for dynamic cases? Looking in the code I see two major changes required:
- We need to compare results for generic case: can be achieved by converting CpuBlockedMemoryDesc to DNNL one and using off_l(...) utility. See https://github.com/openvinotoolkit/oneDNN/blob/f06708e9cf6c3973efee9d2a1a4df086050e1fcd/tests/gtests/test_reorder_common.hpp#L33 as an example.
- Dynamic case support requires several inferences (for different shapes) inside one test run. Seems like can be implemented pretty easy by such approach: https://github.com/openvinotoolkit/openvino/blob/master/src/tests/functional/shared_test_classes/src/base/ov_subgraph.cpp#L54
I understand this is extra request so it definitely can be done in separate PR.
There was a problem hiding this comment.
Okay. Will understand more details and enlarge the test. I have one basic question. Is there any specific reason to constrcut this reorder specific test case directly without Ngraph or mkldnn_graph? Should move this test case into SLT or still in the unit test? Thx.
There was a problem hiding this comment.
Reorder node is a plugin specific op which is not presented in ngraph. In our words we cannot build such subgraph using public OpenVINO API (ngraph funtction). Instead another approach is used: we just compile CPU plugin internals into unit test in order to get direct access to internal nodes.
There was a problem hiding this comment.
@dmitry-gorokhov , Sorry for late reply. The extended the reorder test is in #10003. Pls help to review. Thx!
|
In general LGTM. Will launch internal validation to make sure we don't affect static scenario. Can be merged if internal validation won't show any degradations. |
Details:
Tickets: