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

Remove oneDNN-specific attributes from matmul #49444

Merged
merged 103 commits into from
Apr 6, 2023

Conversation

Silv3S
Copy link
Member

@Silv3S Silv3S commented Dec 29, 2022

PR types

Others

PR changes

OPs

Describe

In #49515 each fuse done for matmul and matmul_v2 was converted to be executed as fused_matmul. Fusion logic was extracted to dedicated fused op and fused kernel. As base matmul is now not fused with anything, all extra attributes can be deleted from OPMaker, .pbtxt file and unit tests.

Changes done in this PR:

  • Remove fusion logic from base matmul op and kernel,
  • Remove placeholders for empty attributes in unit tests,
  • Remove test_matmul_mkldnn_op.py unit tests, which has extra attributes hardcoded in base operators. These test cases are covered in test_onednn_matmul_transpose_reshape_fuse_pass.py and test_onednn_reshape_transpose_matmul_fuse_pass.py. Same for matmul_v2.

@paddle-bot
Copy link

paddle-bot bot commented Dec 29, 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.

@Silv3S
Copy link
Member Author

Silv3S commented Jan 3, 2023

reshape_transpose_matmul_mkldnn_fuse_pass introduced 4 extra oneDNN-specific attributes for matmul op: fused_reshape_X, fused_reshape_Y, fused_transpose_X, fused_transpose_Y. Then in #30011 checkpoint with only one of these attributes (fused_reshape_Y) was added to op version registry.

Now, we are going to delete all oneDNN-specific attributes from base ops. Do you think we could just delete this checkpoint and restore matmul version 0 or new checkpoint should be added and matmul would be version 2 then? @YuanRisheng, @chenwhql please advice

@Silv3S Silv3S requested review from wozna and sfraczek March 14, 2023 11:37
@yaomichael
Copy link

@YuanRisheng please help review, approve and merge

@Silv3S
Copy link
Member Author

Silv3S commented Apr 6, 2023

@zhangting2020 could you please approve removing decorator @skip_check_grad_ci? I deleted test cases with extra hardcoded attributes from test_matmul_mkldnn_op.py as they are already covered in test_onednn_matmul_transpose_reshape_fuse_pass.py.

@Silv3S
Copy link
Member Author

Silv3S commented Apr 6, 2023

@XiaoguangHu01 could you please approve exceeding limit of 20 modified files?

Copy link
Contributor

@zhangting2020 zhangting2020 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

LGTM

@YuanRisheng YuanRisheng merged commit 4d97b25 into PaddlePaddle:develop Apr 6, 2023
@Silv3S Silv3S deleted the matmul_v1_version branch April 6, 2023 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers Intel
Projects
None yet
Development

Successfully merging this pull request may close these issues.