[PATCH 1/2] gdbserver/Makefile.in: Remove ADD_DEPS

[PATCH 1/2] gdbserver/Makefile.in: Remove ADD_DEPS

[Simon Marchi]

ADD_DEPS is defined nowhere, so I presume it's not useful.  If I'm wrong
and this is actually used, there should be a comment explaining where it
comes from.

gdb/gdbserver/Makefile.in:

	* Makefile.in: Remove references to $(ADD_DEPS).
---
 gdb/gdbserver/Makefile.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f2f8a084bdfe..c07f84d51209 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -395,7 +395,7 @@ install-html:
 clean-info: force
 	@$(MAKE) $(FLAGS_TO_PASS) DO=$@ "DODIRS=$(SUBDIRS)" subdir_do
 
-gdbserver$(EXEEXT): $(OBS) ${ADD_DEPS} ${CDEPS} $(LIBGNU) $(LIBIBERTY)
+gdbserver$(EXEEXT): $(OBS) ${CDEPS} $(LIBGNU) $(LIBIBERTY)
 	$(SILENCE) rm -f gdbserver$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
 		-o gdbserver$(EXEEXT) $(OBS) $(LIBGNU) $(LIBIBERTY) \
@@ -430,7 +430,7 @@ IPA_OBJS = \
 
 IPA_LIB = libinproctrace.so
 
-$(IPA_LIB): $(IPA_OBJS) ${ADD_DEPS} ${CDEPS}
+$(IPA_LIB): $(IPA_OBJS) ${CDEPS}
 	$(SILENCE) rm -f $(IPA_LIB)
 	$(ECHO_CXXLD) $(CC_LD) -shared -fPIC -Wl,--soname=$(IPA_LIB) \
 		-Wl,--no-undefined $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
-- 
2.18.0

[PATCH 2/2] Sort objects in gdb and gdbserver Makefiles

[Simon Marchi]

Tom mentioned this a while ago, as a way to give you a cheap sense of
progression in your build, as all object files will be built
alphabetically (including the directory part).  I tried it and I think
it's nice.

gdb/ChangeLog:

	* Makefile.in (LIBGDB_OBS): Sort COMMON_OBS.

gdb/gdbserver/ChangeLog:

	* Makefile.in (gdbserver$(EXEEXT)): Sort OBS.
	(gdbreplay$(EXEEXT)): Sort GDBREPLAY_OBS.
	($(IPA_LIB)): Sort IPA_OBJS.
---
 gdb/Makefile.in           | 2 +-
 gdb/gdbserver/Makefile.in | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d49f3eef446e..87eb825aa991 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1888,7 +1888,7 @@ stamp-init: $(INIT_FILES)
 # against that.
 #
 # init.o is very important.  It pulls in the rest of GDB.
-LIBGDB_OBS = $(COMMON_OBS) init.o
+LIBGDB_OBS = $(sort $(COMMON_OBS)) init.o
 libgdb.a: $(LIBGDB_OBS)
 	-rm -f libgdb.a
 	$(AR) q libgdb.a $(LIBGDB_OBS)
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index c07f84d51209..dfefff915545 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -395,7 +395,7 @@ install-html:
 clean-info: force
 	@$(MAKE) $(FLAGS_TO_PASS) DO=$@ "DODIRS=$(SUBDIRS)" subdir_do
 
-gdbserver$(EXEEXT): $(OBS) ${CDEPS} $(LIBGNU) $(LIBIBERTY)
+gdbserver$(EXEEXT): $(sort $(OBS)) ${CDEPS} $(LIBGNU) $(LIBIBERTY)
 	$(SILENCE) rm -f gdbserver$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
 		-o gdbserver$(EXEEXT) $(OBS) $(LIBGNU) $(LIBIBERTY) \
@@ -406,7 +406,7 @@ all-lib: $(GNULIB_BUILDDIR)/Makefile $(LIBIBERTY_BUILDDIR)/Makefile
 	@$(MAKE) $(FLAGS_TO_PASS) DO=all DODIRS="$(SUBDIRS)" subdir_do
 .PHONY: all-lib
 
-gdbreplay$(EXEEXT): $(GDBREPLAY_OBS) $(LIBGNU) $(LIBIBERTY)
+gdbreplay$(EXEEXT): $(sort $(GDBREPLAY_OBS)) $(LIBGNU) $(LIBIBERTY)
 	$(SILENCE) rm -f gdbreplay$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
 		-o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) $(XM_CLIBS) $(LIBGNU) \
@@ -430,7 +430,7 @@ IPA_OBJS = \
 
 IPA_LIB = libinproctrace.so
 
