Patchwork Build compiler checksum from object files

login
register
mail settings
Submitter Andi Kleen
Date Oct. 7, 2010, 12:26 p.m.
Message ID <1286454385-3116-1-git-send-email-andi@firstfloor.org>
Download mbox | patch
Permalink /patch/67044/
State New
Headers show

Comments

Andi Kleen - Oct. 7, 2010, 12:26 p.m.
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
Jakub Jelinek - Oct. 7, 2010, 12:34 p.m.
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
Pedro Alves - Oct. 7, 2010, 12:58 p.m.
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.
Andi Kleen - Oct. 7, 2010, 1:05 p.m.
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
Pedro Alves - Oct. 7, 2010, 1:10 p.m.
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.
Mike Stump - Oct. 7, 2010, 3:09 p.m.
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.
>
Alexandre Oliva - Oct. 8, 2010, 5:41 a.m.
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.
Andi Kleen - Oct. 8, 2010, 6:15 a.m.
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

Patch

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;
 }