| Message ID | 1286454385-3116-1-git-send-email-andi@firstfloor.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 07, 2010 at 02:26:25PM +0200, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > Linking the compiler first to generate the compiler checksum > is extremly costly on a LTO bootstrap build. The link phase > has to recompile everything. > > Change the checksum computation to run the checksum over the > object files and a file containing the link options instead. > This should be about as good and is much faster. > > Based on discussions with various people. > > Passed bootstrap and test suite on x86_64-linux. Ok to commit? > 2010-10-07 Andi Kleen <ak@linux.intel.com> Two spaces between name and email address. > * Makefile.in (MOSTLYCLEANFILES): Remove cc1*dummy, add > checksum-options. > (checksum-options): Add. > (cc1-dummy): Remove. > (cc1-checksum): Change to run checksum over object files > and options only. > * dummy-checksum.c: Remove. > > gcc/cp > > 2010-10-07 Andi Kleen <ak@linux.intel.com> > > * Makefile.in (c++_OBJS): Remove dummy-checksum.o. > (cc1plus-dummy): Remove. > (checksum-options): Add. > (cc1plus-checksum): Change to run checksum over object files > and options only. This is Make-lang.in, not Makefile.in. > * genchecksum.c: Update copyright. > (usage): Allow multiple arguments. > (BLOCKSIZE): Add. > (dosum): Change for incremental checksum. Remove C output. > (main): Iterate over all argument files. Add C output. This belongs to gcc/ChangeLog, not gcc/cp/ChangeLog. Furthermore, objc/Make-lang.in and objcp/Make-lang.in probably need similar changes. Jakub
On Thursday 07 October 2010 13:26:25, Andi Kleen wrote: > -dummy-checksum.o : dummy-checksum.c $(CONFIG_H) $(SYSTEM_H) > +.PHONY: checksum-options > > -cc1-dummy$(exeext): $(C_OBJS) dummy-checksum.o $(BACKEND) $(LIBDEPS) > - +$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ $(C_OBJS) \ > - dummy-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS) > +checksum-options: > + echo "$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS)" > checksum-options I guess this could write to a temporary and do move-if-change. > > -cc1-checksum.c : cc1-dummy$(exeext) build/genchecksum$(build_exeext) > - build/genchecksum$(build_exeext) cc1-dummy$(exeext) > $@ > +# compute checksum over all object files and the options > +cc1-checksum.c : build/genchecksum$(build_exeext) checksum-options \ > + $(C_OBJS) $(BACKEND) $(LIBDEPS) > + build/genchecksum$(build_exeext) $(C_OBJS) $(BACKEND) $(LIBDEPS) \ > + checksum-options > $@ and so you could remove the .PHONY above, so that this isn't rebuilt everytime, and so that cc1 isn't relinked everytime, even if nothing changed.
Pedro Alves <pedro@codesourcery.com> writes: > >> >> -cc1-checksum.c : cc1-dummy$(exeext) build/genchecksum$(build_exeext) >> - build/genchecksum$(build_exeext) cc1-dummy$(exeext) > $@ >> +# compute checksum over all object files and the options >> +cc1-checksum.c : build/genchecksum$(build_exeext) checksum-options \ >> + $(C_OBJS) $(BACKEND) $(LIBDEPS) >> + build/genchecksum$(build_exeext) $(C_OBJS) $(BACKEND) $(LIBDEPS) \ >> + checksum-options > $@ > > and so you could remove the .PHONY above, so that this isn't rebuilt > everytime, and so that cc1 isn't relinked everytime, even if nothing > changed. The reason I did the .PHONY is to catch the case when someone changes the options passed in ALL_LINKERFLAGS etc. Is that not a problem or is there a way to detect it without .PHONY? -Andi
On Thursday 07 October 2010 14:05:03, Andi Kleen wrote: > Pedro Alves <pedro@codesourcery.com> writes: > > > >> > >> -cc1-checksum.c : cc1-dummy$(exeext) build/genchecksum$(build_exeext) > >> - build/genchecksum$(build_exeext) cc1-dummy$(exeext) > $@ > >> +# compute checksum over all object files and the options > >> +cc1-checksum.c : build/genchecksum$(build_exeext) checksum-options \ > >> + $(C_OBJS) $(BACKEND) $(LIBDEPS) > >> + build/genchecksum$(build_exeext) $(C_OBJS) $(BACKEND) $(LIBDEPS) \ > >> + checksum-options > $@ > > > > and so you could remove the .PHONY above, so that this isn't rebuilt > > everytime, and so that cc1 isn't relinked everytime, even if nothing > > changed. > > The reason I did the .PHONY is to catch the case when someone changes > the options passed in ALL_LINKERFLAGS etc. > > Is that not a problem or is there a way to detect it without .PHONY? I think that's solvable in the same way as the tree-check.h rule? There's an explanation just above that rule.
On Oct 7, 2010, at 5:26 AM, Andi Kleen <andi@firstfloor.org> wrote: > Change the checksum computation to run the checksum over the > object files and a file containing the link options instead. > This should be about as good and is much faster. From the pch perspective the change sounds fine. Linking needs to be deterministic. I think determinism ought to be a documented property of gcc and ld. >
On Oct 7, 2010, Mike Stump <mikestump@comcast.net> wrote: > On Oct 7, 2010, at 5:26 AM, Andi Kleen <andi@firstfloor.org> wrote: >> Change the checksum computation to run the checksum over the >> object files and a file containing the link options instead. >> This should be about as good and is much faster. Thank you thank you thank you! :-) > Linking needs to be deterministic. Which brings to mind -fwhopr=jobserver. Is it deterministic, with a varying number of jobs to share the recompile workload? Parallel builds would be quite painful otherwise.
On Fri, Oct 08, 2010 at 02:41:54AM -0300, Alexandre Oliva wrote: > On Oct 7, 2010, Mike Stump <mikestump@comcast.net> wrote: > > > On Oct 7, 2010, at 5:26 AM, Andi Kleen <andi@firstfloor.org> wrote: > >> Change the checksum computation to run the checksum over the > >> object files and a file containing the link options instead. > >> This should be about as good and is much faster. > > Thank you thank you thank you! :-) I should add it probably only makes a big difference if you're using a LTO bootstrap. Otherwise the link step is not too expensive. > > > Linking needs to be deterministic. > > Which brings to mind -fwhopr=jobserver. Is it deterministic, with a > varying number of jobs to share the recompile workload? Parallel builds > would be quite painful otherwise. The way WHOPR works is that it generates a makefile for the WPA stages. The only thing jobserver changes is how parallel this makefile is executed. jobserver does not change the actual partitioning, that is always done by the sequential LTRANS stage. So it should not change the code at all. -Andi
diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 3d936f6..629d33f 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1489,10 +1489,10 @@ BACKEND = main.o @TREEBROWSER@ libbackend.a $(CPPLIB) $(LIBDECNUMBER) MOSTLYCLEANFILES = insn-flags.h insn-config.h insn-codes.h \ insn-output.c insn-recog.c insn-emit.c insn-extract.c insn-peep.c \ insn-attr.h insn-attrtab.c insn-opinit.c insn-preds.c insn-constants.h \ - tm-preds.h tm-constrs.h \ + tm-preds.h tm-constrs.h checksum-options \ tree-check.h min-insn-modes.c insn-modes.c insn-modes.h \ genrtl.h gt-*.h gtype-*.h gtype-desc.c gtyp-input.list \ - xgcc$(exeext) cpp$(exeext) cc1$(exeext) cc1*-dummy$(exeext) $(EXTRA_PASSES) \ + xgcc$(exeext) cpp$(exeext) cc1$(exeext) $(EXTRA_PASSES) \ $(EXTRA_PARTS) $(EXTRA_PROGRAMS) gcc-cross$(exeext) \ $(SPECS) collect2$(exeext) lto-wrapper$(exeext) \ gcov-iov$(build_exeext) gcov$(exeext) gcov-dump$(exeext) \ @@ -1832,14 +1832,16 @@ $(SPECS): xgcc$(exeext) gcc-cross$(exeext): xgcc$(exeext) cp xgcc$(exeext) gcc-cross$(exeext) -dummy-checksum.o : dummy-checksum.c $(CONFIG_H) $(SYSTEM_H) +.PHONY: checksum-options -cc1-dummy$(exeext): $(C_OBJS) dummy-checksum.o $(BACKEND) $(LIBDEPS) - +$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ $(C_OBJS) \ - dummy-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS) +checksum-options: + echo "$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS)" > checksum-options -cc1-checksum.c : cc1-dummy$(exeext) build/genchecksum$(build_exeext) - build/genchecksum$(build_exeext) cc1-dummy$(exeext) > $@ +# compute checksum over all object files and the options +cc1-checksum.c : build/genchecksum$(build_exeext) checksum-options \ + $(C_OBJS) $(BACKEND) $(LIBDEPS) + build/genchecksum$(build_exeext) $(C_OBJS) $(BACKEND) $(LIBDEPS) \ + checksum-options > $@ cc1-checksum.o : cc1-checksum.c $(CONFIG_H) $(SYSTEM_H) diff --git a/gcc/cp/Make-lang.in b/gcc/cp/Make-lang.in index 4be40b5..62de970 100644 --- a/gcc/cp/Make-lang.in +++ b/gcc/cp/Make-lang.in @@ -86,17 +86,21 @@ CXX_AND_OBJCXX_OBJS = cp/call.o cp/decl.o cp/expr.o cp/pt.o cp/typeck2.o \ # Language-specific object files for C++. CXX_OBJS = cp/cp-lang.o c-family/stub-objc.o $(CXX_AND_OBJCXX_OBJS) -c++_OBJS = $(CXX_OBJS) dummy-checksum.o cc1plus-checksum.o cp/g++spec.o +c++_OBJS = $(CXX_OBJS) cc1plus-checksum.o cp/g++spec.o # Use strict warnings for this front end. cp-warn = $(STRICT_WARN) -cc1plus-dummy$(exeext): $(CXX_OBJS) dummy-checksum.o $(BACKEND) $(LIBDEPS) - $(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \ - $(CXX_OBJS) dummy-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS) +.PHONY: checksum-options -cc1plus-checksum.c : cc1plus-dummy$(exeext) build/genchecksum$(build_exeext) - build/genchecksum$(build_exeext) cc1plus-dummy$(exeext) > $@ +checksum-options: + echo "$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS)" > checksum-options + +# compute checksum over all object files and the options +cc1plus-checksum.c : build/genchecksum$(build_exeext) checksum-options \ + $(CXX_OBJS) $(BACKEND) $(LIBDEPS) + build/genchecksum$(build_exeext) $(CXX_OBJS) $(BACKEND) $(LIBDEPS) \ + checksum-options > $@ cc1plus-checksum.o : cc1plus-checksum.c $(CONFIG_H) $(SYSTEM_H) diff --git a/gcc/dummy-checksum.c b/gcc/dummy-checksum.c deleted file mode 100644 index c90f1ca..0000000 --- a/gcc/dummy-checksum.c +++ /dev/null @@ -1,3 +0,0 @@ -#include "config.h" -#include "system.h" -EXPORTED_CONST unsigned char executable_checksum[16] = { 0 }; diff --git a/gcc/genchecksum.c b/gcc/genchecksum.c index 235d4fe..e0c71f4 100644 --- a/gcc/genchecksum.c +++ b/gcc/genchecksum.c @@ -1,5 +1,5 @@ /* Generate checksums of executables for PCH validation - Copyright (C) 2005, 2007, 2009 + Copyright (C) 2005, 2007, 2009, 2010 Free Software Foundation, Inc. This file is part of GCC. @@ -25,15 +25,18 @@ along with GCC; see the file COPYING3. If not see static void usage (void) { - fputs ("Usage: genchecksums <filename>\n", stderr); + fputs ("Usage: genchecksums <filename> ...\n", stderr); } +/* Important: BLOCKSIZE must be a multiple of 64. */ +#define BLOCKSIZE 4096 + static void -dosum (const char *file) +dosum (struct md5_ctx *ctx, const char *file) { FILE *f; - unsigned char result[16]; - int i; + char buffer[BLOCKSIZE + 72]; + size_t sum; f = fopen (file, "rb"); if (!f) @@ -49,30 +52,70 @@ dosum (const char *file) exit (1); } - if (md5_stream (f, result) != 0 - || fclose (f) != 0) + /* Iterate over full file contents. */ + while (1) + { + /* We read the file in blocks of BLOCKSIZE bytes. One call of the + computation function processes the whole buffer so that with the + next round of the loop another block can be read. */ + size_t n; + sum = 0; + + /* Read block. Take care for partial reads. */ + do + { + n = fread (buffer + sum, 1, BLOCKSIZE - sum, f); + + sum += n; + } + while (sum < BLOCKSIZE && n != 0); + if (n == 0 && ferror (f)) + exit (1); + + /* If end of file is reached, end the loop. */ + if (n == 0) + break; + + /* Process buffer with BLOCKSIZE bytes. Note that + BLOCKSIZE % 64 == 0 + */ + md5_process_block (buffer, BLOCKSIZE, ctx); + } + + /* Add the last bytes if necessary. */ + if (sum > 0) + md5_process_bytes (buffer, sum, ctx); + + if (fclose (f) != 0) { fprintf (stderr, "reading %s: %s\n", file, xstrerror (errno)); exit (1); } - - puts ("#include \"config.h\""); - puts ("#include \"system.h\""); - fputs ("EXPORTED_CONST unsigned char executable_checksum[16] = { ", stdout); - for (i = 0; i < 16; i++) - printf ("0x%02x%s", result[i], i == 15 ? " };\n" : ", "); } int main (int argc, char ** argv) { - if (argc != 2) + struct md5_ctx ctx; + unsigned char result[16]; + int i; + + if (argc < 2) { usage (); return 1; } - dosum (argv[1]); + md5_init_ctx (&ctx); + for (i = 1; i < argc; i++) + dosum (&ctx, argv[i]); + md5_finish_ctx (&ctx, result); + + puts ("#include \"config.h\""); + puts ("#include \"system.h\""); + fputs ("EXPORTED_CONST unsigned char executable_checksum[16] = { ", stdout); + for (i = 0; i < 16; i++) + printf ("0x%02x%s", result[i], i == 15 ? " };\n" : ", "); return 0; }
From: Andi Kleen <ak@linux.intel.com> Linking the compiler first to generate the compiler checksum is extremly costly on a LTO bootstrap build. The link phase has to recompile everything. Change the checksum computation to run the checksum over the object files and a file containing the link options instead. This should be about as good and is much faster. Based on discussions with various people. Passed bootstrap and test suite on x86_64-linux. Ok to commit? gcc/ 2010-10-07 Andi Kleen <ak@linux.intel.com> * Makefile.in (MOSTLYCLEANFILES): Remove cc1*dummy, add checksum-options. (checksum-options): Add. (cc1-dummy): Remove. (cc1-checksum): Change to run checksum over object files and options only. * dummy-checksum.c: Remove. gcc/cp 2010-10-07 Andi Kleen <ak@linux.intel.com> * Makefile.in (c++_OBJS): Remove dummy-checksum.o. (cc1plus-dummy): Remove. (checksum-options): Add. (cc1plus-checksum): Change to run checksum over object files and options only. * genchecksum.c: Update copyright. (usage): Allow multiple arguments. (BLOCKSIZE): Add. (dosum): Change for incremental checksum. Remove C output. (main): Iterate over all argument files. Add C output. --- gcc/Makefile.in | 18 +++++++----- gcc/cp/Make-lang.in | 16 +++++++---- gcc/dummy-checksum.c | 3 -- gcc/genchecksum.c | 73 +++++++++++++++++++++++++++++++++++++++---------- 4 files changed, 78 insertions(+), 32 deletions(-) delete mode 100644 gcc/dummy-checksum.c