Make it possible to have different instrumented and feedback builds without copying gcda files around [pr93401]

Message ID 20200409124411.GA68771@kam.mff.cuni.cz
State New
Headers show
Series
  • Make it possible to have different instrumented and feedback builds without copying gcda files around [pr93401]
Related show

Commit Message

Jan Hubicka April 9, 2020, 12:44 p.m.
Hi,
in GCC 8 we changed -fprofile-generate=<path> to use mangled absolute paths in
the <path> directory. This was necessary to avoid clashes of files when gcc is
executed from different directories to build different sources of same
filename.

However this made it difficult to build projects on setups where instrumented
build is done in one directory, feedback build in different and possibly
training happens in yet another directory structure.  This happens i.e. for
Firefox builds for month or two.

This patch adds -fprofile-prefix-path that can be used to inform gcc where the
root of build directory is and strip it form the gcda filenames.
This is similar to exisitng debug-prefix-map but without the map feature since
it seems useless for profile data.

We spent quite some time with Maritn Liska discussing options and found no
better solution.  I was looking how this work on LLVM and they produce single
profdata file which is then transformed into kind of simple database by
llvmprofdata tool.  This database keys functions by filename and symbol name.
If you arrane two files with same name define static variable with same symbol
name this gets messedup and result in wrong info. So I think this is not very
good solution and preffer the extra option.

Bootstrapped/regtested x86_64-linux. I plan to commit it later today if there
are no complains.

I suppose our manual could have some central section on profile feedback
explaining the whole setup at one place.

Honza

	* common.opt (profile-prefix-path): New option.
	* coverae.c: Include diagnostics.h.
	(coverage_init): Strip profile prefix path.
	* doc/invoke.texi (-fprofile-prefix-path): Document.

Comments

Martin Liška April 15, 2020, 7:29 a.m. | #1
On 4/9/20 2:44 PM, Jan Hubicka wrote:
> Hi,

> in GCC 8 we changed -fprofile-generate=<path> to use mangled absolute paths in

> the <path> directory. This was necessary to avoid clashes of files when gcc is

> executed from different directories to build different sources of same

> filename.

> 

> However this made it difficult to build projects on setups where instrumented

> build is done in one directory, feedback build in different and possibly

> training happens in yet another directory structure.  This happens i.e. for

> Firefox builds for month or two.

> 

> This patch adds -fprofile-prefix-path that can be used to inform gcc where the

> root of build directory is and strip it form the gcda filenames.

> This is similar to exisitng debug-prefix-map but without the map feature since

> it seems useless for profile data.

> 

> We spent quite some time with Maritn Liska discussing options and found no

> better solution.  I was looking how this work on LLVM and they produce single

> profdata file which is then transformed into kind of simple database by

> llvmprofdata tool.  This database keys functions by filename and symbol name.

> If you arrane two files with same name define static variable with same symbol

> name this gets messedup and result in wrong info. So I think this is not very

> good solution and preffer the extra option.


Hello.

I welcome the patch. There are 2 small formatting corrections:

diff --git a/gcc/common.opt b/gcc/common.opt
index 5662d46630a..1e604ba9bb6 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2198,7 +2198,7 @@ Enum(profile_update) String(prefer-atomic) Value(PROFILE_UPDATE_PREFER_ATOMIC)
  
  fprofile-prefix-path=
  Common Joined RejectNegative Var(profile_prefix_path)
-remove prefix from absolute path before manging name for -fprofile-generate= and -fprofile-use=.
+Remove prefix from absolute path before manging name for -fprofile-generate= and -fprofile-use=.
  
  fprofile-generate
  Common
diff --git a/gcc/coverage.c b/gcc/coverage.c
index d09a5749fbd..45c0278f44f 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -1232,8 +1232,8 @@ coverage_init (const char *filename)
  		    filename++;
  		}
  	      else
-		warning (0, "filename %s does not start with profile prefix %s",
-			 filename, profile_prefix_path);
+		warning (0, "filename %qs does not start with profile "
+			 "prefix %qs", filename, profile_prefix_path);
  	    }
  	  filename = mangle_path (filename);
  	  len = strlen (filename);

> 

> Bootstrapped/regtested x86_64-linux. I plan to commit it later today if there

> are no complains.

> 

