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

[Phi]Change Copy from Kernel to basic component utils #43622

Merged
merged 25 commits into from
Jun 24, 2022

Conversation

YuanRisheng
Copy link
Contributor

@YuanRisheng YuanRisheng commented Jun 17, 2022

PR types

Others

PR changes

Others

Describe

当前的CopyKernel存在一些局限性:
1,推理需要kernel里不使用单例,而当前的CopyKernel里存在使用单例的情况且在Kernel里不太好去除。
2,Kernel的形式只能传递一个dev_context参数,而实际的Copy功能有传递俩个dev_context参数的需求。
3,根据datatype,layout,backend等不同可以来区分不同kernel,而copy在算子库中应该是一个工具类的存在,datatype,backend等的不同得出不同的copy是不太合理的。

该PR主要功能将Copy由Kernel变为基础组件工具:
1,去掉原来的copy_kernel(包括文件,kernel注册),取而代之得是tensor_utils.h中的Copy工具函数
2,将所有使用copy_kernel的地方修改为使用tensor_utils.h下的Copy函数。

后续TODO:
1,优化Copy功能实现,拓展俩个dev_context实现接口。
2,去除掉copy里边的单例使用。

@paddle-bot-old
Copy link

你的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.

zyfncg
zyfncg previously approved these changes Jun 21, 2022
auto kernel = phi::KernelFactory::Instance().SelectKernelOrThrowError(
"copy", kernel_key);
VLOG(6) << "copy API kernel: " << kernel;
VLOG(6) << "start copy. ";
Copy link
Contributor

Choose a reason for hiding this comment

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

log用start copy的话感觉还需要一个finish copy来对应

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@chenwhql chenwhql 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

@ZzSean ZzSean left a comment

Choose a reason for hiding this comment

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

LGTM for OP-Benchmark

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 2739bd7 into PaddlePaddle:develop Jun 24, 2022
sneaxiy pushed a commit to sneaxiy/Paddle that referenced this pull request Jun 27, 2022
…3622)

* perfect copy

* deal with conflict

* deal with conflict

* fix compile bugs

* fix unittest bugs

* change code format

* deal with conflict

* modify code by review

* fix ce bugs

* fix ce bugs

* add lo

* perfect code format

* deal with conflicts
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.

5 participants