Skip to content

remove gfortran and dlopen lapack libs #1958

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

Merged
merged 9 commits into from
May 3, 2017

Conversation

gangliao
Copy link
Contributor

@gangliao gangliao commented May 2, 2017

由于大多数情况下,用不到lapack的三个接口,然后编译lapack又需要使用gfortran,因此这里我们dlopen lapack动态库,如果运行时找不到库,再apt-get install liblapacke-dev安装就行了,不需要编译引入复杂的gfrotran编译器以及库。

@gangliao gangliao changed the title remove gfotran and dlopen lapack api remove gfortran and dlopen lapack api May 2, 2017
@gangliao gangliao requested review from hedaoyuan and Xreki May 2, 2017 08:37
Copy link
Contributor

@Xreki Xreki left a comment

Choose a reason for hiding this comment

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

  • DynamicLoad.h/cpp是不是改成DynamicLoader.h/cpp比较好
  • 使用了动态装载的方式,应该是不需要PADDLE_USE_LAPACK这个宏了吧
  • cblas.cmake里面对lapack库的check是不是也应该去掉
  • openblas.cmake里面最后的一步ExternalProject_Add_Step应该去掉

#else
return LAPACKE_sgetrf(order, M, N, A, lda, ipiv);
return dynload::LAPACKE_sgetrf(order, M, N, A, lda, ipiv);
#endif
#else
LOG(FATAL) << "Not implemented";
Copy link
Contributor

@Xreki Xreki May 2, 2017

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.

done


void GetLapackDsoHandle(void** dso_handle) {
#if defined(__APPLE__) || defined(__OSX__)
GetDsoHandleFromSearchPath(FLAGS_warpctc_dir, "liblapack.dylib", dso_handle);
Copy link
Contributor

Choose a reason for hiding this comment

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

typo:FLAGS_warpctc_dir -> FLAGS_lapack_dir

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

@hedaoyuan hedaoyuan requested a review from lzhao4ever May 2, 2017 09:58
@@ -87,9 +132,9 @@ int getrf<float>(const CBLAS_ORDER order,
int* ipiv) {
#ifdef PADDLE_USE_LAPACK
Copy link
Contributor

Choose a reason for hiding this comment

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

PADDLE_USE_LAPACK这个是否可以去掉了?

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

@gangliao
Copy link
Contributor Author

gangliao commented May 2, 2017

DynamicLoad.h/cpp是不是改成DynamicLoader.h/cpp比较好

done

使用了动态装载的方式,应该是不需要PADDLE_USE_LAPACK这个宏了吧

done

cblas.cmake里面对lapack库的check是不是也应该去掉

done

openblas.cmake里面最后的一步ExternalProject_Add_Step应该去掉

这个可以保留,ExternalProject_Add_Step只是copy了几个lapacke头文件, 这个lapacke.h在编译的时候,是需要的。

@gangliao gangliao changed the title remove gfortran and dlopen lapack api remove gfortran and dlopen lapack libs May 2, 2017
@lzhao4ever
Copy link
Contributor

如果使用了动态装载的方式, 不需要。

@gangliao
Copy link
Contributor Author

gangliao commented May 3, 2017

@lzhao4ever 确实不需要了

Copy link
Contributor

@Xreki Xreki left a comment

Choose a reason for hiding this comment

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

LGTM

@gangliao gangliao merged commit d10f6cf into PaddlePaddle:develop May 3, 2017
emailweixu pushed a commit to emailweixu/Paddle that referenced this pull request May 3, 2017
This only fixes the issue for ATLAS
MKL is still another fix.
gangliao added a commit that referenced this pull request May 4, 2017
Fix dynamic loading of Lapack caused by #1958
heavengate pushed a commit to heavengate/Paddle that referenced this pull request Aug 16, 2021
* add christmas application

* fix element source
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