[PATCH] Support variables in expansion of -fprofile-generate option (PR gcov-profile/47618).

[PATCH] Support variables in expansion of -fprofile-generate option (PR gcov-profile/47618).

[Martin Liška]

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

Hi.

Following patch enables to generate more parallel profiles for applications
that do intensive # of invocations. There's some discussion in the PR.

So one example:

$ gcc -fprofile-generate=/tmp/slavia/%p/%q{CPU}/ empty.c -O2 && ./a.out
$ l /tmp/slavia/22234/x86_64/empty.gcda
-rw-r--r-- 1 marxin users 172 May 18 13:20 /tmp/slavia/22234/x86_64/empty.gcda

Ready for trunk?
Thanks,
Martin

gcc/ChangeLog:

2018-05-18  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/47618
	* opts.c (expand_profile_data_prefix): New function.
	(finish_options): Use it.

libgcc/ChangeLog:

2018-05-18  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/47618
	* libgcov-driver-system.c (replace_filename_variables):
	New function.
	(gcov_exit_open_gcda_file): Use it.
---
 gcc/opts.c                     | 35 ++++++++++++++++++++
 libgcc/libgcov-driver-system.c | 73 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)



[-- Attachment #2: 0001-Support-variables-in-expansion-of-fprofile-generate-.patch --]
[-- Type: text/x-patch, Size: 3714 bytes --]

diff --git a/gcc/opts.c b/gcc/opts.c
index 33efcc0d6e7..fc337a9463f 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -690,6 +690,38 @@ default_options_optimization (struct gcc_options *opts,
 			 lang_mask, handlers, loc, dc);
 }
 
+/* Expand variables in x_profile_data_prefix.
+   Currently we support following options:
+
+   %w - current working directory
+   */
+
+static void
+expand_profile_data_prefix (gcc_options *opts)
+{
+  if (opts->x_profile_data_prefix != NULL)
+    {
+      const char *needle = "%w";
+      unsigned needle_strlen = strlen (needle);
+      while (true)
+	{
+	  char *p = CONST_CAST (char *, strstr (opts->x_profile_data_prefix,
+						needle));
+	  if (p)
+	    {
+	      *p = '\0';
+	      char *r = concat (opts->x_profile_data_prefix, getpwd (),
+				p + needle_strlen, NULL);
+
+	      free (CONST_CAST (char *, opts->x_profile_data_prefix));
+	      opts->x_profile_data_prefix = r;
+	    }
+	  else
+	    break;
+      }
+    }
+}
+
 /* After all options at LOC have been read into OPTS and OPTS_SET,
    finalize settings of those options and diagnose incompatible
    combinations.  */
@@ -1059,6 +1091,9 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
   if (opts->x_align_labels > MAX_CODE_ALIGN_VALUE)
     error_at (loc, "-falign-labels=%d is not between 0 and %d",
 	      opts->x_align_labels, MAX_CODE_ALIGN_VALUE);
+
+  /* Expand variables in x_profile_data_prefix.  */
+  expand_profile_data_prefix (opts);
 }
 
 #define LEFT_COLUMN	27
diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 0df44239363..d45ac4b31ba 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -128,6 +128,77 @@ create_file_directory (char *filename)
 #endif
 }
 
