Skip to content
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

Jump to bottom

Add an elementwise + activation fusion pass. #36541

Merged
merged 94 commits into from Mar 14, 2022
Merged

Add an elementwise + activation fusion pass. #36541

merged 94 commits into from Mar 14, 2022

Conversation

tsocha
Copy link
Contributor

@tsocha tsocha commented Oct 19, 2021 •
edited

PR types

Performance optimization

PR changes

Others

Describe

Add a pass which fuse element wise operation with an activation:

  • "elementwise_add", "elementwise_sub", "elementwise_mul"
  • "relu", "tanh", "leaky_relu", "swish", "hardswish", "sqrt", "abs", "clip", "gelu", "relu6", "sigmoid"
  • required attributes are used in pass without registration in makers.

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot-old
Copy link

paddle-bot-old bot commented Oct 19, 2021 •
edited

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@jczaja jczaja requested a review from jakpiase October 19, 2021 09:18
@jczaja
Copy link
Contributor

jczaja commented Oct 19, 2021

@piotrekobiIntel ,@Silv3S Please review

@jczaja jczaja requested a review from wozna October 19, 2021 09:22
@jczaja jczaja added the Intel label Oct 19, 2021
jakpiase
jakpiase previously approved these changes Oct 19, 2021
View reviewed changes
Copy link
Contributor

@jakpiase jakpiase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of really good work, but I have left some comments

paddle/fluid/framework/ir/mkldnn/elt_act_onednn_fuse_pass.cc Outdated Show resolved Hide resolved
paddle/fluid/framework/ir/mkldnn/elt_act_onednn_fuse_pass.cc Outdated Show resolved Hide resolved
paddle/fluid/operators/elementwise/mkldnn/elementwise_mkldnn_op.h Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/ir/inference/test_mkldnn_elt_act_fuse_pass.py Outdated Show resolved Hide resolved
python/paddle/fluid/tests/unittests/ir/inference/test_mkldnn_elt_act_fuse_pass.py Outdated Show resolved Hide resolved
@tsocha tsocha dismissed stale reviews from arlesniak and wozna via 467e868 March 1, 2022 12:44
@tsocha
Copy link
Contributor Author

tsocha commented Mar 4, 2022

Please add a single test based on the new single test framework.

@baoachun
I and @wozna have created an another solution which don't use custom operators.
#40081
In this ⬆️ PR we just set an attribute which is not registered anywhere and it seems to work.
I also added a test on the new test framework there.

Please tell me, should I select this alternative solution instead of custom op from this PR?

@lidanqing-intel lidanqing-intel added this to the 2022 Q1 milestone Mar 7, 2022
@lidanqing-intel lidanqing-intel mentioned this pull request Mar 9, 2022
Closed
@lidanqing-intel
Copy link
Contributor

lidanqing-intel commented Mar 10, 2022 •
edited

@baoachun Hi, Tomasz and Joanna made another solution that adds attributes in the passes, could you please review it #40081 Thanks!

@tsocha tsocha changed the title Add an elementwise + activation fusion pass, add mechanizm to add attributes outside of base op. Add an elementwise + activation fusion pass. Mar 10, 2022
jczaja
jczaja approved these changes Mar 14, 2022
View reviewed changes
Copy link
Contributor

@jczaja jczaja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jczaja jczaja merged commit 3f21916 into PaddlePaddle:develop Mar 14, 2022
@tsocha tsocha deleted the el_add_act_fuse branch June 13, 2022 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jczaja jczaja jczaja approved these changes

@lidanqing-intel lidanqing-intel Awaiting requested review from lidanqing-intel

@sfraczek sfraczek Awaiting requested review from sfraczek

@arlesniak arlesniak Awaiting requested review from arlesniak

@wozna wozna Awaiting requested review from wozna

@jakpiase jakpiase Awaiting requested review from jakpiase

Assignees

@lidanqing-intel lidanqing-intel

Labels
Intel
Projects
None yet
Milestone
2022 Q1
Development

Successfully merging this pull request may close these issues.

[Feature] Add passes that add mkldnn related attributes instead of changing native ops
9 participants
@tsocha @jczaja @arlesniak @lidanqing-intel @baoachun @sfraczek @wozna @jakpiase @Aganlengzi