Add transformer of paddle desc and cinn desc#36100
Add transformer of paddle desc and cinn desc#36100thisjiang merged 12 commits intoPaddlePaddle:developfrom
Conversation
|
Thanks for your contribution! |
| cc_test(cinn_compiled_object_test SRCS cinn_compiled_object_test.cc DEPS cinn_compiled_object) | ||
|
|
||
| cc_library(transform_desc SRCS transform_desc.cc DEPS proto_desc cinn_core) | ||
| cc_test(transform_desc_test SRCS transform_desc_test.cc DEPS transform_desc) No newline at end of file |
There was a problem hiding this comment.
Github reports "No newline at the end of file"
| cc_test(cinn_runner_test SRCS cinn_runner_test.cc DEPS cinn_runner proto_desc) | ||
| cc_test(cinn_compiled_object_test SRCS cinn_compiled_object_test.cc DEPS cinn_compiled_object) | ||
|
|
||
| cc_library(transform_desc SRCS transform_desc.cc DEPS proto_desc cinn_core) |
There was a problem hiding this comment.
最新CINN里面有导出 cinnapi.so，不要链接 cinn_core，链接 cinnapi.so
| using PbVarType = framework::proto::VarType; | ||
| namespace cpp = ::cinn::frontend::paddle::cpp; | ||
|
|
||
| void TransformVarDescToCinn(framework::VarDesc *pb_desc, |
There was a problem hiding this comment.
Should we make framework::VarDesc * as const framework::VarDesc & to match the style of TransformVarDescFromCinn?
There was a problem hiding this comment.
Compare to cpp::Desc, framework::Desc lack of many const API, like clear(), BlockDesc::SetIdx, etc. On the other hand, the paddle node return framework::Desc* even if the node is const, so the parameter in Transform*DescToCinn is framework::VarDesc * rather than const framework::VarDesc &.
| } | ||
|
|
||
| /// For OpDesc transform | ||
| void OpInputsToCinn(framework::OpDesc *pb_desc, cpp::OpDesc *cpp_desc) { |
There was a problem hiding this comment.
Same as above, should we make framework::OpDesc * as const framework::OpDesc & to match the style of OpInputsFromCinn?
| } | ||
| } | ||
|
|
||
| void OpOutputsToCinn(framework::OpDesc *pb_desc, cpp::OpDesc *cpp_desc) { |
There was a problem hiding this comment.
Same as above, there are some similar places, could you change it or tell me why we won't change it?
| namespace framework { | ||
| namespace paddle2cinn { | ||
|
|
||
| namespace cpp = ::cinn::frontend::paddle::cpp; |
There was a problem hiding this comment.
A small C++ knowledge and habit: try to avoid using namespace and namespace = in header file because it can break the encapsulation of namespace.
Let's say, if other file #include<transform_desc.h>, then they can also call cpp:XxDesc to access the variable inside the namespace.
If you write using namespace ::cinn::frontend::paddle::cpp, that's worse, other file can just call XxDesc
So above breaks namespace encapsulation. However, another C++ knowledge is that it is OK to write using namespace and namespace = in source file.
There was a problem hiding this comment.
Thanks, you are right! I'll fix it in next commit.
There was a problem hiding this comment.
Please ask me to review the fix PR, we would like to make sure of it.
| cc_test(cinn_runner_test SRCS cinn_runner_test.cc DEPS cinn_runner proto_desc) | ||
| cc_test(cinn_compiled_object_test SRCS cinn_compiled_object_test.cc DEPS cinn_compiled_object) | ||
|
|
||
| cc_library(transform_desc SRCS transform_desc.cc DEPS proto_desc cinnapi.so) |
There was a problem hiding this comment.
需要吧，我看现有cmake里链接动态库好像都有.so后缀
|
Sorry to inform you that bf9fb2d's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually. |
This PR added some changes to match the CINN change for compilation. It also tried to fix JiangCheng's Problem in PR: #36100 These changes include: 1. Set `CINN_GIT_TAG` to a newer tag 2. CINN now just `make cinnapi -j` 3. We have to add `-DPY_VERSION=${PY_VERSION} -DWITH_TESTING=ON` to CINN cmake args 4. For CINN's third party dependencies, we could just include headers without target_link_libraries 5. Moved `cinn.cmake` from `paddle/cmake` to `paddle/cmake/external` to match old style. External folder contains `lite`, which is the same level of `cinn` 6. CINN added `-DNAMESPACE=cinn_gflags` in `gflags.cmake` to have different gflag namespaces between CINN and Paddle. It solved re-define problem. 7. Change namespace of `::google::` in gflags to `::GFLAGS_NAMESPACE`
4 participants
PR types
New features
PR changes
APIs
Describe
如题。
由于cinn与paddle尚未实现联编，编译会因为找不到文件报错，这里尚未开启
paddle2cinn目录的编译，本地单测结果如下：