> I suppose our manual could have some central section on profile feedback

> explaining the whole setup at one place.


Yes, we have it spread in various places. The option can be mentioned in:
10.4 Brief Description of gcov Data Files

And can you please add the option to GCC 10 changes please?

Thanks,
Martin

> 

> Honza

> 

> 	* common.opt (profile-prefix-path): New option.

> 	* coverae.c: Include diagnostics.h.

> 	(coverage_init): Strip profile prefix path.

> 	* doc/invoke.texi (-fprofile-prefix-path): Document.

> diff --git a/gcc/common.opt b/gcc/common.opt

> index bb2ea4c905d..5662d46630a 100644

> --- a/gcc/common.opt

> +++ b/gcc/common.opt

> @@ -2196,6 +2196,10 @@ Enum(profile_update) String(atomic) Value(PROFILE_UPDATE_ATOMIC)

>   EnumValue

>   Enum(profile_update) String(prefer-atomic) Value(PROFILE_UPDATE_PREFER_ATOMIC)

>   

> +fprofile-prefix-path=

> +Common Joined RejectNegative Var(profile_prefix_path)

> +remove prefix from absolute path before manging name for -fprofile-generate= and -fprofile-use=.

> +

>   fprofile-generate

>   Common

>   Enable common options for generating profile info for profile feedback directed optimizations.

> diff --git a/gcc/coverage.c b/gcc/coverage.c

> index 30ae84df90f..d09a5749fbd 100644

> --- a/gcc/coverage.c

> +++ b/gcc/coverage.c

> @@ -49,6 +49,7 @@ along with GCC; see the file COPYING3.  If not see

>   #include "intl.h"

>   #include "auto-profile.h"

>   #include "profile.h"

> +#include "diagnostic.h"

>   

>   #include "gcov-io.c"

>   

> @@ -1221,6 +1222,19 @@ coverage_init (const char *filename)

>   	  const char *separator = "/";

>   #endif

>   	  filename = concat (getpwd (), separator, filename, NULL);

> +	  if (profile_prefix_path)

> +	    {

> +	      if (!strncmp (filename, profile_prefix_path,

> +			    strlen (profile_prefix_path)))

> +		{

> +		  filename += strlen (profile_prefix_path);

> +		  while (*filename == *separator)

> +		    filename++;

> +		}

> +	      else

> +		warning (0, "filename %s does not start with profile prefix %s",

> +			 filename, profile_prefix_path);

> +	    }

>   	  filename = mangle_path (filename);

>   	  len = strlen (filename);

>   	}

> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi

> index be7b5bb7d71..afd8a6dbc03 100644

> --- a/gcc/doc/invoke.texi

> +++ b/gcc/doc/invoke.texi

> @@ -549,8 +549,9 @@ Objective-C and Objective-C++ Dialects}.

>   @gccoptlist{-p  -pg  -fprofile-arcs  --coverage  -ftest-coverage @gol

>   -fprofile-abs-path @gol

>   -fprofile-dir=@var{path}  -fprofile-generate  -fprofile-generate=@var{path} @gol

> --fprofile-note=@var{path}  -fprofile-update=@var{method} @gol

> --fprofile-filter-files=@var{regex}  -fprofile-exclude-files=@var{regex} -fprofile-reproducibility @gol

> +-fprofile-note=@var{path} -fprofile-prefix-path=@var{path} @gol

> +-fprofile-update=@var{method} -fprofile-filter-files=@var{regex} @gol

> +-fprofile-exclude-files=@var{regex} -fprofile-reproducibility @gol

>   -fsanitize=@var{style}  -fsanitize-recover  -fsanitize-recover=@var{style} @gol

>   -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},... @gol

>   -fsanitize-undefined-trap-on-error  -fbounds-check @gol

> @@ -13398,6 +13399,20 @@ If @var{path} is specified, GCC saves @file{.gcno} file into @var{path}

>   location.  If you combine the option with multiple source files,

>   the @file{.gcno} file will be overwritten.

>   

> +@item -fprofile-prefix-path=@var{path}

> +

> +This option can be used in combination with

> +@option{profile-generate=}@var{profile_dir} and

> +@option{profile-use=}@var{profile_dir} to inform GCC where is the base

> +directory of built source tree.  By default @var{profile_dir} will contain