-$(IPA_LIB): $(IPA_OBJS) ${CDEPS}
+$(IPA_LIB): $(sort $(IPA_OBJS)) ${CDEPS}
 	$(SILENCE) rm -f $(IPA_LIB)
 	$(ECHO_CXXLD) $(CC_LD) -shared -fPIC -Wl,--soname=$(IPA_LIB) \
 		-Wl,--no-undefined $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
-- 
2.18.0

Re: [PATCH 1/2] gdbserver/Makefile.in: Remove ADD_DEPS

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> ADD_DEPS is defined nowhere, so I presume it's not useful.  If I'm wrong
Simon> and this is actually used, there should be a comment explaining where it
Simon> comes from.

git grep shows no other hits, so I agree it's unused.

Tom

Re: [PATCH 2/2] Sort objects in gdb and gdbserver Makefiles

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Tom mentioned this a while ago, as a way to give you a cheap sense of
Simon> progression in your build, as all object files will be built
Simon> alphabetically (including the directory part).  I tried it and I think
Simon> it's nice.

Simon> gdb/ChangeLog:

Simon> 	* Makefile.in (LIBGDB_OBS): Sort COMMON_OBS.

Simon> gdb/gdbserver/ChangeLog:

Simon> 	* Makefile.in (gdbserver$(EXEEXT)): Sort OBS.
Simon> 	(gdbreplay$(EXEEXT)): Sort GDBREPLAY_OBS.
Simon> 	($(IPA_LIB)): Sort IPA_OBJS.

I think it is a nice idea.

The only counter-case I could think of would be if gdb had some object
whose build time was notably slower than everything else.  In that case
you'd want to start it earlier for better parallelization.

However I don't think that's the case, or at least I haven't noticed;
and anyway it isn't like we've micro-optimized the build that way
anyhow.

Tom

Re: [PATCH 2/2] Sort objects in gdb and gdbserver Makefiles

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Tom mentioned this a while ago, as a way to give you a cheap sense of
Simon> progression in your build, as all object files will be built
Simon> alphabetically (including the directory part).

Try this for fun.

Tom

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index d49f3eef446..366abba69fe 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1894,11 +1894,27 @@ libgdb.a: $(LIBGDB_OBS)
 	$(AR) q libgdb.a $(LIBGDB_OBS)
 	$(RANLIB) libgdb.a
 
+ALL_GDB_OBS = gdb.o $(LIBGDB_OBS)
+
+# Usage: set-object-index LIST
+# Where LIST is a list WORD1...WORDN
+# Define $(WORDN)-index to an integer and then recurse on the
+# remaining words.
+define set-object-index =
+$(if $(1),
+  $(lastword $(1))-index := $(words $(1))
+  $(call set-object-index,$(wordlist 1,$(words $(wordlist 2,100000,$(1))),$(1))))
+endef
+
+$(eval $(call set-object-index,$(ALL_GDB_OBS)))
+
+override ECHO_CXX = @echo "[$($@-index) / $(words $(ALL_GDB_OBS))]  CXX    $@  ";
+
 # Removing the old gdb first works better if it is running, at least on SunOS.
-gdb$(EXEEXT): gdb.o $(LIBGDB_OBS) $(CDEPS) $(TDEPLIBS)
+gdb$(EXEEXT): $(ALL_GDB_OBS) $(CDEPS) $(TDEPLIBS)
 	$(SILENCE) rm -f gdb$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_LDFLAGS) $(WIN32LDAPP) \
-		-o gdb$(EXEEXT) gdb.o $(LIBGDB_OBS) \
+		-o gdb$(EXEEXT) $(ALL_GDB_OBS) \
 		$(TDEPLIBS) $(TUI_LIBRARY) $(CLIBS) $(LOADLIBES)
 ifneq ($(CODESIGN_CERT),)
 	$(ECHO_SIGN) $(CODESIGN) -s $(CODESIGN_CERT) gdb$(EXEEXT)

Re: [PATCH 2/2] Sort objects in gdb and gdbserver Makefiles

[Simon Marchi]

On 2018-09-16 16:08, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> Tom mentioned this a while ago, as a way to give you a cheap 
> sense of
> Simon> progression in your build, as all object files will be built
> Simon> alphabetically (including the directory part).
> 
> Try this for fun.

Ah, impressive!  Though to be really useful, it would have to tell you 
how many objects are required to be built this particular time.  For 
example, you touch a header file and 100 .o files need to be rebuilt, it 
would show as "[X / 100]".  Then you would know if you have time to get 
coffee or not.

I pushed my two patches.

Thanks,

Simon

Re: [PATCH 2/2] Sort objects in gdb and gdbserver Makefiles

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Ah, impressive!  Though to be really useful, it would have to tell you
Simon> how many objects are required to be built this particular time.

Yeah.  I couldn't think of a way to do this though.

Tom