lto wrapper verboseness

lto wrapper verboseness

[Nathan Sidwell]

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

Jakub,
this patch augments the lto wrapper to print out the arguments to spawned 
commands when verbose.  I found this useful in debugging recent development.

ok for trunk?

nathan

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

2015-07-29  Nathan Sidwell  <nathan@codesourcery.com>

	* lto-wrapper.c (verbose_exec): New.
	(compile_offload_image, run_gcc): Call it.

Index: gcc/lto-wrapper.c
===================================================================
--- gcc/lto-wrapper.c	(revision 226372)
+++ gcc/lto-wrapper.c	(working copy)
@@ -115,6 +115,15 @@ maybe_unlink (const char *file)
     fprintf (stderr, "[Leaving LTRANS %s]\n", file);
 }
 
+/* Print command being executed.  */
+static void
+verbose_exec (const char *const *argv)
+{
+  while (*argv)
+    fprintf (stderr, " %s", *argv++);
+  fprintf (stderr, "\n");
+}
+
 /* Template of LTRANS dumpbase suffix.  */
 #define DUMPBASE_SUFFIX ".ltrans18446744073709551615"
 
@@ -693,6 +702,8 @@ compile_offload_image (const char *targe
 
       obstack_ptr_grow (&argv_obstack, NULL);
       argv = XOBFINISH (&argv_obstack, char **);
+      if (verbose)
+	verbose_exec (argv);
       fork_execute (argv[0], argv, true);
       obstack_free (&argv_obstack, NULL);
     }
@@ -1156,6 +1167,8 @@ run_gcc (unsigned argc, char *argv[])
 
   new_argv = XOBFINISH (&argv_obstack, const char **);
   argv_ptr = &new_argv[new_head_argc];
+  if (verbose)
+    verbose_exec (new_argv);
   fork_execute (new_argv[0], CONST_CAST (char **, new_argv), true);
 
   if (lto_mode == LTO_MODE_LTO)
