Fdy change dyn load #1301
Fdy change dyn load #1301
Conversation
fandaoyi
requested review from
lljbash,
yewentao256,
zhaoguochun1995,
caikun-pjlab and
mrdanielw
impl/cmake/ImplHelper.cmake
Outdated
| @@ -0,0 +1,56 @@ | |||
| macro(diopi_use_adapter cmd_extra_config) | |||
There was a problem hiding this comment.
用 macro 是特意的吗，里面定义了很多变量，容易污染相关脚本；另外也通过参数以外的方式直接引用了一些外部定义的变量名，挺容易出错的。
impl/cmake/ImplHelper.cmake
Outdated
| add_custom_target(adaptor_gen_dependency DEPENDS ${ADAPTOR_TEMPLATE_CODE}) | ||
| set(ADAPTOR_CSRC_PATH "${ADAPTOR_DIR}/csrc") | ||
|
|
||
| separate_arguments(GenArgs UNIX_COMMAND "--diopi_dir=${DIOPI_IMPL_DIR}/../ --output_dir=${ADAPTOR_CSRC_PATH} ${cmd_extra_config}") |
There was a problem hiding this comment.
用 list (set)的话传参的地方得改, 不能把几个参数放到一个变量里一起传了, 还要解析参数,
There was a problem hiding this comment.
这个名字起的有些问题, 我改叫 cmd_extra_config's' 吧
There was a problem hiding this comment.
用 list (set)的话传参的地方得改, 不能把几个参数放到一个变量里一起传了, 还要解析参数,
其实是可以的，CMake 里面形如 --a=1;--b=2 会被认为是 ; 分割的数组`
There was a problem hiding this comment.
确实, PS: 杨波要改些东西, 等他改完我把这个改了吧
impl/cmake/ImplHelper.cmake
Outdated
| DEPENDS adaptor_gen_dependency | ||
| VERBATIM | ||
| ) | ||
| list(APPEND REAL_IMPL_SRC ${ADAPTOR_CSRC_PATH}/convert.cpp ${ADAPTOR_CSRC_PATH}/diopi_adaptor.cpp ${ADAPTOR_CSRC_PATH}/composite_ops.cpp) |
There was a problem hiding this comment.
要不换成 concat 或者 set？万一 REAL_IMPL_SRC 这个名字被别的地方用了，不知道会构造出什么样的 list。如果在 function 之内的 scope 就没这个顾虑了，主要是它在 macro 之中
There was a problem hiding this comment.
用 macro 主要是改动少, 我直接把原来外边的代码搬过来了. 不用改啥东西. 你要有空都改了, 可以改成 function.
There was a problem hiding this comment.
我找时间改下吧. macro 确实不太好
支持 mx 和 torch 使用 动态加载的 pytorch (和 dipu 使用的 torch 可以不同)
把原有散落在 torch 目录下的动态加载相关的cmake 做了统一(包含 adapter, dynamic load 的 辅助函数). 原来需要手工处理的 patchelf等部分整合到 shell里. 并修改了 动态加载的逻辑, 可以从 lib的当前目录加载,
动态加载的整体逻辑还是参照之前的, 供参考:
https://aicarrier.feishu.cn/wiki/wikcnWKP9AKUEZKYCUprTNrvpod
https://aicarrier.feishu.cn/wiki/wikcnsNe1FkN0iX07OAcLZXaxkc
遗留问题