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

Move the logic of mkldnn layout in GetKernelTypeForVar from ActivationOp to OperatorWithKernel #47104

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

zyfncg
Copy link
Contributor

@zyfncg zyfncg commented Oct 18, 2022

PR types

Others

PR changes

Others

Describe

ActivationOpGetKernelTypeForVar 中对于 mkldnn layout的处理逻辑移动到基类OperatorWithKernel中。

主要目的是为了规范Op定义的相关代码,以便于通过Yaml完成对应代码逻辑的生成。

@paddle-bot
Copy link

paddle-bot bot commented Oct 18, 2022

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

// When the op is first oneDNN op (there was some non oneDNN op
// previously)
// then we also need to rotate shape NHWC -> NCWH
if ((expected_kernel_type.data_layout_ == phi::DataLayout::kMKLDNN) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

此处兼容其他 op 吗?很多 mkldnn op 的 GetKernelTypeForVar 函数中,并没有这部分代码

Do other ops compatible with the code here? Many mkldnn ops do not contain these codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

根据PR39097中的comment来看,这应该是一个通用的逻辑,具体是否有影响可能需要CE模型测试做进一步验证

@zyfncg zyfncg merged commit 95ca886 into PaddlePaddle:develop Oct 19, 2022
@zyfncg zyfncg deleted the mkldnn_layout_transform branch October 19, 2022 06:16
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.

4 participants