+/* Replace filename variables in FILENAME.  We currently support expansion:
+
+   %p - process ID
+   %q{ENV} - value of environment variable ENV
+   */
+
+static char *
+replace_filename_variables (char *filename)
+{
+  char buffer[16];
+  char empty[] = "";
+  for (char *p = filename; *p != '\0'; p++)
+    {
+      unsigned length = strlen (filename);
+      if (*p == '%' && *(p + 1) != '\0')
+	{
+	  unsigned start = p - filename;
+	  p++;
+	  char *replacement = NULL;
+	  switch (*p)
+	    {
+	    case 'p':
+	      sprintf (buffer, "%d", getpid ());
+	      replacement = buffer;
+	      p++;
+	      break;
+	    case 'q':
+	      if (*(p + 1) == '{')
+		{
+		  p += 2;
+		  char *e = strchr (p, '}');
+		  if (e)
+		    {
+		      *e = '\0';
+		      replacement = getenv (p);
+		      if (replacement == NULL)
+			replacement = empty;
+		      p = e + 1;
+		    }
+		  else
+		    return filename;
+		}
+	      break;
+	    default:
+	      return filename;
+	    }
+
+	  /* Concat beginning of the path, replacement and
+	     ending of the path.  */
+	  unsigned end = length - (p - filename);
+	  unsigned repl_length = strlen (replacement);
+
+	  char *buffer = (char *)xmalloc (start + end + repl_length + 1);
+	  char *buffer_ptr = buffer;
+	  memcpy (buffer_ptr, filename, start);
+	  buffer_ptr += start;
+	  memcpy (buffer_ptr, replacement, repl_length);
+	  buffer_ptr += repl_length;
+	  memcpy (buffer_ptr, p, end);
+	  buffer_ptr += end;
+	  *buffer_ptr = '\0';
+
+	  free (filename);
+	  filename = buffer;
+	  p = buffer + start + repl_length;
+	}
+    }
+
+  return filename;
+}
+
 static void
 allocate_filename_struct (struct gcov_filename *gf)
 {
@@ -216,6 +287,8 @@ gcov_exit_open_gcda_file (struct gcov_info *gi_ptr,
     }
   strcpy (dst, fname);
 
+  gf->filename = replace_filename_variables (gf->filename);
+
   if (!gcov_open (gf->filename))
     {
       /* Open failed likely due to missed directory.

Re: [PATCH] Support variables in expansion of -fprofile-generate option (PR gcov-profile/47618).

[Martin Liška]

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

Hi.

I'm sending V2, where I changed:

- removed expansion of '%w', it's handled in: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00729.html
- simplified concatenation in replace_filename_variables
- documentation for the expansion is added

Ready for trunk?

Martin

[-- Attachment #2: 0001-Support-variables-in-expansion-of-fprofile-generate-.patch --]
[-- Type: text/x-patch, Size: 3761 bytes --]

From ac35fffa250685ec9f5fd04c1076558769848f38 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 18 May 2018 13:12:06 +0200
Subject: [PATCH] Support variables in expansion of -fprofile-generate option
 (PR gcov-profile/47618).

gcc/ChangeLog:

2018-05-29  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/47618
	* doc/invoke.texi: Document how -fprofile-dir format
        is extended.

libgcc/ChangeLog:

2018-05-29  Martin Liska  <mliska@suse.cz>

	PR gcov-profile/47618
	* libgcov-driver-system.c (replace_filename_variables): New
        function.
	(gcov_exit_open_gcda_file): Use it.
---
 gcc/doc/invoke.texi            | 14 +++++++
 libgcc/libgcov-driver-system.c | 70 ++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 65f32d67640..42ab7e9211a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11294,6 +11294,20 @@ and its related options.  Both absolute and relative paths can be used.
 By default, GCC uses the current directory as @var{path}, thus the
 profile data file appears in the same directory as the object file.
 
+When an executable is run in a massive parallel environment, it is recommended
+to save profile to different folders.  That can be done with variables
+in @var{path} that are exported during run-time:
+
+@table @gcctabopt
+
+@item %p
+process ID.
+
+@item %q@{VAR@}
+value of environment variable @var{VAR}
+
+@end table
+
 @item -fprofile-generate
 @itemx -fprofile-generate=@var{path}
 @opindex fprofile-generate
diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 0df44239363..1216edb1a50 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -128,6 +128,74 @@ create_file_directory (char *filename)
 #endif
 }
 
+/* Replace filename variables in FILENAME.  We currently support expansion:
+
+   %p - process ID
+   %q{ENV} - value of environment variable ENV
+   */
+
+static char *
+replace_filename_variables (char *filename)
+{
+  char buffer[16];
+  char empty[] = "";
+  for (char *p = filename; *p != '\0'; p++)
+    {
+      unsigned length = strlen (filename);
+      if (*p == '%' && *(p + 1) != '\0')
+	{
+	  unsigned start = p - filename;
+	  p++;
+	  char *replacement = NULL;
+	  switch (*p)
+	    {
+	    case 'p':
+	      sprintf (buffer, "%d", getpid ());
+	      replacement = buffer;
+	      p++;
+	      break;
+	    case 'q':
+	      if (*(p + 1) == '{')
+		{
+		  p += 2;
+		  char *e = strchr (p, '}');
+		  if (e)
+		    {
+		      *e = '\0';
+		      replacement = getenv (p);
+		      if (replacement == NULL)
+			replacement = empty;
+		      p = e + 1;
+		    }
+		  else
+		    return filename;
+		}
+	      break;
+	    default:
+	      return filename;
+	    }
+
+	  /* Concat beginning of the path, replacement and
+	     ending of the path.  */
+	  unsigned end = length - (p - filename);
+	  unsigned repl_length = strlen (replacement);
+
+	  char *buffer = (char *)xmalloc (start + end + repl_length + 1);
+	  char *buffer_ptr = buffer;
+	  buffer_ptr = (char *)mempcpy (buffer_ptr, filename, start);
+	  buffer_ptr = (char *)mempcpy (buffer_ptr, replacement, repl_length);
+	  buffer_ptr = (char *)mempcpy (buffer_ptr, p, end);
+	  *buffer_ptr = '\0';
+
+	  free (filename);
+	  filename = buffer;
+	  p = buffer + start + repl_length;
+	}
+    }
+
+  return filename;
+}
+
 static void
 allocate_filename_struct (struct gcov_filename *gf)
 {
@@ -216,6 +284,8 @@ gcov_exit_open_gcda_file (struct gcov_info *gi_ptr,
     }
   strcpy (dst, fname);
 
+  gf->filename = replace_filename_variables (gf->filename);
+
   if (!gcov_open (gf->filename))
     {
       /* Open failed likely due to missed directory.
-- 
2.17.0

Re: [PATCH] Support variables in expansion of -fprofile-generate option (PR gcov-profile/47618).

[Petr Špaček]

On 29.5.2018 14:03, Martin Liška wrote:
> Hi.
> 
> I'm sending V2, where I changed:
> 
> - removed expansion of '%w', it's handled in: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00729.html
> - simplified concatenation in replace_filename_variables
> - documentation for the expansion is added
> 
> Ready for trunk?

It seems as step in the right direction. Thank you!

What's missing to address
https://github.com/linux-test-project/lcov/issues/37
completely?

-- 
Petr Špaček  @  CZ.NIC

Re: [PATCH] Support variables in expansion of -fprofile-generate option (PR gcov-profile/47618).

[Martin Liška]

On 05/29/2018 02:12 PM, Petr Špaček wrote:
> On 29.5.2018 14:03, Martin Liška wrote:
>> Hi.
>>
>> I'm sending V2, where I changed:
>>
>> - removed expansion of '%w', it's handled in: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00729.html
>> - simplified concatenation in replace_filename_variables
>> - documentation for the expansion is added
>>
>> Ready for trunk?
> 
> It seems as step in the right direction. Thank you!
> 
> What's missing to address
> https://github.com/linux-test-project/lcov/issues/37
> completely?
> 

Well, I added support to print to stdout:
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=260361

And GCC PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82702

mentioned in the lcov issue is also resolver.

Can you please specify what piece is missing for you?
Another way is to use gcov-tool to merge all the profiles (.gcda)
files created during parallel execution.

Martin

Re: [PATCH] Support variables in expansion of -fprofile-generate option (PR gcov-profile/47618).

[Martin Liška]

Installed as r261199.

Martin