This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[incremental] Patch: FYI: job-handling cleanup


I'm checking this in on the incremental-compiler branch.

This combines a few cleanups to the compilation-job handling code.

First, it changes server.c to use the argv code in libiberty.  I
didn't know this existed before; I only ran across it last week.  I
did need one new API here, so I added it.  This change let me remove
some utility code from server.c.

Next, this defers starting the assembler until we've forked the
subprocess.  It also adds a new compilation_job structure, which is
where we'll store per-job information; right now it is fairly minimal
but I expect to add more there later.

Note that this new approach to invoking the assembler makes it a bit
harder to get proper exit status.  This is something I need to work
on; right now I'm thinking that the client-side gcc should probably
send its stderr fd to the server process for error output, and we
should reserve the socket connection for control.  This will make it
simpler to report a correct status back.

Eventually, as part of incremental code generation, we'll be invoking
the assembler multiple times.  So, it is important to start it in the
child rather than the parent.  We'll still need some kind of error
feedback here as well, though.

Tom

include/ChangeLog:
2007-10-25  Tom Tromey  <tromey@redhat.com>

	* libiberty.h (argvtostr): Declare.

libiberty/ChangeLog:
2007-10-25  Tom Tromey  <tromey@redhat.com>

	* argv.c (argvtostr): New function.

gcc/ChangeLog:
2007-10-25  Tom Tromey  <tromey@redhat.com>

	* Makefile.in (GTFILES): Added toplev.c.
	* toplev.c (job_arguments): Removed.
	(struct compilation_job): New.
	(job): New global.
	(compile_file): Collect before forking.  Start 'as'.
	(finalize): Call wait_for_as.
	(wait_for_as): New function.
	(copy_to_vec): Likewise.
	(server_callback): Create a compilation_job.  Don't start or wait
	for as.
	Include gt-toplev.h.
	* server.c: Don't include dyn-string.h.
	(free_argv, split_argv): Removed.
	(request_and_response): Use buildargv, freeargv.
	(client_send_command): Use argvtostr.

2007-10-25  Tom Tromey  <tromey@redhat.com>

Index: include/libiberty.h
===================================================================
--- include/libiberty.h	(revision 129422)
+++ include/libiberty.h	(working copy)
@@ -1,6 +1,6 @@
 /* Function declarations for libiberty.
 
-   Copyright 2001, 2002, 2005 Free Software Foundation, Inc.
+   Copyright 2001, 2002, 2005, 2007 Free Software Foundation, Inc.
    
    Note - certain prototypes declared in this header file are for
    functions whoes implementation copyright does not belong to the
@@ -90,6 +90,9 @@
 
 extern int writeargv PARAMS ((char **, FILE *));
 
+/* Turn argv array into quoted string.  */
+extern char *argvtostr PARAMS ((const char **));
+
 /* Return the last component of a path name.  Note that we can't use a
    prototype here because the parameter is declared inconsistently
    across different systems, sometimes as "char *" and sometimes as
Index: libiberty/argv.c
===================================================================
--- libiberty/argv.c	(revision 129422)
+++ libiberty/argv.c	(working copy)
@@ -1,5 +1,5 @@
 /* Create and destroy argument vectors (argv's)
-   Copyright (C) 1992, 2001 Free Software Foundation, Inc.
+   Copyright (C) 1992, 2001, 2007 Free Software Foundation, Inc.
    Written by Fred Fish @ Cygnus Support
 
 This file is part of the libiberty library.
@@ -28,6 +28,7 @@
 #include "ansidecl.h"
 #include "libiberty.h"
 #include "safe-ctype.h"
+#include "dyn-string.h"
 
 /*  Routines imported from standard C runtime libraries. */
 
@@ -290,6 +291,42 @@
 
 /*
 
+@deftypefn Extension char* argvtostr (const char **@var{argv})
+
+Return a new string which, when run through @code{buildargv}, will
+yield ARGV.  The return value is allocated using @code{malloc}.
+
+@end deftypefn
+
+*/
+
+char *
+argvtostr (const char **argv)
+{
+  dyn_string_t quoted;
+  int i;
+
+  quoted = dyn_string_new (50);
+
+  for (i = 0; argv[i]; ++i)
+    {
+      int j;
+      if (i > 0)
+	dyn_string_append_char (quoted, ' ');
+      for (j = 0; argv[i][j]; ++j)
+	{
+	  char c = argv[i][j];
+          if (ISSPACE(c) || c == '\\' || c == '\'' || c == '"')
+	    dyn_string_append_char (quoted, '\\');
+	  dyn_string_append_char (quoted, c);
+	}
+    }
+
+  return dyn_string_release (quoted);
+}
+
+/*
+
 @deftypefn Extension int writeargv (const char **@var{argv}, FILE *@var{file})
 
 Write each member of ARGV, handling all necessary quoting, to the file
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	(revision 129504)
+++ gcc/toplev.c	(working copy)
@@ -112,6 +112,8 @@
 static void crash_signal (int) ATTRIBUTE_NORETURN;
 static void setup_core_dumping (void);
 static bool compile_file (void);
+static struct pex_obj *start_as (char **);
+static void wait_for_as (struct pex_obj *, int *);
 
 /* Nonzero to dump debug info whilst parsing (-dy option).  */
 static int set_yydebug;
