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

Makefile improvements: rule consolidation, dependency generation #1472

Merged
merged 4 commits into from
Dec 29, 2014

Conversation

longjon
Copy link
Contributor

@longjon longjon commented Nov 24, 2014

This PR addresses #599 using the -MMD option of gcc/clang (and the -M option of nvcc) to automatically generate header file dependencies, finally making the build truly incremental.

In addition, some consolidation of build rules is included (more might be possible), and the explicit enumeration of build directories is made a bit more automatic (further improvements in this direction are also possible).

This is pretty fresh so you might want to let it age a couple days in case I messed something up; I'll be working off this branch.

@@ -472,15 +464,19 @@ $(STATIC_NAME): $(OBJS) | $(LIB_BUILD_DIR)
ar rcs $@ $(OBJS)
@ echo

$(TEST_BUILD_DIR)/%.o: src/$(PROJECT)/test/%.cpp $(HXX_SRCS) $(TEST_HXX_SRCS) \
| $(TEST_BUILD_DIR)
$(BUILD_DIR)/%.o: %.cpp | $(ALL_BUILD_DIRS)
Copy link
Contributor

Choose a reason for hiding this comment

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

How does make know the object files also depend on whichever automatically generated *.hpp header dependencies gcc says are needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make merges multiple rules into one with a union over prerequisites, see https://www.gnu.org/software/make/manual/html_node/Multiple-Rules.html. The additional rules are provided in the .d files included in the last line of the Makefile. (Note that on first build, the .d files don't exist, but this doesn't matter, because everything has to be built anyway.)

@jeffdonahue
Copy link
Contributor

Cool, LGTM! Thanks for setting up the automatic dependency generation, that should speed up builds quite a bit.

@sguada
Copy link
Contributor

sguada commented Nov 26, 2014

Nice job

On Wednesday, November 26, 2014, Jeff Donahue notifications@github.com
wrote:

Cool, LGTM! Thanks for setting up the automatic dependency generation,
that should speed up builds quite a bit.


Reply to this email directly or view it on GitHub
#1472 (comment).

Sergio

@shelhamer
Copy link
Member

Nice! Sorry if I'm blind, but is the nvcc -M part missing?

@longjon
Copy link
Contributor Author

longjon commented Dec 2, 2014

@shelhamer, that's weird, where did that go? I'm glad somebody is paying attention (and git reflog -G exists).

@longjon
Copy link
Contributor Author

longjon commented Dec 11, 2014

Two bugs have been fixed.

  1. Dependency generation was broken for .cu files.

    nvcc uses a different interfaces from gcc for specifying the name of the target and the output dependency file. In particular, the -odir option needs to be used if output is in a different directory, and nvcc assumes the target will have a .o extension. Rather than hack around that assumption, I changed the build targets for .cu files from .cuo files to .o files that live in a separate tree under build/cuda. This might be a little less convenient for certain purposes, but it's more harmonious with the assumptions of nvcc, and it's unusual to dig for object files in the build directory anyway.

  2. make clean was not removing the .build_* directories, and consequently the build symlink was not being recreated after a make clean.

@longjon longjon mentioned this pull request Dec 11, 2014
This will allow nvcc's -M dependency generation option to work
harmoniously, since it assumes that output will have a .o extension.
@longjon
Copy link
Contributor Author

longjon commented Dec 29, 2014

Two more bugs have been fixed.

  1. Dependency generation of test object files was broken, because test dependency files were not included.
  2. There is an untracked dependency of the generated caffe_pretty_print.pb.cc on the generated caffe.pb.h. I normally did not experience a problem when using make -j, as the race was always won; however, make (-j1) revealed the issue. The fix is to use the old trick for protoc generated files: all .pb.o files depend on all .pb.h ones (there are just two of each).

I know of no other issues that have come up in my use of this branch, so (with @jeffdonahue's approval a few bugs ago) I think I'll merge this now (and #1561).

Note that building may still be slow sometimes, because of the monolithic header files and test binary. I'll make an issue about that shortly.

longjon added a commit that referenced this pull request Dec 29, 2014
Makefile improvements: rule consolidation, dependency generation
@longjon longjon merged commit abd2ea3 into BVLC:dev Dec 29, 2014
@longjon longjon deleted the incremental-build branch December 30, 2014 04:59
ronghanghu added a commit to ronghanghu/caffe that referenced this pull request May 28, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
ronghanghu added a commit to ronghanghu/caffe that referenced this pull request May 28, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
ronghanghu added a commit to ronghanghu/caffe that referenced this pull request May 29, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
ShaggO pushed a commit to ShaggO/caffe that referenced this pull request May 29, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
matthiasplappert pushed a commit to matthiasplappert/caffe that referenced this pull request Aug 10, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
cbfinn pushed a commit to cbfinn/caffe that referenced this pull request Aug 12, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
wangyida pushed a commit to wangyida/caffe that referenced this pull request Sep 22, 2015
Automatic header file dependency was introduced in BVLC#1472, but
not correctly applied to matcaffe. Fix it by moving ./caffe_.d
to build/matlab/+caffe/private/caffe_.d and add it to DEPS
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