-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NPUW] Extend ConvToMatmul pattern #27561
base: master
Are you sure you want to change the base?
[NPUW] Extend ConvToMatmul pattern #27561
Conversation
src/plugins/intel_npu/src/plugin/npuw/partitioning/patterns/opt.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One-lines get most of the attention
@@ -1501,6 +1501,7 @@ ConvToMatmul::ConvToMatmul(Context::Ref ctx) { | |||
auto matched_node_transpose_in = node_to_output.at(transpose_in).get_node_shared_ptr(); | |||
auto matched_node_transpose_out = node_to_output.at(transpose_out).get_node_shared_ptr(); | |||
auto matched_node_multiply = node_to_output.at(multiply).get_node_shared_ptr(); | |||
const auto& matched_node_param2_out = uat::_(node_to_output).at_or_at(convert2, multiply); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd call it cvt2_or_multiply
- as it is exactly what you're looking up for in the map.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -1536,10 +1537,10 @@ ConvToMatmul::ConvToMatmul(Context::Ref ctx) { | |||
auto new_reshape2 = std::make_shared<ov::op::v1::Reshape>(matched_node_param2, new_const2, false); | |||
|
|||
// Connect to Reshape | |||
if (ov::op::util::is_parameter(matched_node_param2)) { | |||
if (matched_node_param2_out == matched_node_multiply) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...then the logic here becomes clear
if (matched_node_param2_out == matched_node_multiply) { | |
if (cvt2_or_multiply == matched_node_multiply) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
matched_node_multiply->input(1).replace_source_output(new_reshape2); | ||
matched_node_multiply->validate_and_infer_types(); | ||
} else { // constant -> convert -> multiply | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But some comments documenting what cases you're looking at still would help a lot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Support case where Const has no Convert after it