Skip to content

Commit 6b34ac9

Browse files
committed
Review comments applied
1 parent fccd3d7 commit 6b34ac9

File tree

2 files changed

+109
-72
lines changed

2 files changed

+109
-72
lines changed

src/plugins/intel_cpu/src/graph_optimizer.cpp

+21-25
Original file line numberDiff line numberDiff line change
@@ -2523,32 +2523,28 @@ void GraphOptimizer::mergeTransposeReshapeReorder(Graph& graph,
25232523
}
25242524
OPENVINO_ASSERT(edge, "Parent node '", parentNode->getName(), "' has invalid edges.");
25252525

2526-
// transposeNode support blocked input & non-blocked output, in the case, the Reorder after Transpose cannot be optimized
25272526
std::vector<int> srcPerm;
2528-
if (!reverseOrder) {
2529-
// case 1. transposeNode support blocked input & non-blocked output, in the case, the reorder
2530-
// cannot be optimized
2531-
// case 2. Transpose and Reorder do opposite permutation to each other as expected, but isOptimized is already
2532-
// set false due to some preliminarily checks. We need to reinterpret layout Transpose input without physical
2533-
// change of the memory.
2534-
auto* castedTranspose = dynamic_cast<Transpose*>(transposeNode.get());
2535-
OPENVINO_ASSERT(castedTranspose,
2536-
"[CPU] parent node of type:",
2537-
transposeNode->getTypeStr(),
2538-
" with name: ",
2539-
transposeNode->getName(),
2540-
" is not a transpose node");
2541-
2542-
auto inOrder = transposeNode->getSelectedPrimitiveDescriptor()->getConfig().inConfs[0].getMemDesc()->as<BlockedMemoryDesc>()->getOrder();
2543-
auto outOrder = reorderOutDesc->as<BlockedMemoryDesc>()->getOrder();
2544-
if (!isOptimized || inOrder.size() > outOrder.size()) {
2545-
isOptimized = false;
2546-
// inDesc should be permuted before calling reorder
2547-
auto& ord = castedTranspose->getOrder();
2548-
srcPerm = std::vector<int>(ord.size());
2549-
for (size_t i = 0; i < ord.size(); i++) {
2550-
srcPerm[ord[i]] = i;
2551-
}
2527+
auto* castedTranspose = dynamic_cast<Transpose*>(transposeNode.get());
2528+
OPENVINO_ASSERT(castedTranspose,
2529+
"[CPU] parent node of type:",
2530+
transposeNode->getTypeStr(),
2531+
" with name: ",
2532+
transposeNode->getName(),
2533+
" is not a transpose node");
2534+
2535+
auto inOrder = transposeNode->getSelectedPrimitiveDescriptor()->getConfig().inConfs[0].getMemDesc()->as<BlockedMemoryDesc>()->getOrder();
2536+
auto outOrder = reorderOutDesc->as<BlockedMemoryDesc>()->getOrder();
2537+
// Permutation must be set and reorder mustn't be optimized in 2 cases:
2538+
// 1. Transpose has blocked input & non-blocked output
2539+
// 2. Transpose and Reorder do opposite permutation to each other as expected,
2540+
// but isOptimized is already set to false due to some preliminarily checks.
2541+
if (!isOptimized || inOrder.size() > outOrder.size()) {
2542+
isOptimized = false;
2543+
// inDesc should be permuted before calling reorder
2544+
auto& ord = castedTranspose->getOrder();
2545+
srcPerm = std::vector<int>(ord.size());
2546+
for (size_t i = 0; i < ord.size(); i++) {
2547+
srcPerm[ord[i]] = i;
25522548
}
25532549
}
25542550

src/plugins/intel_cpu/tests/unit/graph/merge_transpose_reorder_test.cpp

+88-47
Original file line numberDiff line numberDiff line change
@@ -3,71 +3,106 @@
33
//
44
#include <gtest/gtest.h>
55

6+
#include <common_test_utils/test_common.hpp>
7+
68
#include "dummy_node.hpp"
7-
#include "nodes/reorder.h"
89
#include "nodes/input.h"
10+
#include "nodes/reorder.h"
911
#include "nodes/transpose.h"
10-
1112
#include "ov_models/builders.hpp"
1213

1314
using namespace ov::intel_cpu;
15+
using LOOK = Edge::LOOK;
16+
struct MergeTransposeReorderTestParams {
17+
enum Result {IS_OPTIMIZED, NOT_OPTIMIZED};
18+
MergeTransposeReorderTestParams(const ov::Shape& testShape,
19+
ov::element::Type_t testPrec,
20+
LayoutType firstNodeLayout,
21+
LOOK firstNodeInplaceDirection,
22+
LayoutType secondNodeLayout,
23+
LOOK secondNodeInplaceDirection,
24+
Result test_result)
25+
: testShape(testShape),
26+
testPrec(testPrec),
27+
firstNodeLayout(firstNodeLayout),
28+
firstNodeInplaceDirection(firstNodeInplaceDirection),
29+
secondNodeLayout(secondNodeLayout),
30+
secondNodeInplaceDirection(secondNodeInplaceDirection),
31+
test_result(test_result) {}
32+
33+
ov::Shape testShape;
34+
ov::element::Type_t testPrec;
35+
LayoutType firstNodeLayout;
36+
LOOK firstNodeInplaceDirection;
37+
LayoutType secondNodeLayout;
38+
LOOK secondNodeInplaceDirection;
39+
Result test_result;
40+
};
41+
using Result = MergeTransposeReorderTestParams::Result;
1442

1543
/*
1644
* MergeTransposeReorderIsOptimizedCPUTest to test the CPU plugin-in MergeTransposeReorder graph optimizer
1745
* under the circumstance that the upstream node or downstream node is inPlaced thereby the inserted Reorder
1846
* cannot be optimized.
1947
*/
20-
class MergeTransposeReorderIsOptimizedCPUTest : public ::testing::Test {
48+
class MergeTransposeReorderIsOptimizedCPUTest : public testing::WithParamInterface<MergeTransposeReorderTestParams>,
49+
public ov::test::TestsCommon {
2150
public:
2251
void Validate() const {
2352
CheckTransposeCount(0);
24-
CheckReorderOptimized(std::string("_fake"), false); // the fused node is of name "reshape_abcd_acdb_fake"
53+
const bool is_optimized = GetParam().test_result == MergeTransposeReorderTestParams::Result::IS_OPTIMIZED;
54+
CheckReorderOptimized(std::string("_fake"), is_optimized); // the fused reorder has always postfix "fake"
2555
}
2656

57+
protected:
2758
void SetUp() override {
28-
CreateGraph();
59+
MergeTransposeReorderTestParams params = GetParam();
60+
CreateGraph(params.testShape,
61+
params.testPrec,
62+
params.firstNodeLayout,
63+
params.firstNodeInplaceDirection,
64+
params.secondNodeLayout,
65+
params.secondNodeInplaceDirection);
2966
}
3067

31-
protected:
32-
/* graph typology
33-
---------
34-
|Input |
35-
---------
36-
|
37-
----------
38-
| Dummy | <*NOTE: fake node with laytout NCSP, and inplace from upstream*>
39-
----------
40-
|
41-
|---------------|
42-
| ---------- |
43-
| |Transpose| |
44-
| --------- |
45-
| | |
46-
| --------- |
47-
| |Reorder | | <*NOTE: Reorder is inheristically inserted since Multiply is asking NSPC input.*>
48-
| --------- |
49-
|---------------|
50-
|
51-
-----------
52-
| Dummy | <*NOTE: fake node with laytout NSPC, and inplace from downstream*>
53-
-----------
54-
|
55-
---------
56-
|Output |
57-
---------
68+
/* graph topology
69+
┌───────┐
70+
│ Input │
71+
└───┬───┘
72+
73+
┌───┴───┐
74+
│ Dummy │ <*NOTE: fake node with firstNodeLayout, and firstNodeInplaceDirection*>
75+
└───┬───┘
76+
77+
┌────┴────┐
78+
│Transpose│ <*NOTE: Reorder is inserted before/after Transpose depending on first/second node layouts.*>
79+
└────┬────┘
80+
81+
┌───┴───┐
82+
│ Dummy │ <*NOTE: fake node with secondNodeLayout, and secondNodeInplaceDirection*>
83+
└───┬───┘
84+
85+
┌────┴───┐
86+
│ Output │
87+
└────────┘
5888
*/
59-
void CreateGraph() {
60-
//
89+
void CreateGraph(const ov::Shape& testShape,
90+
const ov::element::Type_t& testPrec,
91+
LayoutType firstNodeLayout,
92+
LOOK firstNodeInplaceDirection,
93+
LayoutType secondNodeLayout,
94+
LOOK secondNodeInplaceDirection) {
6195
Config conf;
6296
conf.rtCacheCapacity = 100;
6397
auto context = std::make_shared<GraphContext>(conf, nullptr, nullptr, false);
6498
const dnnl::engine cpuEngine = context->getEngine();
6599

66100
m_graph = std::unique_ptr<Graph>(new Graph());
67101

102+
OPENVINO_ASSERT(testShape.size() == 4 || testShape.size() == 3, "Only 4D and 3D shapes are supported");
68103
// ov::Model with only a transpose node
69-
ov::ParameterVector params{std::make_shared<ov::op::v0::Parameter>(testPrec, ov::Shape(testShape))};
70-
auto order = std::vector<int32_t>{0, 3, 1, 2};
104+
ov::ParameterVector params{std::make_shared<ov::op::v0::Parameter>(testPrec, testShape)};
105+
auto order = testShape.size() == 4 ? std::vector<int32_t>{0, 3, 1, 2} : std::vector<int32_t>{0, 2, 1};
71106
auto constOrder = ngraph::builder::makeConstant(ov::element::i32, {order.size()}, order);
72107
auto transpose = std::make_shared<ov::op::v1::Transpose>(params[0], constOrder);
73108
ov::ResultVector results{std::make_shared<ov::op::v0::Result>(transpose)};
@@ -86,18 +121,16 @@ class MergeTransposeReorderIsOptimizedCPUTest : public ::testing::Test {
86121

87122
auto inputNode = std::make_shared<node::Input>(params[0], context);
88123

89-
// dummy ncsp + inPlace LOOK_UP
90124
auto dummyNode1 = std::make_shared<cpu_unit_test::DummyNode>(
91-
testShape, testPrec, "reshape", "DummyNode", context, LayoutType::ncsp, Edge::LOOK::LOOK_UP);
125+
testShape, testPrec, "reshape", "DummyNode", context, firstNodeLayout, firstNodeInplaceDirection);
92126

93127
auto orderNode = std::make_shared<node::Input>(constOrder, context); // const order
94128
auto transposeNode = std::make_shared<node::Transpose>(transpose, context);
95129
transposeNode->filterSupportedPrimitiveDescriptors();
96130

97-
// dummy nspc + inPlace LOOK_DOWN
98-
const ov::Shape shape_tranpose{testShape[0], testShape[3], testShape[1], testShape[2]}; // shape after transpose
131+
const auto& transpose_shape = transpose->get_output_shape(0);
99132
auto dummyNode2 = std::make_shared<cpu_unit_test::DummyNode>(
100-
shape_tranpose, testPrec, "multiply", "DummyNode", context, LayoutType::nspc, Edge::LOOK::LOOK_DOWN);
133+
transpose_shape, testPrec, "multiply", "DummyNode", context, secondNodeLayout, secondNodeInplaceDirection);
101134

102135
auto outputNode = std::make_shared<node::Input>(results[0], context);
103136

@@ -146,12 +179,20 @@ class MergeTransposeReorderIsOptimizedCPUTest : public ::testing::Test {
146179
}
147180

148181
private:
149-
const ov::element::Type_t testPrec = ov::element::Type_t::f32;
150-
const ov::Shape testShape{1, 3, 8, 16};
151-
152182
std::unique_ptr<Graph> m_graph;
153-
}; // class MergeTransposeReorderIsOptimizedCPUTest
183+
}; // class MergeTransposeReorderIsOptimizedCPUTest
154184

155-
TEST_F(MergeTransposeReorderIsOptimizedCPUTest, smoke_Run_MergeTransposeReorder_isOptimized) {
185+
TEST_P(MergeTransposeReorderIsOptimizedCPUTest, smoke_Run_MergeTransposeReorder_isOptimized) {
156186
Validate();
157-
}
187+
}
188+
189+
const std::vector<MergeTransposeReorderTestParams> params = {
190+
{{1, 3, 8, 16}, ov::element::f32, LayoutType::ncsp, LOOK::LOOK_UP, LayoutType::nspc, LOOK::LOOK_DOWN, Result::NOT_OPTIMIZED},
191+
{{1, 3, 8, 16}, ov::element::f32, LayoutType::ncsp, LOOK::LOOK_DOWN, LayoutType::nspc, LOOK::LOOK_UP, Result::IS_OPTIMIZED},
192+
{{3, 8, 16}, ov::element::f32, LayoutType::nspc, LOOK::LOOK_UP, LayoutType::ncsp, LOOK::LOOK_DOWN, Result::NOT_OPTIMIZED},
193+
{{3, 8, 16}, ov::element::f32, LayoutType::nspc, LOOK::LOOK_DOWN, LayoutType::ncsp, LOOK::LOOK_UP, Result::IS_OPTIMIZED},
194+
};
195+
196+
INSTANTIATE_TEST_SUITE_P(smoke_Run_MergeTransposeReorder_isOptimized,
197+
MergeTransposeReorderIsOptimizedCPUTest,
198+
::testing::ValuesIn(params));

0 commit comments

Comments
 (0)