@@ -1264,6 +1277,8 @@ cont:
 	    }
 	  else
 	    {
+	      if (verbose)
+		verbose_exec (new_argv);
 	      fork_execute (new_argv[0], CONST_CAST (char **, new_argv),
 			    true);
 	      maybe_unlink (input_name);

Re: lto wrapper verboseness

[Richard Biener]

On Thu, Jul 30, 2015 at 1:05 AM, Nathan Sidwell <nathan@acm.org> wrote:
> Jakub,
> this patch augments the lto wrapper to print out the arguments to spawned
> commands when verbose.  I found this useful in debugging recent development.
>
> ok for trunk?

Err - fork_execute through collect_execute already does this if
verbose || debug.

So better figure out why this doesn't work.

Richard.

> nathan

Re: lto wrapper verboseness

[Jakub Jelinek]

On Thu, Jul 30, 2015 at 10:09:05AM +0200, Richard Biener wrote:
> On Thu, Jul 30, 2015 at 1:05 AM, Nathan Sidwell <nathan@acm.org> wrote:
> > Jakub,
> > this patch augments the lto wrapper to print out the arguments to spawned
> > commands when verbose.  I found this useful in debugging recent development.
> >
> > ok for trunk?
> 
> Err - fork_execute through collect_execute already does this if
> verbose || debug.
> 
> So better figure out why this doesn't work.

All I can add is that it really doesn't work, so far to get at the arguments
passed to the offloading cc1/cc1plus/f951, I had to strace -s 1024 -v and
capture what has been passed to the compiler and the content of the
temporary file with options.

	Jakub

Re: lto wrapper verboseness

[Richard Biener]

On Thu, Jul 30, 2015 at 10:15 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Jul 30, 2015 at 10:09:05AM +0200, Richard Biener wrote:
>> On Thu, Jul 30, 2015 at 1:05 AM, Nathan Sidwell <nathan@acm.org> wrote:
>> > Jakub,
>> > this patch augments the lto wrapper to print out the arguments to spawned
>> > commands when verbose.  I found this useful in debugging recent development.
>> >
>> > ok for trunk?
>>
>> Err - fork_execute through collect_execute already does this if
>> verbose || debug.
>>
>> So better figure out why this doesn't work.
>
> All I can add is that it really doesn't work, so far to get at the arguments
> passed to the offloading cc1/cc1plus/f951, I had to strace -s 1024 -v and
> capture what has been passed to the compiler and the content of the
> temporary file with options.

Is this because collect_execute prints the variant with the response file?
Then better fix that.  Of course we only print the driver invocation - but
that's the same with the proposed patch.

Richard.

>         Jakub

Re: lto wrapper verboseness

[Nathan Sidwell]

On 07/30/15 04:28, Richard Biener wrote:
> On Thu, Jul 30, 2015 at 10:15 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, Jul 30, 2015 at 10:09:05AM +0200, Richard Biener wrote:
>>> On Thu, Jul 30, 2015 at 1:05 AM, Nathan Sidwell <nathan@acm.org> wrote:
>>>> Jakub,
>>>> this patch augments the lto wrapper to print out the arguments to spawned
>>>> commands when verbose.  I found this useful in debugging recent development.
>>>>
>>>> ok for trunk?
>>>
>>> Err - fork_execute through collect_execute already does this if
>>> verbose || debug.
>>>
>>> So better figure out why this doesn't work.
>>
>> All I can add is that it really doesn't work, so far to get at the arguments
>> passed to the offloading cc1/cc1plus/f951, I had to strace -s 1024 -v and
>> capture what has been passed to the compiler and the content of the
>> temporary file with options.
>
> Is this because collect_execute prints the variant with the response file?
> Then better fix that.  Of course we only print the driver invocation - but
> that's the same with the proposed patch.

It did surprise me that the usual machinery wasn't printing this stuff, but I 
didn't investigate why.


nathan

Re: lto wrapper verboseness

[Thomas Schwinge]

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

Hi!

On Thu, 30 Jul 2015 10:09:05 +0200, Richard Biener <richard.guenther@gmail.com> wrote:
> On Thu, Jul 30, 2015 at 1:05 AM, Nathan Sidwell <nathan@acm.org> wrote:
> > Jakub,
> > this patch augments the lto wrapper to print out the arguments to spawned
> > commands when verbose.  I found this useful in debugging recent development.
> >
> > ok for trunk?
> 
> Err - fork_execute through collect_execute already does this if
> verbose || debug.
> 
> So better figure out why this doesn't work.

We need to pass on the verbose flag.  I once came up with the following
patch (depends on "Refactor intelmic-mkoffload.c argv building",
<http://news.gmane.org/find-root.php?message_id=%3C87r3ljuez8.fsf%40kepler.schwinge.homeip.net%3E>);
OK for trunk?

commit ad0b6608cff22b62b73016c91c74b21a168acb46
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Tue Aug 4 14:08:23 2015 +0200

    Pass on the verbose flag "-v" to/in the mkoffloads
    
    	gcc/
    	* config/i386/intelmic-mkoffload.c (main): Parse "-v" flag.
    	(generate_target_descr_file, generate_target_offloadend_file)
    	(generate_host_descr_file, prepare_target_image, main): Pass it
    	on.
    	* config/nvptx/mkoffload.c (main): Parse "-v" flag.
    	(compile_native, main): Pass it on.
    	* lto-wrapper.c (compile_offload_image): Likewise.
---
 gcc/config/i386/intelmic-mkoffload.c |   24 +++++++++++++++++++++---
 gcc/config/nvptx/mkoffload.c         |    6 ++++++
 gcc/lto-wrapper.c                    |    2 ++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mkoffload.c
index 8d5af0d..ae1bde0 100644
--- gcc/config/i386/intelmic-mkoffload.c
+++ gcc/config/i386/intelmic-mkoffload.c
@@ -281,6 +281,8 @@ generate_target_descr_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
@@ -319,6 +321,8 @@ generate_target_offloadend_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
@@ -380,9 +384,11 @@ generate_host_descr_file (const char *host_compiler)
   fclose (src_file);
 
   unsigned new_argc = 0;
-#define NEW_ARGC_MAX 9
+#define NEW_ARGC_MAX 10
   const char *new_argv[NEW_ARGC_MAX];
   new_argv[new_argc++] = host_compiler;
+  if (verbose)
+    new_argv[new_argc++] = "-v";
   new_argv[new_argc++] = "-c";
   new_argv[new_argc++] = "-fPIC";
   new_argv[new_argc++] = "-shared";
@@ -429,6 +435,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
@@ -448,7 +456,7 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   compile_for_target (&argv_obstack);
 
   unsigned objcopy_argc;
-#define OBJCOPY_ARGC_MAX 11
+#define OBJCOPY_ARGC_MAX 12
   const char *objcopy_argv[OBJCOPY_ARGC_MAX];
 
   /* Run objcopy.  */
@@ -457,6 +465,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   sprintf (rename_section_opt, ".data=%s", image_section_name);
   objcopy_argc = 0;
   objcopy_argv[objcopy_argc++] = "objcopy";
+  if (verbose)
+    objcopy_argv[objcopy_argc++] = "-v";
   objcopy_argv[objcopy_argc++] = "-B";
   objcopy_argv[objcopy_argc++] = "i386";
   objcopy_argv[objcopy_argc++] = "-I";
@@ -510,6 +520,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
 
   objcopy_argc = 0;
   objcopy_argv[objcopy_argc++] = "objcopy";
+  if (verbose)
+    objcopy_argv[objcopy_argc++] = "-v";
   objcopy_argv[objcopy_argc++] = target_so_filename;
   objcopy_argv[objcopy_argc++] = "--redefine-sym";
   objcopy_argv[objcopy_argc++] = opt_for_objcopy[0];
@@ -565,6 +577,8 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (strcmp (argv[i], "-v") == 0)
+	verbose = true;
     }
 
   const char *target_so_filename
@@ -573,13 +587,15 @@ main (int argc, char **argv)
   const char *host_descr_filename = generate_host_descr_file (host_compiler);
 
   unsigned new_argc;
-#define NEW_ARGC_MAX 9
+#define NEW_ARGC_MAX 10
   const char *new_argv[NEW_ARGC_MAX];
 
   /* Perform partial linking for the target image and host side descriptor.
      As a result we'll get a finalized object file with all offload data.  */
   new_argc = 0;
   new_argv[new_argc++] = "ld";
+  if (verbose)
+    new_argv[new_argc++] = "-v";
   new_argv[new_argc++] = "-m";
   switch (offload_abi)
     {
@@ -605,6 +621,8 @@ main (int argc, char **argv)
      to avoid conflicting between different DSO and an executable.  */
   new_argc = 0;
   new_argv[new_argc++] = "objcopy";
+  if (verbose)
+    new_argv[new_argc++] = "-v";
   new_argv[new_argc++] = "-L";
   new_argv[new_argc++] = symbols[0];
   new_argv[new_argc++] = "-L";
diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
index 78ac8fe..eed099f 100644
--- gcc/config/nvptx/mkoffload.c
+++ gcc/config/nvptx/mkoffload.c
@@ -919,6 +919,8 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   switch (offload_abi)
     {
     case OFFLOAD_ABI_LP64:
@@ -1022,11 +1024,15 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (strcmp (argv[i], "-v") == 0)
+	verbose = true;
     }
 
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, driver);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
   switch (offload_abi)
     {
diff --git gcc/lto-wrapper.c gcc/lto-wrapper.c
index e13a82a..f932758 100644
--- gcc/lto-wrapper.c
+++ gcc/lto-wrapper.c
@@ -685,6 +685,8 @@ compile_offload_image (const char *target, const char *compiler_path,
       struct obstack argv_obstack;
       obstack_init (&argv_obstack);
       obstack_ptr_grow (&argv_obstack, compiler);
+      if (verbose)
+	obstack_ptr_grow (&argv_obstack, "-v");
       obstack_ptr_grow (&argv_obstack, "-o");
       obstack_ptr_grow (&argv_obstack, filename);
 


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: lto wrapper verboseness

[Bernd Schmidt]

>
> We need to pass on the verbose flag.  I once came up with the following
> patch (depends on "Refactor intelmic-mkoffload.c argv building",
> <http://news.gmane.org/find-root.php?message_id=%3C87r3ljuez8.fsf%40kepler.schwinge.homeip.net%3E>);
> OK for trunk?
>
>     /* Run objcopy.  */
> @@ -457,6 +465,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
>     sprintf (rename_section_opt, ".data=%s", image_section_name);
>     objcopy_argc = 0;
>     objcopy_argv[objcopy_argc++] = "objcopy";
> +  if (verbose)
> +    objcopy_argv[objcopy_argc++] = "-v";
>     objcopy_argv[objcopy_argc++] = "-B";
>     objcopy_argv[objcopy_argc++] = "i386";
>     objcopy_argv[objcopy_argc++] = "-I";

I'm not convinced we gain much by passing "-v" to objcopy, but I'll 
leave that for the Intel folks to decide.

Other than that, ok if all argv arrays are constructed using obstacks.


Bernd

Re: lto wrapper verboseness

[Thomas Schwinge]

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

Hi!

On Mon, 28 Sep 2015 12:23:37 +0200, Bernd Schmidt <bschmidt@redhat.com> wrote:
> >
> > We need to pass on the verbose flag.  I once came up with the following
> > patch (depends on "Refactor intelmic-mkoffload.c argv building",
> > <http://news.gmane.org/find-root.php?message_id=%3C87r3ljuez8.fsf%40kepler.schwinge.homeip.net%3E>);
> > OK for trunk?
> >
> >     /* Run objcopy.  */
> > @@ -457,6 +465,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
> >     sprintf (rename_section_opt, ".data=%s", image_section_name);
> >     objcopy_argc = 0;
> >     objcopy_argv[objcopy_argc++] = "objcopy";
> > +  if (verbose)
> > +    objcopy_argv[objcopy_argc++] = "-v";
> >     objcopy_argv[objcopy_argc++] = "-B";
> >     objcopy_argv[objcopy_argc++] = "i386";
> >     objcopy_argv[objcopy_argc++] = "-I";
> 
> I'm not convinced we gain much by passing "-v" to objcopy, but I'll 
> leave that for the Intel folks to decide.

Actually it may even be harmful if objcopy (and likewise, ld) write to
stdout, and that disturbs the lto-wrapper usage of communication by
listing object files on stdout (as I understand it)...  So, I dropped
that.

> Other than that, ok if all argv arrays are constructed using obstacks.

Thanks for the review.  Committed in r228301:

commit 74d66c731d39a1464a34ec790432d068215a246b
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Wed Sep 30 16:42:32 2015 +0000

    Pass on the verbose flag "-v" to/in the mkoffloads
    
    	gcc/
    	* config/i386/intelmic-mkoffload.c (main): Parse "-v" flag.
    	(generate_target_descr_file, generate_target_offloadend_file)
    	(generate_host_descr_file, prepare_target_image, main): Pass it
    	on.
    	* config/nvptx/mkoffload.c (main): Parse "-v" flag.
    	(compile_native, main): Pass it on.
    	* lto-wrapper.c (compile_offload_image): Likewise.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@228301 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog                        | 10 ++++++++++
 gcc/config/i386/intelmic-mkoffload.c | 10 ++++++++++
 gcc/config/nvptx/mkoffload.c         |  6 ++++++
 gcc/lto-wrapper.c                    |  2 ++
 4 files changed, 28 insertions(+)

diff --git gcc/ChangeLog gcc/ChangeLog
index 15f84ab..ba82953 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,4 +1,14 @@
 2015-09-30  Thomas Schwinge  <thomas@codesourcery.com>
+
+	* config/i386/intelmic-mkoffload.c (main): Parse "-v" flag.
+	(generate_target_descr_file, generate_target_offloadend_file)
+	(generate_host_descr_file, prepare_target_image, main): Pass it
+	on.
+	* config/nvptx/mkoffload.c (main): Parse "-v" flag.
+	(compile_native, main): Pass it on.
+	* lto-wrapper.c (compile_offload_image): Likewise.
+
+2015-09-30  Thomas Schwinge  <thomas@codesourcery.com>
 	    Ilya Verbin  <ilya.verbin@intel.com>
 	    Andrey Turetskiy  <andrey.turetskiy@intel.com>
 
diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mkoffload.c
index ae88ecd..14f3fb3 100644
--- gcc/config/i386/intelmic-mkoffload.c
+++ gcc/config/i386/intelmic-mkoffload.c
@@ -281,6 +281,8 @@ generate_target_descr_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
@@ -319,6 +321,8 @@ generate_target_offloadend_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
@@ -382,6 +386,8 @@ generate_host_descr_file (const char *host_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, host_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
   obstack_ptr_grow (&argv_obstack, "-shared");
@@ -428,6 +434,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
   obstack_ptr_grow (&argv_obstack, "-shared");
   obstack_ptr_grow (&argv_obstack, "-fPIC");
@@ -560,6 +568,8 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (strcmp (argv[i], "-v") == 0)
+	verbose = true;
     }
 
   const char *target_so_filename
diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
index fe0e8cd..99c0254 100644
--- gcc/config/nvptx/mkoffload.c
+++ gcc/config/nvptx/mkoffload.c
@@ -925,6 +925,8 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, compiler);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   switch (offload_abi)
     {
     case OFFLOAD_ABI_LP64:
@@ -1028,11 +1030,15 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (strcmp (argv[i], "-v") == 0)
+	verbose = true;
     }
 
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, driver);
+  if (verbose)
+    obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
   switch (offload_abi)
     {
diff --git gcc/lto-wrapper.c gcc/lto-wrapper.c
index e13a82a..f932758 100644
--- gcc/lto-wrapper.c
+++ gcc/lto-wrapper.c
@@ -685,6 +685,8 @@ compile_offload_image (const char *target, const char *compiler_path,
       struct obstack argv_obstack;
       obstack_init (&argv_obstack);
       obstack_ptr_grow (&argv_obstack, compiler);
+      if (verbose)
+	obstack_ptr_grow (&argv_obstack, "-v");
       obstack_ptr_grow (&argv_obstack, "-o");
       obstack_ptr_grow (&argv_obstack, filename);
 


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

[PR other/65021] mkoffloads -save-temps handling, and cleanup cleanup (was: lto wrapper verboseness)

[Thomas Schwinge]

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

Hi!

In a similar vein to the earlier patch to "Pass on the verbose flag "-v"
to/in the mkoffloads", here is a patch to make the mkoffloads handle
"-save-temps", and this patch also happens to address
<https://gcc.gnu.org/PR65021>, "nvptx mkoffload doesn't clean up its
temporary files".  OK for trunk?

commit 693388578c8b71ed0dd7ee07a4153442e92e17de
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Mon Oct 5 11:36:51 2015 +0200

    [PR other/65021] mkoffloads -save-temps handling, and cleanup cleanup
    
    	gcc/
    	PR other/65021
    	* config/i386/intelmic-mkoffload.c (mkoffload_atexit): Rename
    	function to...
    	(mkoffload_cleanup): ... this.  Adjust all users.
    	(maybe_unlink): Look at save_temps and verbose flags instead of
    	debug flag.
    	(main): Parse "-save-temps" flag.
    	(generate_target_descr_file, generate_target_offloadend_file)
    	(generate_host_descr_file, prepare_target_image): Pass it on.
    	* config/nvptx/mkoffload.c (tool_cleanup): Implement.
    	(mkoffload_cleanup): New function.
    	(maybe_unlink): Look at save_temps and verbose flags instead of
    	debug flag.
    	(main): Instead of calling utils_cleanup, register atexit handler
    	for mkoffload_cleanup.
    	(main): Parse "-save-temps" flag.
    	(compile_native, main): Pass it on.
    	* lto-wrapper.c (compile_offload_image): Likewise.
---
 gcc/ChangeLog                        |  2 +-
 gcc/config/i386/intelmic-mkoffload.c | 30 ++++++++++++++++++++++-------
 gcc/config/nvptx/mkoffload.c         | 37 ++++++++++++++++++++++++++----------
 gcc/lto-wrapper.c                    |  2 ++
 4 files changed, 53 insertions(+), 18 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index 0d740a2..708ac08 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -464,7 +464,7 @@
 
 	* config/i386/intelmic-mkoffload.c (main): Parse "-v" flag.
 	(generate_target_descr_file, generate_target_offloadend_file)
-	(generate_host_descr_file, prepare_target_image, main): Pass it on.
+	(generate_host_descr_file, prepare_target_image): Pass it on.
 	* config/nvptx/mkoffload.c (main): Parse "-v" flag.
 	(compile_native, main): Pass it on.
 	* lto-wrapper.c (compile_offload_image): Likewise.
diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mkoffload.c
index 14f3fb3..828b415 100644
--- gcc/config/i386/intelmic-mkoffload.c
+++ gcc/config/i386/intelmic-mkoffload.c
@@ -45,6 +45,7 @@ const char *temp_files[MAX_NUM_TEMPS];
 enum offload_abi offload_abi = OFFLOAD_ABI_UNSET;
 
 /* Delete tempfiles and exit function.  */
+
 void
 tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
 {
@@ -53,19 +54,24 @@ tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
 }
 
 static void
-mkoffload_atexit (void)
+mkoffload_cleanup (void)
 {
   tool_cleanup (false);
 }
 
-/* Unlink FILE unless we are debugging.  */
+/* Unlink FILE unless requested otherwise.  */
+
 void
 maybe_unlink (const char *file)
 {
-  if (debug)
-    notice ("[Leaving %s]\n", file);
-  else
-    unlink_if_ordinary (file);
+  if (!save_temps)
+    {
+      if (unlink_if_ordinary (file)
+	  && errno != ENOENT)
+	fatal_error (input_location, "deleting file %s: %m", file);
+    }
+  else if (verbose)
+    fprintf (stderr, "[Leaving %s]\n", file);
 }
 
 /* Add or change the value of an environment variable, outputting the
@@ -281,6 +287,8 @@ generate_target_descr_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
@@ -321,6 +329,8 @@ generate_target_offloadend_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
@@ -386,6 +396,8 @@ generate_host_descr_file (const char *host_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, host_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
@@ -434,6 +446,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
@@ -536,7 +550,7 @@ main (int argc, char **argv)
   gcc_init_libintl ();
   diagnostic_initialize (global_dc, 0);
 
-  if (atexit (mkoffload_atexit) != 0)
+  if (atexit (mkoffload_cleanup) != 0)
     fatal_error (input_location, "atexit failed");
 
   const char *host_compiler = getenv ("COLLECT_GCC");
@@ -568,6 +582,8 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (strcmp (argv[i], "-save-temps") == 0)
+	save_temps = true;
       else if (strcmp (argv[i], "-v") == 0)
 	verbose = true;
     }
diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
index ff538e2..ba5a138 100644
--- gcc/config/nvptx/mkoffload.c
+++ gcc/config/nvptx/mkoffload.c
@@ -58,26 +58,36 @@ enum offload_abi offload_abi = OFFLOAD_ABI_UNSET;
 
 /* Delete tempfiles.  */
 
-/* Unlink a temporary file unless requested otherwise.  */
+void
+tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
+{
+  if (ptx_cfile_name)
+    maybe_unlink (ptx_cfile_name);
+  if (ptx_name)
+    maybe_unlink (ptx_name);
+}
+
+static void
+mkoffload_cleanup (void)
+{
+  tool_cleanup (false);
+}
+
+/* Unlink FILE unless requested otherwise.  */
 
 void
 maybe_unlink (const char *file)
 {
-  if (! debug)
+  if (!save_temps)
     {
       if (unlink_if_ordinary (file)
 	  && errno != ENOENT)
 	fatal_error (input_location, "deleting file %s: %m", file);
     }
-  else
+  else if (verbose)
     fprintf (stderr, "[Leaving %s]\n", file);
 }
 
-void
-tool_cleanup (bool)
-{
-}
-
 /* Add or change the value of an environment variable, outputting the
    change to standard error if in verbose mode.  */
 static void
@@ -353,6 +363,8 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   switch (offload_abi)
@@ -387,6 +399,9 @@ main (int argc, char **argv)
   progname = "mkoffload";
   diagnostic_initialize (global_dc, 0);
 
+  if (atexit (mkoffload_cleanup) != 0)
+    fatal_error (input_location, "atexit failed");
+
   char *collect_gcc = getenv ("COLLECT_GCC");
   if (collect_gcc == NULL)
     fatal_error (input_location, "COLLECT_GCC must be set.");
@@ -461,6 +476,8 @@ main (int argc, char **argv)
 #undef STR
       else if (strcmp (argv[i], "-fopenmp") == 0)
 	fopenmp = true;
+      else if (strcmp (argv[i], "-save-temps") == 0)
+	save_temps = true;
       else if (strcmp (argv[i], "-v") == 0)
 	verbose = true;
     }
@@ -468,6 +485,8 @@ main (int argc, char **argv)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, driver);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
@@ -532,7 +551,5 @@ main (int argc, char **argv)
 
   compile_native (ptx_cfile_name, outname, collect_gcc);
 
-  utils_cleanup (false);
-
   return 0;
 }
diff --git gcc/lto-wrapper.c gcc/lto-wrapper.c
index f932758..20e67ed 100644
--- gcc/lto-wrapper.c
+++ gcc/lto-wrapper.c
@@ -685,6 +685,8 @@ compile_offload_image (const char *target, const char *compiler_path,
       struct obstack argv_obstack;
       obstack_init (&argv_obstack);
       obstack_ptr_grow (&argv_obstack, compiler);
+      if (save_temps)
+	obstack_ptr_grow (&argv_obstack, "-save-temps");
       if (verbose)
 	obstack_ptr_grow (&argv_obstack, "-v");
       obstack_ptr_grow (&argv_obstack, "-o");


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PR other/65021] mkoffloads -save-temps handling, and cleanup cleanup

[Bernd Schmidt]

On 10/05/2015 12:04 PM, Thomas Schwinge wrote:
> In a similar vein to the earlier patch to "Pass on the verbose flag "-v"
> to/in the mkoffloads", here is a patch to make the mkoffloads handle
> "-save-temps", and this patch also happens to address
> <https://gcc.gnu.org/PR65021>, "nvptx mkoffload doesn't clean up its
> temporary files".  OK for trunk?

The patch is ok.

>   static void
> -mkoffload_atexit (void)
> +mkoffload_cleanup (void)
>   {
>     tool_cleanup (false);
>   }

Don't quite see the need for this change, but don't feel strongly enough 
about it to make you resubmit.


Bernd

Re: [PR other/65021] mkoffloads -save-temps handling, and cleanup cleanup

[Thomas Schwinge]

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

Hi Bernd!

On Mon, 5 Oct 2015 14:12:06 +0200, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 10/05/2015 12:04 PM, Thomas Schwinge wrote:
> > In a similar vein to the earlier patch to "Pass on the verbose flag "-v"
> > to/in the mkoffloads", here is a patch to make the mkoffloads handle
> > "-save-temps", and this patch also happens to address
> > <https://gcc.gnu.org/PR65021>, "nvptx mkoffload doesn't clean up its
> > temporary files".  OK for trunk?
> 
> The patch is ok.

Thanks for the prompt review!

> >   static void
> > -mkoffload_atexit (void)
> > +mkoffload_cleanup (void)
> >   {
> >     tool_cleanup (false);
> >   }
> 
> Don't quite see the need for this change, but don't feel strongly enough 
> about it to make you resubmit.

It's for uniformity, to make it easy for the reader: that's what the
other users of gcc/collect-utils.c are doing.  Oh, actually only
gcc/lto-wrapper.c; but gcc/collect2.c doesn't...

Maybe some more refactoring could be done here, possibly also to remove
duplicated code amongst users of gcc/collect-utils.c as well as in the
mkoffloads.

Anyway, I committed my patch without modifications to trunk in r228488.

commit 558e6810f0a18b67eb8474bd86db23ab7de4f2fe
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Mon Oct 5 14:07:50 2015 +0000

    [PR other/65021] mkoffloads -save-temps handling, and cleanup cleanup
    
    	gcc/
    	PR other/65021
    	* config/i386/intelmic-mkoffload.c (mkoffload_atexit): Rename
    	function to...
    	(mkoffload_cleanup): ... this.  Adjust all users.
    	(maybe_unlink): Look at save_temps and verbose flags instead of
    	debug flag.
    	(main): Parse "-save-temps" flag.
    	(generate_target_descr_file, generate_target_offloadend_file)
    	(generate_host_descr_file, prepare_target_image): Pass it on.
    	* config/nvptx/mkoffload.c (tool_cleanup): Implement.
    	(mkoffload_cleanup): New function.
    	(maybe_unlink): Look at save_temps and verbose flags instead of
    	debug flag.
    	(main): Instead of calling utils_cleanup, register atexit handler
    	for mkoffload_cleanup.
    	(main): Parse "-save-temps" flag.
    	(compile_native, main): Pass it on.
    	* lto-wrapper.c (compile_offload_image): Likewise.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@228488 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog                        | 23 +++++++++++++++++++++-
 gcc/config/i386/intelmic-mkoffload.c | 30 ++++++++++++++++++++++-------
 gcc/config/nvptx/mkoffload.c         | 37 ++++++++++++++++++++++++++----------
 gcc/lto-wrapper.c                    |  2 ++
 4 files changed, 74 insertions(+), 18 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index e665b6b..5340f47 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,24 @@
+2015-10-05  Thomas Schwinge  <thomas@codesourcery.com>
+
+	PR other/65021
+	* config/i386/intelmic-mkoffload.c (mkoffload_atexit): Rename
+	function to...
+	(mkoffload_cleanup): ... this.  Adjust all users.
+	(maybe_unlink): Look at save_temps and verbose flags instead of
+	debug flag.
+	(main): Parse "-save-temps" flag.
+	(generate_target_descr_file, generate_target_offloadend_file)
+	(generate_host_descr_file, prepare_target_image): Pass it on.
+	* config/nvptx/mkoffload.c (tool_cleanup): Implement.
+	(mkoffload_cleanup): New function.
+	(maybe_unlink): Look at save_temps and verbose flags instead of
+	debug flag.
+	(main): Instead of calling utils_cleanup, register atexit handler
+	for mkoffload_cleanup.
+	(main): Parse "-save-temps" flag.
+	(compile_native, main): Pass it on.
+	* lto-wrapper.c (compile_offload_image): Likewise.
+
 2015-10-05  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
 
 	* gimple.h (gimple_op_ptr): Require a non const gimple *.
@@ -763,7 +784,7 @@
 
 	* config/i386/intelmic-mkoffload.c (main): Parse "-v" flag.
 	(generate_target_descr_file, generate_target_offloadend_file)
-	(generate_host_descr_file, prepare_target_image, main): Pass it on.
+	(generate_host_descr_file, prepare_target_image): Pass it on.
 	* config/nvptx/mkoffload.c (main): Parse "-v" flag.
 	(compile_native, main): Pass it on.
 	* lto-wrapper.c (compile_offload_image): Likewise.
diff --git gcc/config/i386/intelmic-mkoffload.c gcc/config/i386/intelmic-mkoffload.c
index 14f3fb3..828b415 100644
--- gcc/config/i386/intelmic-mkoffload.c
+++ gcc/config/i386/intelmic-mkoffload.c
@@ -45,6 +45,7 @@ const char *temp_files[MAX_NUM_TEMPS];
 enum offload_abi offload_abi = OFFLOAD_ABI_UNSET;
 
 /* Delete tempfiles and exit function.  */
+
 void
 tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
 {
@@ -53,19 +54,24 @@ tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
 }
 
 static void
-mkoffload_atexit (void)
+mkoffload_cleanup (void)
 {
   tool_cleanup (false);
 }
 
-/* Unlink FILE unless we are debugging.  */
+/* Unlink FILE unless requested otherwise.  */
+
 void
 maybe_unlink (const char *file)
 {
-  if (debug)
-    notice ("[Leaving %s]\n", file);
-  else
-    unlink_if_ordinary (file);
+  if (!save_temps)
+    {
+      if (unlink_if_ordinary (file)
+	  && errno != ENOENT)
+	fatal_error (input_location, "deleting file %s: %m", file);
+    }
+  else if (verbose)
+    fprintf (stderr, "[Leaving %s]\n", file);
 }
 
 /* Add or change the value of an environment variable, outputting the
@@ -281,6 +287,8 @@ generate_target_descr_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
@@ -321,6 +329,8 @@ generate_target_offloadend_file (const char *target_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
@@ -386,6 +396,8 @@ generate_host_descr_file (const char *host_compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, host_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-c");
@@ -434,6 +446,8 @@ prepare_target_image (const char *target_compiler, int argc, char **argv)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, target_compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
@@ -536,7 +550,7 @@ main (int argc, char **argv)
   gcc_init_libintl ();
   diagnostic_initialize (global_dc, 0);
 
-  if (atexit (mkoffload_atexit) != 0)
+  if (atexit (mkoffload_cleanup) != 0)
     fatal_error (input_location, "atexit failed");
 
   const char *host_compiler = getenv ("COLLECT_GCC");
@@ -568,6 +582,8 @@ main (int argc, char **argv)
 			 "unrecognizable argument of option " STR);
 	}
 #undef STR
+      else if (strcmp (argv[i], "-save-temps") == 0)
+	save_temps = true;
       else if (strcmp (argv[i], "-v") == 0)
 	verbose = true;
     }
diff --git gcc/config/nvptx/mkoffload.c gcc/config/nvptx/mkoffload.c
index ff538e2..ba5a138 100644
--- gcc/config/nvptx/mkoffload.c
+++ gcc/config/nvptx/mkoffload.c
@@ -58,26 +58,36 @@ enum offload_abi offload_abi = OFFLOAD_ABI_UNSET;
 
 /* Delete tempfiles.  */
 
-/* Unlink a temporary file unless requested otherwise.  */
+void
+tool_cleanup (bool from_signal ATTRIBUTE_UNUSED)
+{
+  if (ptx_cfile_name)
+    maybe_unlink (ptx_cfile_name);
+  if (ptx_name)
+    maybe_unlink (ptx_name);
+}
+
+static void
+mkoffload_cleanup (void)
+{
+  tool_cleanup (false);
+}
+
+/* Unlink FILE unless requested otherwise.  */
 
 void
 maybe_unlink (const char *file)
 {
-  if (! debug)
+  if (!save_temps)
     {
       if (unlink_if_ordinary (file)
 	  && errno != ENOENT)
 	fatal_error (input_location, "deleting file %s: %m", file);
     }
-  else
+  else if (verbose)
     fprintf (stderr, "[Leaving %s]\n", file);
 }
 
-void
-tool_cleanup (bool)
-{
-}
-
 /* Add or change the value of an environment variable, outputting the
    change to standard error if in verbose mode.  */
 static void
@@ -353,6 +363,8 @@ compile_native (const char *infile, const char *outfile, const char *compiler)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, compiler);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   switch (offload_abi)
@@ -387,6 +399,9 @@ main (int argc, char **argv)
   progname = "mkoffload";
   diagnostic_initialize (global_dc, 0);
 
+  if (atexit (mkoffload_cleanup) != 0)
+    fatal_error (input_location, "atexit failed");
+
   char *collect_gcc = getenv ("COLLECT_GCC");
   if (collect_gcc == NULL)
     fatal_error (input_location, "COLLECT_GCC must be set.");
@@ -461,6 +476,8 @@ main (int argc, char **argv)
 #undef STR
       else if (strcmp (argv[i], "-fopenmp") == 0)
 	fopenmp = true;
+      else if (strcmp (argv[i], "-save-temps") == 0)
+	save_temps = true;
       else if (strcmp (argv[i], "-v") == 0)
 	verbose = true;
     }
