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

[Thomas Schwinge <thomas at codesourcery dot com>]

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: signature.asc
Description: PGP signature