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

Conv relu mkldnn fuse pass #33664

Merged
merged 22 commits into from
Jul 1, 2021
Merged

Conv relu mkldnn fuse pass #33664

merged 22 commits into from
Jul 1, 2021

Conversation

fengxiaoshuai
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

enhance conv_relu_mkldnn_pass

@paddle-bot-old
Copy link

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

attrs {
name: "name"
type: STRING
}
attrs {
Copy link
Contributor

Choose a reason for hiding this comment

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

只是调整一下name的位置?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

调试导致

Copy link
Contributor

Choose a reason for hiding this comment

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

这个改动我理解没有任何意义,,,没有意义的改动还是不要留着吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,已修改

attrs {
name: "@ENABLE_CACHE_RUNTIME_CONTEXT@"
type: BOOLEAN
}
Copy link
Contributor

Choose a reason for hiding this comment

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

属性重复添加

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经删除

@@ -83,6 +88,12 @@ void ConvActivationFusePass::ApplyImpl(ir::Graph* graph) const {
desc->SetAttr("fuse_beta",
activation->Op()->GetAttrIfExists<float>("beta"));

if (!IsCompat(*desc)) {
LOG(WARNING)
<< "conv_activation_mkldnn_fuse_pass in out fc op compat failed.";
Copy link
Contributor

Choose a reason for hiding this comment

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

这个log是不是写的有问题?这儿是fc op 吗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已修改

@@ -12,10 +12,22 @@ extra {
name: "threshold"
Copy link
Contributor

Choose a reason for hiding this comment

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

请确认一下 threshold是不是应该属于def?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

已经修改

.End()
// float, optional, default=6.0
.AddAttr("threshold")
.IsOptional()
Copy link
Contributor

Choose a reason for hiding this comment

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

添加IsType() 约束条件,后面类似

Copy link
Contributor Author

Choose a reason for hiding this comment

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

全部添加

attrs {
name: "name"
type: STRING
}
attrs {
Copy link
Contributor

Choose a reason for hiding this comment

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

这个改动我理解没有任何意义,,,没有意义的改动还是不要留着吧

@@ -83,6 +88,12 @@ void ConvActivationFusePass::ApplyImpl(ir::Graph* graph) const {
desc->SetAttr("fuse_beta",
activation->Op()->GetAttrIfExists<float>("beta"));

if (!IsCompat(*desc)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这一部分删了吧,不用检测。如果这儿检测失败提前返回的话,graph属于改了一部分的状态,就完蛋了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,已经删除

Copy link
Contributor

@winter-wang winter-wang left a comment

Choose a reason for hiding this comment

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

LGTM

@Shixiaowei02 Shixiaowei02 merged commit cc5d4b1 into PaddlePaddle:develop Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants