[LTO] PATCH: Fix WPA errors seen with make -j2

[LTO] PATCH: Fix WPA errors seen with make -j2

[Simon Baldwin]

LTO whole program analysis writes temporary intermediate files named
bogus0.lto.o, bogus1.lto.o, and so on, into the output directory.  If multiple
LTO links occur in that directory at the same time (parallel make, say),
intermediate files from one LTO link can overwrite those of the other while
it's still executing, causing it to fail in exciting and unpredictable ways.

This patch addresses the problem by embedding the process ID in the
intermediate file's name.  Comments?  Thanks.


gcc/lto/ChangeLog
2008-10-24  Simon Baldwin  <simonb@google.com>

	* lto.c (lto_wpa_write_files): Embed a process ID into the names
	of intermediate wpa files.


Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 141322)
+++ gcc/lto/lto.c	(working copy)
@@ -612,6 +612,7 @@ lto_wpa_write_files (void)
   cgraph_node_set set;
   bitmap decls;
   VEC(bitmap,heap) *inlined_decls = NULL;
+  const int pid = (int) getpid ();
 
   /* Include all inlined function. */
   for (i = 0; VEC_iterate (cgraph_node_set, lto_cgraph_node_sets, i, set); i++)
@@ -625,7 +626,7 @@ lto_wpa_write_files (void)
 
   for (i = 0; VEC_iterate (cgraph_node_set, lto_cgraph_node_sets, i, set); i++)
     {
-      size_t needed = 16;
+      size_t needed = 24;
       size_t len = needed;
       char * temp_filename = XNEWVEC (char, len);
 
@@ -636,7 +637,7 @@ lto_wpa_write_files (void)
 	      len = needed;
 	      temp_filename = XRESIZEVEC (char, temp_filename, len);
 	    }
-	  needed = snprintf (temp_filename, len, "bogus%d.lto.o", i);
+	  needed = snprintf (temp_filename, len, "bogus_%d_%d.lto.o", pid, i);
 	}
       while (needed >= len);
 

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Rafael Espindola]

2008/10/24 Simon Baldwin <simonb@google.com>:
> LTO whole program analysis writes temporary intermediate files named
> bogus0.lto.o, bogus1.lto.o, and so on, into the output directory.  If multiple
> LTO links occur in that directory at the same time (parallel make, say),
> intermediate files from one LTO link can overwrite those of the other while
> it's still executing, causing it to fail in exciting and unpredictable ways.
>
> This patch addresses the problem by embedding the process ID in the
> intermediate file's name.  Comments?  Thanks.

Is there anything that prevents us from using a temporary directory?
So one process uses temp-oexoe/* and another process uses
temp-oepxey/*.


Cheers,
-- 
Rafael Avila de Espindola

Google | Gordon House | Barrow Street | Dublin 4 | Ireland
Registered in Dublin, Ireland | Registration Number: 368047

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Diego Novillo]

On Fri, Oct 24, 2008 at 07:03, Simon Baldwin <simonb@google.com> wrote:
> LTO whole program analysis writes temporary intermediate files named
> bogus0.lto.o, bogus1.lto.o, and so on, into the output directory.  If multiple
> LTO links occur in that directory at the same time (parallel make, say),
> intermediate files from one LTO link can overwrite those of the other while
> it's still executing, causing it to fail in exciting and unpredictable ways.
>
> This patch addresses the problem by embedding the process ID in the
> intermediate file's name.  Comments?  Thanks.

I like Rafael's idea of creating temporary directories for LTRANS and
putting all the files in there.  So:

1- Create direcory ltans-<pid>
2- Create output files "<ltransdir>/<dump-base-name>-NN.lto.o"
3- If save_temps_flag is not set, remove all the files and the
directory when done.  Otherwise, keep them.


Diego.

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Rafael Espindola]

> 1- Create direcory ltans-<pid>

Why not use mkdtemp?

> Diego.
>

Cheers,
-- 
Rafael Avila de Espindola

Google | Gordon House | Barrow Street | Dublin 4 | Ireland
Registered in Dublin, Ireland | Registration Number: 368047

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Diego Novillo]

On Fri, Oct 24, 2008 at 09:20, Rafael Espindola <espindola@google.com> wrote:
>> 1- Create direcory ltans-<pid>
>
> Why not use mkdtemp?

Sure, much better.


Thanks.  Diego.

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Simon Baldwin]

Rafael Espindola wrote:
>> 1- Create direcory ltans-<pid>
>>     
>
> Why not use mkdtemp?
>   

There's also make_temp_file().  Before this version of the patch I 
investigated using that to just throw a bunch more cc*.mumble files into 
/tmp (or wherever else gcc is using for temporary files).  I settled on 
the simpler one, though, as using make_temp_files() was a bit more 
invasive overall (and may push an API change onto lto_elf_open_file).

Given that there's a preference for "proper" temporary files over bogus* 
files, then, any reason why we should bother with the extra step of 
creating a directory for all this stuff, rather than just creating a new 
and unique file for each with make_temp_file or similar?  There's no 
particular reason why they should all be called bogus*, since their 
names are transmitted in the ltrans.out file passed on the command 
line.  And making each just a plain and uniquely named file in /tmp will 
help performance where /tmp is a memfs.

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Rafael Espindola]

> There's also make_temp_file().  Before this version of the patch I
> investigated using that to just throw a bunch more cc*.mumble files into
> /tmp (or wherever else gcc is using for temporary files).  I settled on the
> simpler one, though, as using make_temp_files() was a bit more invasive
> overall (and may push an API change onto lto_elf_open_file).
>
> Given that there's a preference for "proper" temporary files over bogus*
> files, then, any reason why we should bother with the extra step of creating
> a directory for all this stuff, rather than just creating a new and unique
> file for each with make_temp_file or similar?  There's no particular reason
> why they should all be called bogus*, since their names are transmitted in
> the ltrans.out file passed on the command line.  And making each just a
> plain and uniquely named file in /tmp will help performance where /tmp is a
> memfs.

Having a directory makes it slightly easier to reproduce/reduce bugs,
but plain /tmp files is nice too.

Thanks,
-- 
Rafael Avila de Espindola

Google | Gordon House | Barrow Street | Dublin 4 | Ireland
Registered in Dublin, Ireland | Registration Number: 368047

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Diego Novillo]

On Fri, Oct 24, 2008 at 09:39, Simon Baldwin <simonb@google.com> wrote:

> There's also make_temp_file().  Before this version of the patch I
> investigated using that to just throw a bunch more cc*.mumble files into
> /tmp (or wherever else gcc is using for temporary files).  I settled on the
> simpler one, though, as using make_temp_files() was a bit more invasive
> overall (and may push an API change onto lto_elf_open_file).

Good point.  In that case, make_temp_file is fine.


Diego.

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Ollie Wild]

On Fri, Oct 24, 2008 at 6:39 AM, Simon Baldwin <simonb@google.com> wrote:
>
> There's also make_temp_file().  Before this version of the patch I
> investigated using that to just throw a bunch more cc*.mumble files into
> /tmp (or wherever else gcc is using for temporary files).  I settled on the
> simpler one, though, as using make_temp_files() was a bit more invasive
> overall (and may push an API change onto lto_elf_open_file).

I think you guys are all overlooking one important point.  These files
are going to exist in /tmp at the same time and potentially consume a
*very* large amount of space.  Many user's /tmp directories may have
size restrictions which make this unfeasible.

I would suggest creating these somewhere inside the build directory,
where we know the user is expecting to consume a lot of space.

Ollie

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Simon Baldwin]

Ollie Wild wrote:
> On Fri, Oct 24, 2008 at 6:39 AM, Simon Baldwin <simonb@google.com> wrote:
>   
>> There's also make_temp_file().  Before this version of the patch I
>> investigated using that to just throw a bunch more cc*.mumble files into
>> /tmp (or wherever else gcc is using for temporary files).  I settled on the
>> simpler one, though, as using make_temp_files() was a bit more invasive
>> overall (and may push an API change onto lto_elf_open_file).
>>     
>
> I think you guys are all overlooking one important point.  These files
> are going to exist in /tmp at the same time and potentially consume a
> *very* large amount of space.  Many user's /tmp directories may have
> size restrictions which make this unfeasible.
>
> I would suggest creating these somewhere inside the build directory,
> where we know the user is expecting to consume a lot of space.
>   

That adds a bit of extra awkwardness.

Libiberty's make_temp_file() only understands, for uniqueness purposes, 
temporary files residing in $TMPDIR and cousins.  Its underlying 
mkstemps() would do the job, but this is currently not exposed as part 
of the libiberty API, so at minimum there'd be a change to libiberty to 
go that route.

Libc's mktemp(3), mkstemp(3), tempnam(3), and tmpnam(3) are all marked 
do-not-use in their respective man pages.  tmpfile(3) is $TMPDIR-biased 
and returns a FILE*, but not the associated name.

It seems that the straightforward routes are to either add a new 
libiberty function to generate a unique (to the build directory) 
temporary filename without prefixing with $TMPDIR first, or go back to 
just adding the process ID to the filename as first suggested.  Any 
other ideas here?

Finally, there's currently no intermediate file deletion (which is of 
course less of a problem with bogus* filenames, as subsequent builds 
overwrite early ones so they don't accumulate!).  Deleting them is easy, 
but retaining them for debugging needs to be flagged somehow.  Passing 
either collect2's -debug flag or -save-temps through the ltrans driver, 
make, gcc, and back into lto1 for -fwpa, quickly becomes convoluted.  At 
the moment, I'm contemplating a simple check on an environment variable 
for whether to retain these files for debugging or delete them.  Seem 
reasonable?

Thanks.

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Ollie Wild]

On Mon, Oct 27, 2008 at 9:27 AM, Simon Baldwin <simonb@google.com> wrote:
>
> It seems that the straightforward routes are to either add a new libiberty
> function to generate a unique (to the build directory) temporary filename
> without prefixing with $TMPDIR first, or go back to just adding the process
> ID to the filename as first suggested.  Any other ideas here?

Here's another possibility.  Since the -fwpa (and implicitly -fltrans)
invocations of lto1 are driven by the linker driver and since it
already has code for passing environment variables, it probably
wouldn't be too much work to have collect2 (a) create a temporary
directory in the current build directory and (b) set TMPDIR to point
to the new directory.  That would allow us to use the libiberty code
as is.

Ollie

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Diego Novillo]

On Mon, Oct 27, 2008 at 15:42, Ollie Wild <aaw@google.com> wrote:

> Here's another possibility.  Since the -fwpa (and implicitly -fltrans)
> invocations of lto1 are driven by the linker driver and since it
> already has code for passing environment variables, it probably
> wouldn't be too much work to have collect2 (a) create a temporary
> directory in the current build directory and (b) set TMPDIR to point
> to the new directory.  That would allow us to use the libiberty code
> as is.

Sounds good to me.  Long term, I like the idea of

1- Create a unique directory in $CWD
2- Emit all the temporary object files there
3- Clean up only if -save-temps is not given

But I would do this as a separate patch.  For now the important thing
is to fix the overwriting of object files.

One could argue that setting TMPDIR explicitly in collect2 can be a
negative surprise from a usability POV, but for now I don't mind
either way.  I'm more concerned with fixing the bug first.


Diego.

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Simon Baldwin]

[-- Attachment #1: Type: text/plain, Size: 2022 bytes --]

Diego Novillo wrote:
> On Mon, Oct 27, 2008 at 15:42, Ollie Wild <aaw@google.com> wrote:
>
>   
>> Here's another possibility.  Since the -fwpa (and implicitly -fltrans)
>> invocations of lto1 are driven by the linker driver and since it
>> already has code for passing environment variables, it probably
>> wouldn't be too much work to have collect2 (a) create a temporary
>> directory in the current build directory and (b) set TMPDIR to point
>> to the new directory.  That would allow us to use the libiberty code
>> as is.
>>     
>
> Sounds good to me.  Long term, I like the idea of
>
> 1- Create a unique directory in $CWD
> 2- Emit all the temporary object files there
> 3- Clean up only if -save-temps is not given
>
> But I would do this as a separate patch.  For now the important thing
> is to fix the overwriting of object files.
>
> One could argue that setting TMPDIR explicitly in collect2 can be a
> negative surprise from a usability POV, but for now I don't mind
> either way.  I'm more concerned with fixing the bug first.


It turns out that the alterations to libiberty to create a 
make_cwd_temp_file() are trivial, and have the advantage of using 
already battle-tested code; this seems preferable to playing games with 
TMPDIR just to avoid changing libiberty.

Attached below, then, is an alternative slant on this patch that creates 
unique temporary files, named with the traditional gcc pattern, in place 
of bogus*.mumble in the build directory.  The patch also adds code to 
delete these files when no longer required, unless -debug is passed to 
collect2 (-Wl,-debug).  Using -debug this rather than -save-temps seems 
slightly more appropriate since at this point in a WPA build we're 
(conceptually, at least from the user perspective) linking rather than 
compiling.

This patch seems to be performing well in current testing.  Any thoughts?

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902


[-- Attachment #2: bogus_lto.patch --]
[-- Type: text/x-patch, Size: 6990 bytes --]

LTO whole program analysis writes temporary intermediate files named
bogus0.lto.o, bogus1.lto.o, and so on, into the output directory.  If multiple
LTO links occur in that directory at the same time (parallel make, say),
intermediate files from one LTO link can overwrite those of the other while
it's still executing, causing it to fail in exciting and unpredictable ways.

This patch addresses the problem by replacing bogus*.lto.o files with
uniquely named temporary files, located in the build directory.  It also adds
deletion of these intermediate files, suppressed with -debug to collect2.


gcc/lto/ChangeLog
2008-10-28  Simon Baldwin  <simonb@google.com>

	* lto.c (lto_wpa_write_files): Create intermediate files with
	make_cwd_temp_file().
	(lto_maybe_unlink): New.  Delete intermediate WPA files unless
	WPA_SAVE_LTRANS is set.
	(lto_main): Call lto_maybe_unlink() for intermediate WPA files.
	* ltrans-driver: Do not strip directory from output files.

include/ChangeLog
2008-10-28  Simon Baldwin  <simonb@google.com>

	* libiberty.h (make_cwd_temp_file): New declaration.

libiberty/ChangeLog
2008-10-28  Simon Baldwin  <simonb@google.com>

	* make-temp-file.c (make_temp_file_common): Split out from original
	make_temp_file() function.
	* (make_temp_file): Call make_temp_file_common() with tmp dir.
	* (make_cwd_temp_file): New.  Call make_temp_file_common() with "./".


Index: gcc/lto/lto.c
===================================================================
--- gcc/lto/lto.c	(revision 141381)
+++ gcc/lto/lto.c	(working copy)
@@ -26,6 +26,7 @@ Boston, MA 02110-1301, USA.  */
 #include "toplev.h"
 #include "tree.h"
 #include "tm.h"
+#include "libiberty.h"
 #include "cgraph.h"
 #include "ggc.h"
 #include "tree-ssa-operands.h"
@@ -625,20 +626,7 @@ lto_wpa_write_files (void)
 
   for (i = 0; VEC_iterate (cgraph_node_set, lto_cgraph_node_sets, i, set); i++)
     {
-      size_t needed = 16;
-      size_t len = needed;
-      char * temp_filename = XNEWVEC (char, len);
-
-      do 
-	{
-	  if (needed > len)
-	    {
-	      len = needed;
-	      temp_filename = XRESIZEVEC (char, temp_filename, len);
-	    }
-	  needed = snprintf (temp_filename, len, "bogus%d.lto.o", i);
-	}
-      while (needed >= len);
+      char * temp_filename = make_cwd_temp_file (".lto.o");
 
       output_files[i] = temp_filename;
 
@@ -969,6 +957,20 @@ lto_fixup_decls (struct lto_file_decl_da
   pointer_set_destroy (seen);
 }
 
+/* Unlink a temporary LTRANS file unless requested otherwise.  */
+
+static void
+lto_maybe_unlink (const char *file)
+{
+  if (!getenv ("WPA_SAVE_LTRANS"))
+    {
+      if (unlink_if_ordinary (file))
+        error ("deleting LTRANS file %s: %m", file);
+    }
+  else
+    fprintf (stderr, "[Leaving LTRANS %s]\n", file);
+}
+
 void
 lto_main (int debug_p ATTRIBUTE_UNUSED)
 {
@@ -1114,7 +1116,10 @@ lto_main (int debug_p ATTRIBUTE_UNUSED)
       lto_execute_ltrans (output_files);
 
       for (i = 0; output_files[i]; ++i)
-	XDELETEVEC (output_files[i]);
+        {
+	  lto_maybe_unlink (output_files[i]);
+	  free (output_files[i]);
+        }
       XDELETEVEC (output_files);
     }
 
Index: gcc/lto/ltrans-driver
===================================================================
--- gcc/lto/ltrans-driver	(revision 141381)
+++ gcc/lto/ltrans-driver	(working copy)
@@ -52,7 +52,7 @@ inputlist="$@"
 outputlist=
 for input in $inputlist
 do
-  output=`basename $input .o`.ltrans.o
+  output=`dirname $input`/`basename $input .o`.ltrans.o
   outputlist="$outputlist $output"
 
   echo "$output: $input" >> $makefile
Index: gcc/collect2.c
===================================================================
--- gcc/collect2.c	(revision 141381)
+++ gcc/collect2.c	(working copy)
@@ -933,6 +933,10 @@ maybe_run_lto_and_relink (char **lto_ld_
 	  strcpy (tmp, ltrans_output_file);
 
 	  *lto_c_ptr++ = "-fwpa";
+
+	  /* Save intermediate WPA files in lto1 if debug.  */
+	  if (debug)
+	    putenv ("WPA_SAVE_LTRANS=1");
 	}
       else
 	fatal ("invalid LTO mode");
Index: include/libiberty.h
===================================================================
--- include/libiberty.h	(revision 141381)
+++ include/libiberty.h	(working copy)
@@ -215,6 +215,7 @@ extern char *choose_temp_base (void) ATT
 /* Return a temporary file name or NULL if unable to create one.  */
 
 extern char *make_temp_file (const char *) ATTRIBUTE_MALLOC;
+extern char *make_cwd_temp_file (const char *) ATTRIBUTE_MALLOC;
 
 /* Remove a link to a file unless it is special. */
 
Index: libiberty/make-temp-file.c
===================================================================
--- libiberty/make-temp-file.c	(revision 141381)
+++ libiberty/make-temp-file.c	(working copy)
@@ -80,6 +80,7 @@ static const char usrtmp[] =
 { DIR_SEPARATOR, 'u', 's', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
 static const char vartmp[] =
 { DIR_SEPARATOR, 'v', 'a', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
+static const char cwd[] = { '.', DIR_SEPARATOR, 0 };
 
 static char *memoized_tmpdir;
 
@@ -133,22 +134,15 @@ choose_tmpdir (void)
   return tmpdir;
 }
 
-/*
-
-@deftypefn Replacement char* make_temp_file (const char *@var{suffix})
-
-Return a temporary file name (as a string) or @code{NULL} if unable to
-create one.  @var{suffix} is a suffix to append to the file name.  The
-string is @code{malloc}ed, and the temporary file has been created.
-
-@end deftypefn
-
-*/
+/* Common function to create temporary files.
+   Returns a temporary file name (as a string) or NULL if unable to
+   create one.  SUFFIX is a suffix to append to the file name, and BASE
+   is a prefix.  The return string is allocated, and the temporary file
+   created before returning.  */
 
 char *
-make_temp_file (const char *suffix)
+make_temp_file_common (const char *suffix, const char *base)
 {
-  const char *base = choose_tmpdir ();
   char *temp_filename;
   int base_len, suffix_len;
   int fd;
@@ -179,3 +173,41 @@ make_temp_file (const char *suffix)
     abort ();
   return temp_filename;
 }
+
+/*
+
+@deftypefn Replacement char* make_temp_file (const char *@var{suffix})
+
+Return a temporary file name (as a string) or @code{NULL} if unable to
+create one.  @var{suffix} is a suffix to append to the file name.  The
+string is @code{malloc}ed, and the temporary file has been created.  The
+temporary file is created in an appropriate temporary directory.
+
+@end deftypefn
+
+*/
+
+char *
+make_temp_file (const char *suffix)
+{
+  return make_temp_file_common (suffix, choose_tmpdir ());
+}
+
+/*
+
+@deftypefn Replacement char* make_cwd_temp_file (const char *@var{suffix})
+
+Return a temporary file name (as a string) or @code{NULL} if unable to
+create one.  @var{suffix} is a suffix to append to the file name.  The
+string is @code{malloc}ed, and the temporary file has been created.  The
+temporary file is created in the current working directory.
+
+@end deftypefn
+
+*/
+
+char *
+make_cwd_temp_file (const char *suffix)
+{
+  return make_temp_file_common (suffix, cwd);
+}

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Diego Novillo]

[ Adding DJ and Ian to the CC list for the libiberty changes in this patch. ]

On Tue, Oct 28, 2008 at 10:37, Simon Baldwin <simonb@google.com> wrote:
> Diego Novillo wrote:
>>
>> On Mon, Oct 27, 2008 at 15:42, Ollie Wild <aaw@google.com> wrote:
>>
>>
>>>
>>> Here's another possibility.  Since the -fwpa (and implicitly -fltrans)
>>> invocations of lto1 are driven by the linker driver and since it
>>> already has code for passing environment variables, it probably
>>> wouldn't be too much work to have collect2 (a) create a temporary
>>> directory in the current build directory and (b) set TMPDIR to point
>>> to the new directory.  That would allow us to use the libiberty code
>>> as is.
>>>
>>
>> Sounds good to me.  Long term, I like the idea of
>>
>> 1- Create a unique directory in $CWD
>> 2- Emit all the temporary object files there
>> 3- Clean up only if -save-temps is not given
>>
>> But I would do this as a separate patch.  For now the important thing
>> is to fix the overwriting of object files.
>>
>> One could argue that setting TMPDIR explicitly in collect2 can be a
>> negative surprise from a usability POV, but for now I don't mind
>> either way.  I'm more concerned with fixing the bug first.
>
>
> It turns out that the alterations to libiberty to create a
> make_cwd_temp_file() are trivial, and have the advantage of using already
> battle-tested code; this seems preferable to playing games with TMPDIR just
> to avoid changing libiberty.
>
> Attached below, then, is an alternative slant on this patch that creates
> unique temporary files, named with the traditional gcc pattern, in place of
> bogus*.mumble in the build directory.  The patch also adds code to delete
> these files when no longer required, unless -debug is passed to collect2
> (-Wl,-debug).  Using -debug this rather than -save-temps seems slightly more
> appropriate since at this point in a WPA build we're (conceptually, at least
> from the user perspective) linking rather than compiling.
>
> This patch seems to be performing well in current testing.  Any thoughts?


> LTO whole program analysis writes temporary intermediate files named
> bogus0.lto.o, bogus1.lto.o, and so on, into the output directory.  If multiple
> LTO links occur in that directory at the same time (parallel make, say),
> intermediate files from one LTO link can overwrite those of the other while
> it's still executing, causing it to fail in exciting and unpredictable ways.
>
> This patch addresses the problem by replacing bogus*.lto.o files with
> uniquely named temporary files, located in the build directory.  It also adds
> deletion of these intermediate files, suppressed with -debug to collect2.
>
>
> gcc/lto/ChangeLog
> 2008-10-28  Simon Baldwin  <simonb@google.com>
>
> 	* lto.c (lto_wpa_write_files): Create intermediate files with
> 	make_cwd_temp_file().
> 	(lto_maybe_unlink): New.  Delete intermediate WPA files unless
> 	WPA_SAVE_LTRANS is set.
> 	(lto_main): Call lto_maybe_unlink() for intermediate WPA files.
> 	* ltrans-driver: Do not strip directory from output files.
>
> include/ChangeLog
> 2008-10-28  Simon Baldwin  <simonb@google.com>
>
> 	* libiberty.h (make_cwd_temp_file): New declaration.
>
> libiberty/ChangeLog
> 2008-10-28  Simon Baldwin  <simonb@google.com>
>
> 	* make-temp-file.c (make_temp_file_common): Split out from original
> 	make_temp_file() function.
> 	* (make_temp_file): Call make_temp_file_common() with tmp dir.
> 	* (make_cwd_temp_file): New.  Call make_temp_file_common() with "./".
>
>
> Index: gcc/lto/lto.c
> ===================================================================
> --- gcc/lto/lto.c	(revision 141381)
> +++ gcc/lto/lto.c	(working copy)
> @@ -26,6 +26,7 @@ Boston, MA 02110-1301, USA.  */
>  #include "toplev.h"
>  #include "tree.h"
>  #include "tm.h"
> +#include "libiberty.h"
>  #include "cgraph.h"
>  #include "ggc.h"
>  #include "tree-ssa-operands.h"
> @@ -625,20 +626,7 @@ lto_wpa_write_files (void)
>
>    for (i = 0; VEC_iterate (cgraph_node_set, lto_cgraph_node_sets, i, set); i++)
>      {
> -      size_t needed = 16;
> -      size_t len = needed;
> -      char * temp_filename = XNEWVEC (char, len);
> -
> -      do
> -	{
> -	  if (needed > len)
> -	    {
> -	      len = needed;
> -	      temp_filename = XRESIZEVEC (char, temp_filename, len);
> -	    }
> -	  needed = snprintf (temp_filename, len, "bogus%d.lto.o", i);
> -	}
> -      while (needed >= len);
> +      char * temp_filename = make_cwd_temp_file (".lto.o");

No space after '*'.

> +    }
> +  else
> +    fprintf (stderr, "[Leaving LTRANS %s]\n", file);

No extraneous output, please.  This can generate too much noise.



> Index: include/libiberty.h
> ===================================================================
> --- include/libiberty.h	(revision 141381)
> +++ include/libiberty.h	(working copy)
> @@ -215,6 +215,7 @@ extern char *choose_temp_base (void) ATT
>  /* Return a temporary file name or NULL if unable to create one.  */
>
>  extern char *make_temp_file (const char *) ATTRIBUTE_MALLOC;
> +extern char *make_cwd_temp_file (const char *) ATTRIBUTE_MALLOC;
>
>  /* Remove a link to a file unless it is special. */
>
> Index: libiberty/make-temp-file.c
> ===================================================================
> --- libiberty/make-temp-file.c	(revision 141381)
> +++ libiberty/make-temp-file.c	(working copy)
> @@ -80,6 +80,7 @@ static const char usrtmp[] =
>  { DIR_SEPARATOR, 'u', 's', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
>  static const char vartmp[] =
>  { DIR_SEPARATOR, 'v', 'a', 'r', DIR_SEPARATOR, 't', 'm', 'p', 0 };
> +static const char cwd[] = { '.', DIR_SEPARATOR, 0 };
>
>  static char *memoized_tmpdir;
>
> @@ -133,22 +134,15 @@ choose_tmpdir (void)
>    return tmpdir;
>  }
>
> -/*
> -
> -@deftypefn Replacement char* make_temp_file (const char *@var{suffix})
> -
> -Return a temporary file name (as a string) or @code{NULL} if unable to
> -create one.  @var{suffix} is a suffix to append to the file name.  The
> -string is @code{malloc}ed, and the temporary file has been created.
> -
> -@end deftypefn
> -
> -*/
> +/* Common function to create temporary files.
> +   Returns a temporary file name (as a string) or NULL if unable to
> +   create one.  SUFFIX is a suffix to append to the file name, and BASE
> +   is a prefix.  The return string is allocated, and the temporary file
> +   created before returning.  */
>
>  char *
> -make_temp_file (const char *suffix)
> +make_temp_file_common (const char *suffix, const char *base)
>  {
> -  const char *base = choose_tmpdir ();
>    char *temp_filename;
>    int base_len, suffix_len;
>    int fd;
> @@ -179,3 +173,41 @@ make_temp_file (const char *suffix)
>      abort ();
>    return temp_filename;
>  }
> +
> +/*
> +
> +@deftypefn Replacement char* make_temp_file (const char *@var{suffix})
> +
> +Return a temporary file name (as a string) or @code{NULL} if unable to
> +create one.  @var{suffix} is a suffix to append to the file name.  The
> +string is @code{malloc}ed, and the temporary file has been created.  The
> +temporary file is created in an appropriate temporary directory.
> +
> +@end deftypefn
> +
> +*/
> +
> +char *
> +make_temp_file (const char *suffix)
> +{
> +  return make_temp_file_common (suffix, choose_tmpdir ());
> +}
> +
> +/*
> +
> +@deftypefn Replacement char* make_cwd_temp_file (const char *@var{suffix})
> +
> +Return a temporary file name (as a string) or @code{NULL} if unable to
> +create one.  @var{suffix} is a suffix to append to the file name.  The
> +string is @code{malloc}ed, and the temporary file has been created.  The
> +temporary file is created in the current working directory.
> +
> +@end deftypefn
> +
> +*/
> +
> +char *
> +make_cwd_temp_file (const char *suffix)
> +{
> +  return make_temp_file_common (suffix, cwd);
> +}