@@ -397,11 +399,24 @@
 DEF_VEC_P(cchar_p);
 DEF_VEC_ALLOC_P(cchar_p,gc);
 
-/* Saved command-line arguments for the current job.  The server
-   copies these into GC-allocated memory so that file names, etc, are
-   not deleted when the current job completes.  */
-static GTY(()) VEC(cchar_p,gc) *job_arguments;
+/* The current compilation job.  */
+struct compilation_job GTY(())
+{
+  /* The command-line arguments to the compiler.  This is GC allocated
+     so that file names and the like are not deleted after the current
+     job has completed.  */
+  VEC(cchar_p,gc) *cc1_arguments;
 
+  /* The command-line arguments to the assembler.  */
+  VEC(cchar_p,gc) *as_arguments;
+
+  /* If not null, the executing assembler sub-process.  */
+  struct pex_obj * GTY ((skip)) as_process;
+};
+
+/* The current compilation job.  */
+static GTY(()) struct compilation_job *job;
+
 /* The current working directory of a translation.  It's generally the
    directory from which compilation was initiated, but a preprocessed
    file may specify the original directory in which it was
@@ -1075,12 +1090,19 @@
   if (flag_syntax_only)
     return false;
 
-  if (server_mode && server_start_back_end ())
-    return false;
-
   if (server_mode)
     {
+      /* Collect here so that the child process has the minimal amount
+	 of unclaimed garbage.  */
+      ggc_collect ();
+
+      if (server_start_back_end ())
+	return false;
+
+      job->as_process = start_as ((char **) VEC_address (cchar_p,
+							 job->as_arguments));
       gcc_assert (server_asm_out_file && !asm_out_file);
+
       asm_out_file = server_asm_out_file;
       server_asm_out_file = NULL;
       init_asm_output (main_input_filename);
@@ -2167,6 +2189,14 @@
       server_asm_out_file = NULL;
     }
 
+  if (job->as_process)
+    {
+      int result;
+      wait_for_as (job->as_process, &result);
+      /* FIXME: handle error */
+      job->as_process = NULL;
+    }
+
   finish_optimization_passes ();
 
   if (mem_report)
@@ -2250,10 +2280,33 @@
   return px;
 }
 
+static void
+wait_for_as (struct pex_obj *px, int *result)
+{
+  /* FIXME: send a single byte back to the client for status?
+     FIXME: this function should return an error indication.  */
+  pex_get_status (px, 1, result);
+  pex_free (px);
+}
+
+/* Copy an argument vector into a VEC, reallocating all the strings
+   via the GC.  */
+static void
+copy_to_vec (VEC (cchar_p, gc) **out, char **args, int *n_args)
+{
+  int i;
+  *out = NULL;
+  for (i = 0; args[i]; ++i)
+    VEC_safe_push (cchar_p, gc, *out, ggc_strdup (args[i]));
+  VEC_safe_push (cchar_p, gc, *out, NULL);
+  if (n_args)
+    *n_args = i - 1;
+}
+
 void
 server_callback (int fd, char **cc1_argv, char **as_argv)
 {
-  struct pex_obj *px;
+  int n;
 
   /* For now, work single-threaded and just stomp on global state as
      needed.  */
@@ -2267,31 +2320,16 @@
   errorcount = 0;
   sorrycount = 0;
 
-  px = start_as (as_argv);
+  job = GGC_CNEW (struct compilation_job);
+  copy_to_vec (&job->as_arguments, as_argv, NULL);
+  copy_to_vec (&job->cc1_arguments, cc1_argv, &n);
 
-  if (px)
-    {
-      int n, result;
-      /* Copy the arguments into GC memory.  */
-      job_arguments = NULL;
-      for (n = 0; cc1_argv[n]; ++n)
-	{
-	  VEC_safe_push (cchar_p, gc, job_arguments, ggc_strdup (cc1_argv[n]));
-	}
-      VEC_safe_push (cchar_p, gc, job_arguments, NULL);
-      decode_options (n, VEC_address (cchar_p, job_arguments));
+  decode_options (n, VEC_address (cchar_p, job->cc1_arguments));
+  flag_unit_at_a_time = 1;
 
-      flag_unit_at_a_time = 1;
+  init_local_tick ();
+  do_compile ();
 
-      init_local_tick ();		/* FIXME... */
-      do_compile ();
-
-      /* FIXME: send a single byte back to the client for status?
-	 FIXME: this function should return an error indication.  */
-      pex_get_status (px, 1, &result);
-      pex_free (px);
-    }
-
   /* Clean up after this compilation.  FIXME: instead of clean_up()
      running before a job, as we have in a couple files, we should
      clean each module after compilation.  This will reduce peak
@@ -2299,6 +2337,8 @@
   cgraph_reset ();
   cgraph_unit_reset ();
   varpool_reset ();
+  job = NULL;
+
   ggc_collect ();
 
   /* Make sure to close dup'd stderr, so that client will terminate
@@ -2344,3 +2384,5 @@
 
   return (SUCCESS_EXIT_CODE);
 }
+
+#include "gt-toplev.h"
Index: gcc/server.c
===================================================================
--- gcc/server.c	(revision 129504)
+++ gcc/server.c	(working copy)
@@ -23,7 +23,6 @@
 #include "coretypes.h"
 #include "server.h"
 #include "errors.h"
-#include "dyn-string.h"
 #include "opts.h"
 
 #include <sys/socket.h>
@@ -138,73 +137,6 @@
   return sockfd;
 }
 
-/* Free the contents of an ARGV array, then the array itself.  */
-static void
-free_argv (char **argv)
-{
-  int i;
-  for (i = 0; argv[i]; ++i)
-    free (argv[i]);
-  free (argv);
-}
-
-/* Split a string into elements at spaces.  A backslash in the string
-   quotes the following character.  The resulting array should later
-   be freed with free_argv.  */
-static char **
-split_argv (char *buffer)
-{
-  char **argv = NULL;
-  int alloc = 0;
-  int used = 0;
-  char *p = buffer;
-  dyn_string_t str = dyn_string_new (20);
-
-  while (*p)
-    {
-      if (*p == '\\')
-	{
-	  ++p;
-	  if (!*p)
-	    {
-	      /* Not actually valid...  */
-	      break;
-	    }
-	  dyn_string_append_char (str, *p);
-	}
-      else if (*p == ' ')
-	{
-	  if (used == alloc)
-	    {
-	      alloc = 2 * alloc + 20;
-	      argv = (char **) xrealloc (argv, alloc * sizeof (char *));
-	    }
-	  argv[used++] = dyn_string_release (str);
-	  if (!*p)
-	    {
-	      str = NULL;
-	      break;
-	    }
-	  str = dyn_string_new (20);
-	}
-      else
-	dyn_string_append_char (str, *p);
-      ++p;
-    }
-
-  if (used + 2 >= alloc)
-    {
-      alloc = used + 2;
-      argv = (char **) xrealloc (argv, alloc * sizeof (char *));
-    }
-
-  if (str)
-    argv[used++] = dyn_string_release (str);
-  argv[used] = NULL;
-
-  return argv;
-}
-
 /* Helper function for server_main_loop.  Read a request from the file
    descriptor REQFD and take action.  */
 static bool
@@ -231,7 +163,7 @@
 	    break;
 	  buffer[len] = '\0';
 
-	  argv = split_argv (buffer);
+	  argv = buildargv (buffer);
 	  free (buffer);
 	  /* FIXME: obviously this is pretty lame.  */
 	  if (count >= 2)
@@ -252,8 +184,8 @@
 	      continue;
 	    }
 	  server_callback (reqfd, argvs[0], argvs[1]);
-	  free_argv (argvs[0]);
-	  free_argv (argvs[1]);
+	  freeargv (argvs[0]);
+	  freeargv (argvs[1]);
 	  count = 0;
 	  break;
 	}
@@ -374,30 +306,13 @@
 bool
 client_send_command (const char **argv)
 {
-  dyn_string_t quoted;
   char cmd, *line;
-  int i, len;
+  int len;
   bool result;
 
   gcc_assert (connection_fd >= 0);
 
-  quoted = dyn_string_new (50);
-
-  for (i = 0; argv[i]; ++i)
-    {
-      int j;
-      if (i > 0)
-	dyn_string_append_char (quoted, ' ');
-      for (j = 0; argv[i][j]; ++j)
-	{
-	  if (argv[i][j] == ' ' || argv[i][j] == '\\')
-	    dyn_string_append_char (quoted, '\\');
-	  dyn_string_append_char (quoted, argv[i][j]);
-	}
-    }
-
-  line = dyn_string_release (quoted);
-
+  line = argvtostr (argv);
   len = strlen (line);
   cmd = 'X';
 
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 129504)
+++ gcc/Makefile.in	(working copy)
@@ -3078,7 +3078,8 @@
   $(srcdir)/profile.c $(srcdir)/regclass.c \
   $(srcdir)/reg-stack.c $(srcdir)/cfglayout.c $(srcdir)/cfglayout.h \
   $(srcdir)/sdbout.c $(srcdir)/stor-layout.c \
-  $(srcdir)/stringpool.c $(srcdir)/tree.c $(srcdir)/varasm.c \
+  $(srcdir)/stringpool.c $(srcdir)/toplev.c $(srcdir)/tree.c \
+  $(srcdir)/varasm.c \
   $(srcdir)/tree-mudflap.c $(srcdir)/tree-flow.h $(srcdir)/tree-scalar-evolution.c \
   $(srcdir)/tree-ssanames.c $(srcdir)/tree-eh.c $(srcdir)/tree-ssa-address.c \
   $(srcdir)/tree-phinodes.c $(srcdir)/tree-cfg.c \


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]