[Snippets] Added support of Port Descriptor #106
Conversation
a-sidorova
force-pushed
the
feature/snippets/lin_ir/port_desc
branch
from
34e4cc0 to
938afe3
Compare
| } | ||
| } | ||
|
|
||
| PortDescriptor PortDescriptor::deserialize(const std::string& serialized_info) { |
There was a problem hiding this comment.
Minor for discussion: do we really need deserialization of Port descriptor? If we do, than is it sufficient to enable correct serialization-deserialization of the subgraph?
There was a problem hiding this comment.
I don't know because I just moved it from TensorDescriptor too. I don't know for what it, to be honest. I removed it.
Moreover, I don't understand why we need serialization :)
There was a problem hiding this comment.
Let's discuss it on review of the main PR.
src/plugins/intel_cpu/src/transformations/snippets/x64/op/brgemm_cpu.hpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_cpu/src/transformations/snippets/x64/op/brgemm_cpu.cpp
Show resolved
Hide resolved
| template<typename T> | ||
| void set_full_port_desc(const T& port) { | ||
| const auto& shape = port.get_shape(); | ||
| ngraph::snippets::PortManager::set_port_descriptor_ptr(port, std::make_shared<ngraph::snippets::PortDescriptor>(shape, |
There was a problem hiding this comment.
Do we absolutely need subtensor filled with FULL_DIM here? Can it just be equal to shape?
There was a problem hiding this comment.
We can use in general shape for subtensor. Because just image that we have Brgemm with input_shape0=[1,16]. It's equal to common subtensor for eltwises. But we mustn't add Brgemm and Eltwise into one Loop. So to avoid these situations, I added the separate flag FULL_DIM - it fully described schedule by full axis.
There was a problem hiding this comment.
TODO: Discussed offline: use VECTOR_SIZE instead of FULL_DIM
a-sidorova
force-pushed
the
feature/snippets/lin_ir/port_desc
branch
from
ea5e09a to
af947cb
Compare
a-sidorova
force-pushed
the
feature/snippets/lin_ir/port_desc
branch
from
af947cb to
1f76a5d
Compare
src/plugins/intel_cpu/src/transformations/snippets/x64/op/brgemm_cpu.cpp
Outdated
Show resolved
Hide resolved
src/plugins/intel_cpu/src/transformations/snippets/x64/op/brgemm_cpu.cpp
Outdated
Show resolved
Hide resolved
* [Snippets] Added support of Port Descriptor * review * Added Softmax support via common pipeline * Added Brgemm marking via general pipeline * TensorDescriptor -> ExpressionPort * Refactored expression factory * ExpressionPort - is interface * refactoring * fixed init loops * fixed brgemm ops * Moved PortDescriptor to lowered level * Removed PortDesc getters and setters from ExpressionPort and Tensor * Applied comments
* [Snippets] Added support of Port Descriptor * review * Added Softmax support via common pipeline * Added Brgemm marking via general pipeline * TensorDescriptor -> ExpressionPort * Refactored expression factory * ExpressionPort - is interface * refactoring * fixed init loops * fixed brgemm ops * Moved PortDescriptor to lowered level * Removed PortDesc getters and setters from ExpressionPort and Tensor * Applied comments
Details:
Tickets: