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

ENH: Merge multiple static libs into singe one #2533

Merged
merged 3 commits into from
Jun 21, 2017

Conversation

gangliao
Copy link
Contributor

@gangliao gangliao commented Jun 20, 2017

ENHANCEMENT:

If we have lots of source files under framework folder, each of them will invoke cc_library(filename SRC ...) to generate *.a, finally, we must provide a target name framework (framework.a) for the outside usage.

Therefore, we need to merge multiple small libraries into larger one libframework.a. This PR is to solve this problem.

For example,
If you want to merge libddim.a and libplace.a ... into libframework.a, you can use cc_library as follows:

cc_library(framework DEPS ddim place)

After this PR be merged, we can completely refactor the underlying building system of Paddle.

@gangliao gangliao requested review from wangkuiyi and Xreki June 20, 2017 13:47
@gangliao
Copy link
Contributor Author

This PR is verified, please try it.

CI failed because of no space left on device.
Error response from daemon: Error processing tar file(exit status 1): write /build/paddle-0.1.1-Linux.deb: no space left on device

Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

if (cc_library_DEPS)
merge_static_libs(${TARGET_NAME} ${cc_library_DEPS})
else()
message(FATAL "Please specify source file or library in cc_library.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about changing the error message to be "cc_library requires at least SRCS or DEPS"?

# tensor.cc
# DEPS
# variant)
function(merge_static_libs TARGET_NAME)
Copy link
Collaborator

Choose a reason for hiding this comment

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

merge ==> compose ?

if (nv_library_DEPS)
merge_static_libs(${TARGET_NAME} ${nv_library_DEPS})
else()
message(FATAL "Please specify source file or library in nv_library.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about "nv_library requires at least SRCS or DEPS"?

@wangkuiyi wangkuiyi merged commit 603fd43 into PaddlePaddle:develop Jun 21, 2017
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.

2 participants