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] move unbind to phi #39789

Merged
merged 5 commits into from Feb 23, 2022
Merged

[phi] move unbind to phi #39789

merged 5 commits into from Feb 23, 2022

Conversation

zhiqiu
Copy link
Contributor

@zhiqiu zhiqiu commented Feb 21, 2022

PR types

Breaking changes

PR changes

OPs

Describe

move unbind to phi

@paddle-bot-old
Copy link

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

@zhiqiu zhiqiu changed the title move unbind to phi [phi] move unbind to phi Feb 22, 2022
@zhiqiu zhiqiu requested a review from chenwhql February 22, 2022 09:19
#include "paddle/fluid/operators/detail/strided_memcpy.h"
#include "paddle/phi/backends/all_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/cpu/concat_and_split.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

这里好像不太对,为什么只需要cpu的,是不是还是得用operators/math下concat_and_split

Copy link
Contributor

Choose a reason for hiding this comment

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

这里其实应该迁移过来的,cpu和gpu下面的concat_and_split应该在funcs下更好,之前迁移的不到位

Copy link
Contributor Author

@zhiqiu zhiqiu Feb 22, 2022

Choose a reason for hiding this comment

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

已迁移

#include "paddle/phi/backends/all_context.h"
#include "paddle/phi/core/kernel_registry.h"
#include "paddle/phi/kernels/cpu/concat_and_split.h"
#include "paddle/phi/kernels/flatten_kernel.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

flatten_kernel好像没有使用?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯嗯

namespace phi {

template <typename T, typename Context>
void UnbindKernel(const Context& ctx,
Copy link
Contributor

Choose a reason for hiding this comment

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

这种情况,应该属于在impl下面加cpu和gpu公共实现,然后在cpu和gpu下面分别注册的写法?kernels根目录下kernel.cc中的实现,需要是完全设备无关的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的

#include <vector>

#include "paddle/fluid/framework/convert_utils.h"
#include "paddle/fluid/framework/eigen.h"
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.

好的

@@ -14,7 +14,7 @@ limitations under the License. */

#include "paddle/fluid/operators/math/concat_and_split.h"

#include "paddle/phi/kernels/cpu/concat_and_split.h"
#include "paddle/phi/kernels/funcs/concat_and_split_functor.h"
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.

后续清理下

#include <vector>

#include "paddle/fluid/framework/convert_utils.h"
#include "paddle/fluid/framework/eigen.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

#include <vector>

#include "paddle/fluid/framework/convert_utils.h"
#include "paddle/fluid/framework/eigen.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

@zhiqiu zhiqiu merged commit dba694f into PaddlePaddle:develop Feb 23, 2022
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.

None yet

3 participants