Patchwork [RFC] Parallel build broken on trunk.

login
register
mail settings
Submitter Marcus Shawcroft
Date Nov. 20, 2012, 4:35 p.m.
Message ID <CAFqB+Pzh0U3HMng5GgnxBp2cbt+rdj7hDuTcf24DLFwP9CLEwA@mail.gmail.com>
Download mbox | patch
Permalink /patch/200395/
State New
Headers show

Comments

Marcus Shawcroft - Nov. 20, 2012, 4:35 p.m.
Folks, Parallel builds contain a race due to a missing dependency
between gengtype-lex.o and $(BCONFIG_H).

This was introduced by the commit:

http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00926.html

.. which injects an include of bconfig.h into the top of
gengtype-lex.c but does not make both of the objects built from that
file dependent on bconfig.h

There is a simple fix, below, but I'm concerned that this is just
papering over the cracks, is it really correct that both the build and
host machine variants include the same config file?

Cheers
/Marcus
Laurynas Biveinis - Nov. 21, 2012, 9:48 a.m.
> Folks, Parallel builds contain a race due to a missing dependency
> between gengtype-lex.o and $(BCONFIG_H).
>
> This was introduced by the commit:
>
> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00926.html
>
> .. which injects an include of bconfig.h into the top of
> gengtype-lex.c but does not make both of the objects built from that
> file dependent on bconfig.h
>
> There is a simple fix, below, but I'm concerned that this is just
> papering over the cracks, is it really correct that both the build and
> host machine variants include the same config file?

Probably not, but I see your patch not as papering over but rather a
missing piece of what's been committed in 2010. Fixing parallel builds
does not preclude a better fix for build/host separation later.

> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index d74e7b3..8e8f4d3 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3885,7 +3885,7 @@ build/gengenrtl.o : gengenrtl.c $(BCONFIG_H)
> $(SYSTEM_H) rtl.def
>  # the build-%: rule doesn't apply to them.
>
>  gengtype-lex.o build/gengtype-lex.o : gengtype-lex.c gengtype.h $(SYSTEM_H)
> -gengtype-lex.o: $(CONFIG_H)
> +gengtype-lex.o: $(CONFIG_H) $(BCONFIG_H)
>  CFLAGS-gengtype-lex.o += -DGENERATOR_FILE
>  build/gengtype-lex.o: $(BCONFIG_H)

This is OK with a proper ChangeLog entry. Hopefully I am not
overstepping my gengtype reviewer powers here.

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index d74e7b3..8e8f4d3 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3885,7 +3885,7 @@  build/gengenrtl.o : gengenrtl.c $(BCONFIG_H)
$(SYSTEM_H) rtl.def
 # the build-%: rule doesn't apply to them.

 gengtype-lex.o build/gengtype-lex.o : gengtype-lex.c gengtype.h $(SYSTEM_H)
-gengtype-lex.o: $(CONFIG_H)
+gengtype-lex.o: $(CONFIG_H) $(BCONFIG_H)
 CFLAGS-gengtype-lex.o += -DGENERATOR_FILE
 build/gengtype-lex.o: $(BCONFIG_H)