@@ -468,6 +485,8 @@ main (int argc, char **argv)
   struct obstack argv_obstack;
   obstack_init (&argv_obstack);
   obstack_ptr_grow (&argv_obstack, driver);
+  if (save_temps)
+    obstack_ptr_grow (&argv_obstack, "-save-temps");
   if (verbose)
     obstack_ptr_grow (&argv_obstack, "-v");
   obstack_ptr_grow (&argv_obstack, "-xlto");
@@ -532,7 +551,5 @@ main (int argc, char **argv)
 
   compile_native (ptx_cfile_name, outname, collect_gcc);
 
-  utils_cleanup (false);
-
   return 0;
 }
diff --git gcc/lto-wrapper.c gcc/lto-wrapper.c
index f932758..20e67ed 100644
--- gcc/lto-wrapper.c
+++ gcc/lto-wrapper.c
@@ -685,6 +685,8 @@ compile_offload_image (const char *target, const char *compiler_path,
       struct obstack argv_obstack;
       obstack_init (&argv_obstack);
       obstack_ptr_grow (&argv_obstack, compiler);
+      if (save_temps)
+	obstack_ptr_grow (&argv_obstack, "-save-temps");
       if (verbose)
 	obstack_ptr_grow (&argv_obstack, "-v");
       obstack_ptr_grow (&argv_obstack, "-o");


Grüße,
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]