-
Notifications
You must be signed in to change notification settings - Fork 88
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
Pointwise + Concat fusion #2785
Conversation
This build is not recommended to merge 🔴 |
🔴bert_large_uncased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2785 +/- ##
===========================================
+ Coverage 91.82% 91.83% +0.01%
===========================================
Files 477 477
Lines 18112 18146 +34
===========================================
+ Hits 16631 16665 +34
Misses 1481 1481 ☔ View full report in Codecov by Sentry. |
return pm; | ||
}); | ||
auto* post_pm = mpm.create_module("noop:concat" + std::to_string(get_noop_counter())); | ||
auto x = post_pm->add_parameter("!x0", shape{concat_ins->get_shape().type()}); |
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.
Why is this prefixed with a !
? I would prefer not to have it start with a special character because this will become _x0
in the C++.
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 wasn't sure. I copied same logic from pointwise_concat_pointwise
fusion. I can remove it.
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.
It uses x0
in that pass.
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.
AMDMIGraphX/src/fuse_concat.cpp
Line 158 in b1a24e5
auto param = rm->add_parameter("!" + concat_param_name, concat_param->get_shape()); |
I am referring this line.
We should probably only do this input fusion if the number of noops is low. I would use something like |
Added that rule and a test. |
Fixes #2735