The libiberty bits look fine to me, but changes to libiberty
affect more than GCC.  Ian, DJ, could you review these changes?
Thanks.


Diego.

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Simon Baldwin]

Diego Novillo wrote:
> ...
>> +    }
>> +  else
>> +    fprintf (stderr, "[Leaving LTRANS %s]\n", file);
>>     
>
> No extraneous output, please.  This can generate too much noise.
>
>   

It's only emitted with -Wl,-debug, in line with other notices from 
collect2 when invoked for debugging, for example maybe_unlink() in 
collect2.c (it's just that we're not in collect2 at this point).

Still want it eliminated?

-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Diego Novillo]

On Tue, Oct 28, 2008 at 11:33, Simon Baldwin <simonb@google.com> wrote:

> It's only emitted with -Wl,-debug, in line with other notices from collect2
> when invoked for debugging, for example maybe_unlink() in collect2.c (it's
> just that we're not in collect2 at this point).

Ah, right.  OK then.


Diego.

Re: [LTO] PATCH: Fix WPA errors seen with make -j2

[Ian Lance Taylor]

"Diego Novillo" <dnovillo@google.com> writes:

> [ Adding DJ and Ian to the CC list for the libiberty changes in this patch. ]

The libiberty changes are OK except:

> +/*
> +
> +@deftypefn Replacement char* make_cwd_temp_file (const char *@var{suffix})
> +
> +Return a temporary file name (as a string) or @code{NULL} if unable to
> +create one.  @var{suffix} is a suffix to append to the file name.  The
> +string is @code{malloc}ed, and the temporary file has been created.  The
> +temporary file is created in the current working directory.
> +
> +@end deftypefn
> +
> +*/

Change "Replacement" to "Supplemental" in the first line of the
comment.

Thanks.

Ian