> +files with mangled absolute paths of all object files in the built project.

> +This is not desirable when directory used to build the instrumented binary

> +differs from the directory used to build the binary optimized with profile

> +feedback because the profile data will not be found during the optimized build.

> +In such setups @option{-fprofile-prefix-path=}@var{path} with @var{path}

> +pointing to the base directory of the build can be used to strip the irrelevant

> +part of the path and keep all file names relative to the main build directory.

> +

>   @item -fprofile-update=@var{method}

>   @opindex fprofile-update

>   

>

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index bb2ea4c905d..5662d46630a 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2196,6 +2196,10 @@  Enum(profile_update) String(atomic) Value(PROFILE_UPDATE_ATOMIC)
 EnumValue
 Enum(profile_update) String(prefer-atomic) Value(PROFILE_UPDATE_PREFER_ATOMIC)
 
+fprofile-prefix-path=
+Common Joined RejectNegative Var(profile_prefix_path)
+remove prefix from absolute path before manging name for -fprofile-generate= and -fprofile-use=.
+
 fprofile-generate
 Common
 Enable common options for generating profile info for profile feedback directed optimizations.
diff --git a/gcc/coverage.c b/gcc/coverage.c
index 30ae84df90f..d09a5749fbd 100644
--- a/gcc/coverage.c
+++ b/gcc/coverage.c
@@ -49,6 +49,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "intl.h"
 #include "auto-profile.h"
 #include "profile.h"
+#include "diagnostic.h"
 
 #include "gcov-io.c"
 
@@ -1221,6 +1222,19 @@  coverage_init (const char *filename)
 	  const char *separator = "/";
 #endif
 	  filename = concat (getpwd (), separator, filename, NULL);
+	  if (profile_prefix_path)
+	    {
+	      if (!strncmp (filename, profile_prefix_path,
+			    strlen (profile_prefix_path)))
+		{
+		  filename += strlen (profile_prefix_path);
+		  while (*filename == *separator)
+		    filename++;
+		}
+	      else
+		warning (0, "filename %s does not start with profile prefix %s",
+			 filename, profile_prefix_path);
+	    }
 	  filename = mangle_path (filename);
 	  len = strlen (filename);
 	}
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index be7b5bb7d71..afd8a6dbc03 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -549,8 +549,9 @@  Objective-C and Objective-C++ Dialects}.
 @gccoptlist{-p  -pg  -fprofile-arcs  --coverage  -ftest-coverage @gol
 -fprofile-abs-path @gol
 -fprofile-dir=@var{path}  -fprofile-generate  -fprofile-generate=@var{path} @gol
--fprofile-note=@var{path}  -fprofile-update=@var{method} @gol
--fprofile-filter-files=@var{regex}  -fprofile-exclude-files=@var{regex} -fprofile-reproducibility @gol
+-fprofile-note=@var{path} -fprofile-prefix-path=@var{path} @gol
+-fprofile-update=@var{method} -fprofile-filter-files=@var{regex} @gol
+-fprofile-exclude-files=@var{regex} -fprofile-reproducibility @gol
 -fsanitize=@var{style}  -fsanitize-recover  -fsanitize-recover=@var{style} @gol
 -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},... @gol
 -fsanitize-undefined-trap-on-error  -fbounds-check @gol
@@ -13398,6 +13399,20 @@  If @var{path} is specified, GCC saves @file{.gcno} file into @var{path}
 location.  If you combine the option with multiple source files,
 the @file{.gcno} file will be overwritten.
 
+@item -fprofile-prefix-path=@var{path}
+
+This option can be used in combination with
+@option{profile-generate=}@var{profile_dir} and
+@option{profile-use=}@var{profile_dir} to inform GCC where is the base
+directory of built source tree.  By default @var{profile_dir} will contain
+files with mangled absolute paths of all object files in the built project.
+This is not desirable when directory used to build the instrumented binary
+differs from the directory used to build the binary optimized with profile
+feedback because the profile data will not be found during the optimized build.
+In such setups @option{-fprofile-prefix-path=}@var{path} with @var{path}
+pointing to the base directory of the build can be used to strip the irrelevant
+part of the path and keep all file names relative to the main build directory.
+
 @item -fprofile-update=@var{method}
 @opindex fprofile-update