Patchwork [build] improve dependency graph for build/gen%

login
register
mail settings
Submitter Ralf Wildenhues
Date Sept. 29, 2010, 8:38 p.m.
Message ID <20100929203824.GD25571@gmx.de>
Download mbox | patch
Permalink /patch/66095/
State New
Headers show

Comments

Ralf Wildenhues - Sept. 29, 2010, 8:38 p.m.
Pattern rules
  foo% : bar% baz
        ...

have a distinct disadvantage over static pattern rules
  foo1 foo2 foo3 foo4 : foo% : bar% baz
        ...

in that GNU make applies the former only if /some/ prerequisite of a
target actually exists or can be made.  Most of the time, this is not
a problem, because either the makefile has all remaining information to
make a prerequisite, or the rest of the tree has been built previously
anyway.

However, if the rest of the tree is not built, maybe because toplevel
had missing dependency edges, then this can lead to weird error messages
or 'make' wrongly thinking everything is up to date.

For example, in an up to date build tree,
  cd gcc
  rm -f ../libiberty/libiberty.a ../build-$build/libiberty/libiberty.a
  make

will *not* fail, although build/genhooks and others depend on
libiberty.a in one of the above places (an updated libiberty.a will
however cause the needed relinks).

This issue caused the weird error message in
<http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45796>.

The "downside" of static pattern rules is that all targets need to be
listed explicitly.  The patch does this by moving the pattern rule and
adjusting the rest.

Tested with a full bootstrap, with a --disable-bootstrap build, and by
removing some files arbitrarily and visually checking incremental
rebuild output for consistency.

OK for trunk?

This does not fix the PR yet, but makes the failure obvious.

Thanks,
Ralf

build: more correct build rules for build/gen% programs.

gcc/ChangeLog:
2010-09-29  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

	PR bootstrap/45796
	* Makefile.in (build/gen%$(build_exeext)): Move rule after all
	special-casing for generators and turn into ...
	((genprog:%=build/gen%$(build_exeext))): ... this static pattern
	rule, for better error messages in case of toplevel dependency
	errors.
	(genprog): Add hooks, rename to ...
	(genprogerr): ... this, and let genprog also contain check,
	checksum, condmd.
	((genprog:%=build/gen%$(build_exeext))): Rename to ...
	((genprogerr:%=build/gen%$(build_exeext))): ... this.
	(build/genhooks$(build_exeext)): Remove now-unneeded dependency.
Paolo Bonzini - Sept. 30, 2010, 10:30 a.m.
> 2010-09-29  Ralf Wildenhues<Ralf.Wildenhues@gmx.de>
>
> 	PR bootstrap/45796
> 	* Makefile.in (build/gen%$(build_exeext)): Move rule after all
> 	special-casing for generators and turn into ...
> 	((genprog:%=build/gen%$(build_exeext))): ... this static pattern
> 	rule, for better error messages in case of toplevel dependency
> 	errors.
> 	(genprog): Add hooks, rename to ...
> 	(genprogerr): ... this, and let genprog also contain check,
> 	checksum, condmd.
> 	((genprog:%=build/gen%$(build_exeext))): Rename to ...
> 	((genprogerr:%=build/gen%$(build_exeext))): ... this.
> 	(build/genhooks$(build_exeext)): Remove now-unneeded dependency.

Ok, thanks!

Paolo

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 0c78323..36b6349 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3904,11 +3904,6 @@  build/genhooks.o : genhooks.c target.def $(BCONFIG_H) $(SYSTEM_H) errors.h
 # since they need to run on this machine
 # even if GCC is being compiled to run on some other machine.
 
-# As a general rule...
-build/gen%$(build_exeext): build/gen%.o $(BUILD_LIBDEPS)
-	$(LINKER_FOR_BUILD) $(BUILD_LINKERFLAGS) $(BUILD_LDFLAGS) -o $@ \
-	    $(filter-out $(BUILD_LIBDEPS), $^) $(BUILD_LIBS)
-
 # All these programs use the RTL reader ($(BUILD_RTL)).
 genprogrtl = attr attrtab automata codes conditions config emit \
 	     extract flags opinit output peep preds recog
@@ -3918,16 +3913,23 @@  $(genprogrtl:%=build/gen%$(build_exeext)): $(BUILD_RTL)
 genprogmd = $(genprogrtl) mddeps constants enums
 $(genprogmd:%=build/gen%$(build_exeext)): $(BUILD_MD)
 
-# All generator programs need to report errors
-genprog = $(genprogmd) genrtl modes gtype
-$(genprog:%=build/gen%$(build_exeext)): $(BUILD_ERRORS)
+# All these programs need to report errors.
+genprogerr = $(genprogmd) genrtl modes gtype hooks
+$(genprogerr:%=build/gen%$(build_exeext)): $(BUILD_ERRORS)
+
+# Remaining build programs.
+genprog = $(genprogerr) check checksum condmd
 
 # These programs need libs over and above what they get from the above list.
 build/genautomata$(build_exeext) : BUILD_LIBS += -lm
 
 # These programs are not linked with the MD reader.
 build/gengtype$(build_exeext) : build/gengtype-lex.o build/gengtype-parse.o
-build/genhooks$(build_exeext) : $(BUILD_ERRORS)
+
+# Rule for the generator programs:
+$(genprog:%=build/gen%$(build_exeext)): build/gen%$(build_exeext): build/gen%.o $(BUILD_LIBDEPS)
+	$(LINKER_FOR_BUILD) $(BUILD_LINKERFLAGS) $(BUILD_LDFLAGS) -o $@ \
+	    $(filter-out $(BUILD_LIBDEPS), $^) $(BUILD_LIBS)
 
 # Generated source files for gengtype.
 gengtype-lex.c : gengtype-lex.l