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

Unify core_avx and core_noavx to paddle_core #45895

Closed
wants to merge 5 commits into from

Conversation

chenwhql
Copy link
Contributor

@chenwhql chenwhql commented Sep 8, 2022

PR types

Function optimization

PR changes

Others

Describe

Unify core_avx and core_noavx to paddle_core

之前paddle为了使一个whl包同时支持avx和noavx,采用了一种比较trick的方式,编译两遍paddle,分别编译avx和noavx,最后打包到一起,#17889

这种方式虽然解决了问题,但也引入了新的问题:

  1. 编译效率差
  2. paddle whl包体积大,由于体积过大的问题,之前在linux和windows已经重新拆分为两个不同的包,由用户进行选择下载安装,而mac由于只编译cpu,体积还比较小,并没有进行拆分,因此在修改代码的时候还需要为mac保留原先拆分编译和打包的逻辑
  3. copy修改文件名导致soname不一致,外部无法正确链接,之前通过patchelf修复了此问题,但是patchelf本身也有一些使用限制,比如限制包的使用大小,目前在gcc5.4+cuda10.1、gcc9+cuda11上都会触发包大小上限无法正常使用,因此还是需要从根本上解决此问题,使动态库名称一致且规范,Fix core so name mismatch error #43977

本PR清理了一些历史代码,将动态库名称统一改为paddle_core

同时需要在PaddleCustomDevice中对这两个命名的使用增加一个if分支:https://github.com/PaddlePaddle/PaddleCustomDevice/search?q=core_avx

PaddlePaddle/PaddleCustomDevice#105

TODO:参考linux shared library的命名公约,下个PR更名为libpaddle:https://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html

@paddle-bot
Copy link

paddle-bot bot commented Sep 8, 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.

@chenwhql chenwhql changed the title Unify core_avx and core_noavx to paddle_core Unify core_avx and core_noavx to libpaddle Sep 15, 2022
@chenwhql chenwhql changed the title Unify core_avx and core_noavx to libpaddle Unify core_avx and core_noavx to paddle_core Sep 15, 2022
@@ -1635,6 +1638,7 @@ All parameter, weight, gradient are variables in Paddle.
m.def("init_devices", []() { framework::InitDevices(); });
m.def("init_default_kernel_signatures",
[]() { framework::InitDefaultKernelSignatureMap(); });
m.def("is_compiled_with_avx", IsCompiledWithCUDA);
Copy link
Contributor

Choose a reason for hiding this comment

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

IsCompiledWithCUDA -> IsCompiledWithAVX


if avx_supported():
if platform.system().lower() != 'darwin' or (platform.system().lower()
== 'darwin' and avx_supported()):
Copy link
Contributor

Choose a reason for hiding this comment

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

是不是没有考虑这种情况:用户机器是支持avx的,但是安装了noavx包(只编译noavx的安装包,没有把avx和noavx合并)。
这时候这个包里没有paddle_core?

libs = [':core_avx.so']
if not core.has_avx_core and core.has_noavx_core:
libs = [':core_noavx.so']
libs = [':paddle_core.so']
Copy link
Contributor

Choose a reason for hiding this comment

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

macos上只编noavx会不会有问题?
原来这段判断是否需要对macos特殊处理?

if not core.has_avx_core and core.has_noavx_core:
     libs = [':core_noavx.so']

libs = [':core_avx.so']
if not core.has_avx_core and core.has_noavx_core:
libs = [':core_noavx.so']
libs = [':paddle_core.so']
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

@chenwhql chenwhql closed this Sep 15, 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.

4 participants