-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[Snippets][CPU] Make ExtractUnsupportedTransposes HW dependent #32676
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
base: master
Are you sure you want to change the base?
[Snippets][CPU] Make ExtractUnsupportedTransposes HW dependent #32676
Conversation
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.
Please revert changes in this file
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
| ::testing::Values(MHA::default_thread_count), | ||
| ::testing::Values(2), // decomposed Transpose + MHA | ||
| ::testing::Values(MHA::default_thread_count), | ||
| ::testing::Values(expected_nodes_mha_4d_f32), // decomposed Transpose + MHA |
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.
Let's remove the comment from here since it is not actual
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
| static constexpr size_t expected_nodes_mha_4d_f32 = 4; | ||
| #else | ||
| static constexpr size_t expected_nodes_mha_4d_f32 = 2; |
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.
Let's add an explanatory comment here
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, added for all ifdefs
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.
Same as above: let's leave a comment for expected_nodes_mha_mul_add , and remove the comment in INSTANTIATE_TEST_SUITE_P
The same for other test files
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
| static CommonOptimizations::Config conf(1, true); | ||
| static bool initialized = false; | ||
| if (!initialized) { | ||
| conf.set_transpose_support_callback([](const std::shared_ptr<const ov::Node>& node) -> bool { |
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.
Ideally, we need to share the callback code between tests and plugin transformation pipeline. Maybe we can introduce some helper for that? The helper could have e.g. bool parameter which defines if the "is_brgemm_case" check should be applied as well
What do you think?
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.
Good idea, implemented
c40d374 to
beebc20
Compare
2e50d87 to
107df70
Compare
Details:
Introduce a callback function for
ExtractUnsupportedTransposespass as a part ofCommonOptimizations::Configto customize pass behavior depending on Transpose support.For example, ARM64 platform supports transpose decomposition, but MatMul with Transpose A/B is not supported so far. Rest of the (potential) platforms mark Transpose as not supported completely
An alternative approach for #32592
Tickets: