Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-05 Thread Egeyar Bagcioglu





I'm sending the updated patch based on Egeyar's work.
It utilizes a new environmental variable and uses the currently
existing -frecord-gcc-switches option.

Thoughts?


I am leaving it to the more experienced to comment on redefining the 
functionality of -frecord-gcc-switches.


The code seems pretty neat to me. Thanks Martin!

Best regards
Egeyar


Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-05 Thread Martin Liška

Hi.

I'm sending the updated patch based on Egeyar's work.
It utilizes a new environmental variable and uses the currently
existing -frecord-gcc-switches option.

Thoughts?
Martin
>From 9436d12e7a540691c6f2d6e2db4730a138e5c458 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Thu, 5 Mar 2020 09:39:17 +0100
Subject: [PATCH] Change semantics of -frecord-gcc-switches.

gcc/ChangeLog:

2020-03-05  Martin Liska  
	Egeyar Bagcioglu  

	* common.opt: Make flag_record_gcc_switches also
	a driver option.
	* doc/tm.texi: Regenerate.
	* gcc.c (set_driver_command_line_envvar): New.
	(driver_handle_option): Handle OPT_frecord_gcc_switches
	and export command line into a ENV variable.
	(driver::main): Save command line.
	(driver::set_commandline): New.
	* gcc.h (set_commandline): New.
	* target.def: Simplify documentation by removal
	of unused enum values.
	* target.h (elf_record_gcc_switches): Change function type.
	* toplev.c (init_asm_output): Update call.
	* varasm.c (elf_record_gcc_switches): Record GCC version
	string and content of GCC_DRIVER_COMMAND_LINE.
---
 gcc/common.opt  |  2 +-
 gcc/doc/tm.texi | 37 ++---
 gcc/gcc.c   | 41 +
 gcc/gcc.h   |  1 +
 gcc/target.def  | 37 ++---
 gcc/target.h|  2 +-
 gcc/toplev.c| 11 +--
 gcc/varasm.c| 46 ++
 8 files changed, 59 insertions(+), 118 deletions(-)

diff --git a/gcc/common.opt b/gcc/common.opt
index fa9da505fc2..60ca5a521c5 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2246,7 +2246,7 @@ Common Joined RejectNegative Var(common_deferred_options) Defer
 ; records information in the assembler output file as comments, so
 ; they never reach the object file.
 frecord-gcc-switches
-Common Report Var(flag_record_gcc_switches)
+Common Driver Report Var(flag_record_gcc_switches)
 Record gcc command line switches in the object file.
 
 freg-struct-return
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 19985adac3e..30c71d60aff 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -8061,43 +8061,10 @@ need to override this if your target has special flags that might be
 set via @code{__attribute__}.
 @end deftypefn
 
-@deftypefn {Target Hook} int TARGET_ASM_RECORD_GCC_SWITCHES (print_switch_type @var{type}, const char *@var{text})
+@deftypefn {Target Hook} void TARGET_ASM_RECORD_GCC_SWITCHES (void)
 Provides the target with the ability to record the gcc command line
 switches that have been passed to the compiler, and options that are
-enabled.  The @var{type} argument specifies what is being recorded.
-It can take the following values:
-
-@table @gcctabopt
-@item SWITCH_TYPE_PASSED
-@var{text} is a command line switch that has been set by the user.
-
-@item SWITCH_TYPE_ENABLED
-@var{text} is an option which has been enabled.  This might be as a
-direct result of a command line switch, or because it is enabled by
-default or because it has been enabled as a side effect of a different
-command line switch.  For example, the @option{-O2} switch enables
-various different individual optimization passes.
-
-@item SWITCH_TYPE_DESCRIPTIVE
-@var{text} is either NULL or some descriptive text which should be
-ignored.  If @var{text} is NULL then it is being used to warn the
-target hook that either recording is starting or ending.  The first
-time @var{type} is SWITCH_TYPE_DESCRIPTIVE and @var{text} is NULL, the
-warning is for start up and the second time the warning is for
-wind down.  This feature is to allow the target hook to make any
-necessary preparations before it starts to record switches and to
-perform any necessary tidying up after it has finished recording
-switches.
-
-@item SWITCH_TYPE_LINE_START
-This option can be ignored by this target hook.
-
-@item  SWITCH_TYPE_LINE_END
-This option can be ignored by this target hook.
-@end table
-
-The hook's return value must be zero.  Other return values may be
-supported in the future.
+enabled.
 
 By default this hook is set to NULL, but an example implementation is
 provided for ELF based targets.  Called @var{elf_record_gcc_switches},
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 9f790db0daf..673ba2935f9 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -235,6 +235,11 @@ static int verbose_only_flag;
 
 static int print_subprocess_help;
 
+/* argc and argv used to call gcc.  Necessary for
+   --record-gcc-command-line option.  */
+static unsigned int driver_gcc_argc;
+static const char **driver_gcc_argv;
+
 /* Linker suffix passed to -fuse-ld=... */
 static const char *use_ld;
 
@@ -3724,6 +3729,28 @@ set_source_date_epoch_envvar ()
   setenv ("SOURCE_DATE_EPOCH", source_date_epoch, 0);
 }
 
+/* Set GCC_DRIVER_COMMAND_LINE enviromental variable that is later
+   used by -frecord-gcc-switches option.  */
+
+static void
+set_driver_command_line_envvar ()
+{
+  unsigned int length = 0;
+  for (unsigned int i = 0; i < driver_gcc_argc; i++)
+length += 

Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Richard Biener
On Wed, Mar 4, 2020 at 8:39 PM Egeyar Bagcioglu
 wrote:
>
>
>
> On 3/4/20 6:23 PM, Martin Liška wrote:
> > On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote:
> >> Thanks Richard.
> >>
> >> I do not have write-access to the GCC repo. I'd be glad if someone
> >> commits it for me.
> >
> > Can we please wait? I'm really convinced we do not want one another
> > very similar
> > functionality.
>
> I am sorry. I thought Richard's comment was only about the third patch,
> which is also necessary for the current -frecord-gcc-switches.
> Therefore, my reply was only for that patch as well.

Yes, my comment / approval was only about preserving the early compile
options for LTO.  I'll commit that piece momentarily.

Richard.

> Regards
> Egeyar
>
> > I would definitely recommend to change the semantics
> > of -frecord-gcc-switches to what the patchset does.
> >
> > That's why I added Nick as he's the author of the original
> > implementation.
> > Reasoning is provided in my previous email.
> >
> > Martin
>


Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Egeyar Bagcioglu




On 3/4/20 6:33 PM, Jakub Jelinek wrote:

On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote:

On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote:

Thanks Richard.

I do not have write-access to the GCC repo. I'd be glad if someone commits it 
for me.

Can we please wait? I'm really convinced we do not want one another very similar
functionality. I would definitely recommend to change the semantics
of -frecord-gcc-switches to what the patchset does.

That's why I added Nick as he's the author of the original implementation.
Reasoning is provided in my previous email.

Also, what is the reason for storing the option in a file, rather than say
putting them into an environment variable from which the backend can pick it
up?


I needed to pass that information from one process to the other and I 
picked a way to do so. Moreover, passing files is what gcc does; 
therefore, it didn't seem unnatural to do so. Furthermore, gcc specs 
already has its way to create temporary file names so I didn't have to 
worry about what level of thread safety is acceptable. If you tell me 
why an environment variable is better, we can discuss and I might 
implement that too.



   I must say I don't really see advantages of this over
-grecord-gcc-switches, recording all options looks very bloaty and will
include mostly stuff you don't really care about (such as, e.g. the -I
options without knowing what was the current directory when the source file
has been compiled), on the other side will not record interesting options
that -grecord-gcc-switches records (say, if some code is compiled with
-march=native, this new option will record that, rather than what it really
is),


1) -grecord-gcc-switches is quite similar to -frecord-gcc-switches and 
my first and main arguement about the latter is valid for the former:
You cannot easily construct the right command line to invoke the same 
compilation from the switches. Switches are useful to check that the 
compiler is doing the expected. The command line is useful when you want 
the compiler to do the same thing again.


2) The output is exactly what gcc takes as command line to make that 
compilation. It is bloaty, surely, when so is what's provided to gcc to 
make it compile as desired. On the other hand, -grecord-gcc-switches is 
just much bloatier since it does not work alone. It's embedded in dwarf.


3) Many systems automatically strip dwarf, together with this 
information. I do not think there is an easy way to separate this 
information and keep it.


Since this comes up often, I'd like to emphasize that the internal 
switches of gcc are different than the user-given options of gcc. The 
purpose of this new option is really to catch the user given options of 
gcc and only them, so much so that I mark it SWITCH_IGNORE within the 
specs to avoid it propagated to any child processes of gcc. This option 
is simply to retrieve how the call was made to GCC. Currently, there are 
no other options giving us this information.



  but I won't stand in a way unless such an option would be on by
default.


I totally agree that this should not be on by default.

Best regards
Egeyar



Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Egeyar Bagcioglu




On 3/4/20 6:23 PM, Martin Liška wrote:

On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote:

Thanks Richard.

I do not have write-access to the GCC repo. I'd be glad if someone 
commits it for me.


Can we please wait? I'm really convinced we do not want one another 
very similar
functionality. 


I am sorry. I thought Richard's comment was only about the third patch, 
which is also necessary for the current -frecord-gcc-switches. 
Therefore, my reply was only for that patch as well.


Regards
Egeyar


I would definitely recommend to change the semantics
of -frecord-gcc-switches to what the patchset does.

That's why I added Nick as he's the author of the original 
implementation.

Reasoning is provided in my previous email.

Martin




Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Jakub Jelinek
On Wed, Mar 04, 2020 at 06:42:29PM +0100, Martin Liška wrote:
> >  I must say I don't really see advantages of this over
> > -grecord-gcc-switches, recording all options looks very bloaty and will
> > include mostly stuff you don't really care about (such as, e.g. the -I
> > options without knowing what was the current directory when the source file
> > has been compiled), on the other side will not record interesting options
> > that -grecord-gcc-switches records (say, if some code is compiled with
> > -march=native, this new option will record that, rather than what it really
> > is), but I won't stand in a way unless such an option would be on by
> > default.
> 
> Yes, it's a minor disadvantage. On the other hand one can check the fortify
> macros. I don't care much about them too, but what's the biggest benefit to me
> is that each argument will not go into it's own mergeable section. Then
> you will not see something like:

Well, the fortify macro is questionable, because as a macro, it can be
either specified on the command line, or e.g. defined in the source before
including headers, so -g3 seems much better way to query it.

> The output is useless and can't disambiguate each compiler
> invocations.

Sure, I'm not talking about -frecord-gcc-switches, that option is indeed
not really useful.

Jakub



Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Martin Liška

On 3/4/20 6:33 PM, Jakub Jelinek wrote:

On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote:

On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote:

Thanks Richard.

I do not have write-access to the GCC repo. I'd be glad if someone commits it 
for me.


Can we please wait? I'm really convinced we do not want one another very similar
functionality. I would definitely recommend to change the semantics
of -frecord-gcc-switches to what the patchset does.

That's why I added Nick as he's the author of the original implementation.
Reasoning is provided in my previous email.


Also, what is the reason for storing the option in a file, rather than say
putting them into an environment variable from which the backend can pick it
up?


That's smart suggestion! It would not be the first environment variable that
we use, right?


 I must say I don't really see advantages of this over
-grecord-gcc-switches, recording all options looks very bloaty and will
include mostly stuff you don't really care about (such as, e.g. the -I
options without knowing what was the current directory when the source file
has been compiled), on the other side will not record interesting options
that -grecord-gcc-switches records (say, if some code is compiled with
-march=native, this new option will record that, rather than what it really
is), but I won't stand in a way unless such an option would be on by
default.


Yes, it's a minor disadvantage. On the other hand one can check the fortify
macros. I don't care much about them too, but what's the biggest benefit to me
is that each argument will not go into it's own mergeable section. Then
you will not see something like:

$ gcc -O0 foo.c -frecord-gcc-switches -c -g
$ gcc -Ofast bar.c -frecord-gcc-switches -c -g0
$ gcc foo.o bar.o
$ readelf -p .GCC.command.line a.out

String dump of section '.GCC.command.line':
  [ 0]  foo.c
  [ 6]  -mtune=generic
  [15]  -march=x86-64
  [23]  -g
  [26]  -O0
  [2a]  -frecord-gcc-switches
  [40]  bar.c
  [46]  -g0
  [4a]  -Ofast

The output is useless and can't disambiguate each compiler
invocations.

Martin



Jakub





Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Jakub Jelinek
On Wed, Mar 04, 2020 at 06:23:10PM +0100, Martin Liška wrote:
> On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote:
> > Thanks Richard.
> > 
> > I do not have write-access to the GCC repo. I'd be glad if someone commits 
> > it for me.
> 
> Can we please wait? I'm really convinced we do not want one another very 
> similar
> functionality. I would definitely recommend to change the semantics
> of -frecord-gcc-switches to what the patchset does.
> 
> That's why I added Nick as he's the author of the original implementation.
> Reasoning is provided in my previous email.

Also, what is the reason for storing the option in a file, rather than say
putting them into an environment variable from which the backend can pick it
up?  I must say I don't really see advantages of this over
-grecord-gcc-switches, recording all options looks very bloaty and will
include mostly stuff you don't really care about (such as, e.g. the -I
options without knowing what was the current directory when the source file
has been compiled), on the other side will not record interesting options
that -grecord-gcc-switches records (say, if some code is compiled with
-march=native, this new option will record that, rather than what it really
is), but I won't stand in a way unless such an option would be on by
default.

Jakub



Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Martin Liška

On 3/4/20 4:25 PM, Egeyar Bagcioglu wrote:

Thanks Richard.

I do not have write-access to the GCC repo. I'd be glad if someone commits it 
for me.


Can we please wait? I'm really convinced we do not want one another very similar
functionality. I would definitely recommend to change the semantics
of -frecord-gcc-switches to what the patchset does.

That's why I added Nick as he's the author of the original implementation.
Reasoning is provided in my previous email.

Martin


Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Egeyar Bagcioglu




On 3/4/20 4:34 PM, Andreas Schwab wrote:

On Mär 04 2020, Richard Biener wrote:


--record-gcc-command-line is not a FSF GCC option, there's
-frecord-gcc-switches though which

--record-gcc-command-line is translated to -frecord-gcc-switches by the
driver.  That happens for all double-dash options that match an -f
option.


I think there is a misunderstanding here.

I have just implemented --record-gcc-command-line. I have been testing 
it and no it does not act as -frecord-gcc-switches.


Regards
Egeyar


Andreas.





Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Andreas Schwab
On Mär 04 2020, Richard Biener wrote:

> --record-gcc-command-line is not a FSF GCC option, there's
> -frecord-gcc-switches though which

--record-gcc-command-line is translated to -frecord-gcc-switches by the
driver.  That happens for all double-dash options that match an -f
option.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Egeyar Bagcioglu




On 3/4/20 10:00 AM, Richard Biener wrote:

On Tue, Mar 3, 2020 at 5:41 PM Egeyar Bagcioglu
 wrote:

This patch is for .GCC.command.line sections in LTO objects to be copied
into the final objects as in the following example:

[egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line
[egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line 
-DFORTIFY=2
[egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out
[egeyar@localhost lto]$ readelf -p .GCC.command.line a.out

String dump of section '.GCC.command.line':
   [ 0]  10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g 
--record-gcc-command-line
   [56]  10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g 
--record-gcc-command-line -DFORTIFY=2

--record-gcc-command-line is not a FSF GCC option, there's
-frecord-gcc-switches though which
(also) populates .GCC.command.line


Right. This is also necessary for preserving the outcome of 
-frecord-gcc-switches option and that issue is reported to me by Martin 
Liska.



OK.


Thanks Richard.

I do not have write-access to the GCC repo. I'd be glad if someone 
commits it for me.


Regards
Egeyar



Thanks,
Richard.


Regards
Egeyar

libiberty:
2020-02-27  Egeyar Bagcioglu  

 * simple-object.c (handle_lto_debug_sections): Name
 ".GCC.command.line" among debug sections to be copied over
 from lto objects.
---
  libiberty/simple-object.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c
index d9c648a..3b3ca9c 100644
--- a/libiberty/simple-object.c
+++ b/libiberty/simple-object.c
@@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename)
   COMDAT sections in objects produced by GCC.  */
else if (strcmp (name, ".comment") == 0)
  return strcpy (newname, name);
+  /* Copy over .GCC.command.line section under the same name if present.  */
+  else if (strcmp (name, ".GCC.command.line") == 0)
+return strcpy (newname, name);
free (newname);
return NULL;
  }
--
1.8.3.1





Re: [PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-04 Thread Richard Biener
On Tue, Mar 3, 2020 at 5:41 PM Egeyar Bagcioglu
 wrote:
>
> This patch is for .GCC.command.line sections in LTO objects to be copied
> into the final objects as in the following example:
>
> [egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line
> [egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line 
> -DFORTIFY=2
> [egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out
> [egeyar@localhost lto]$ readelf -p .GCC.command.line a.out
>
> String dump of section '.GCC.command.line':
>   [ 0]  10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g 
> --record-gcc-command-line
>   [56]  10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g 
> --record-gcc-command-line -DFORTIFY=2

--record-gcc-command-line is not a FSF GCC option, there's
-frecord-gcc-switches though which
(also) populates .GCC.command.line

OK.

Thanks,
Richard.

> Regards
> Egeyar
>
> libiberty:
> 2020-02-27  Egeyar Bagcioglu  
>
> * simple-object.c (handle_lto_debug_sections): Name
> ".GCC.command.line" among debug sections to be copied over
> from lto objects.
> ---
>  libiberty/simple-object.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c
> index d9c648a..3b3ca9c 100644
> --- a/libiberty/simple-object.c
> +++ b/libiberty/simple-object.c
> @@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename)
>   COMDAT sections in objects produced by GCC.  */
>else if (strcmp (name, ".comment") == 0)
>  return strcpy (newname, name);
> +  /* Copy over .GCC.command.line section under the same name if present.  */
> +  else if (strcmp (name, ".GCC.command.line") == 0)
> +return strcpy (newname, name);
>free (newname);
>return NULL;
>  }
> --
> 1.8.3.1
>


[PATCH v2 3/3] Keep .GCC.command.line sections of LTO objetcs.

2020-03-03 Thread Egeyar Bagcioglu
This patch is for .GCC.command.line sections in LTO objects to be copied
into the final objects as in the following example:

[egeyar@localhost lto]$ gcc -flto -O3 demo.c -c -g --record-gcc-command-line
[egeyar@localhost lto]$ gcc -flto -O2 demo2.c -c -g --record-gcc-command-line 
-DFORTIFY=2
[egeyar@localhost lto]$ gcc demo.o demo2.o -o a.out  
[egeyar@localhost lto]$ readelf -p .GCC.command.line a.out 

String dump of section '.GCC.command.line':
  [ 0]  10.0.1 20200227 (experimental) : gcc -flto -O3 demo.c -c -g 
--record-gcc-command-line
  [56]  10.0.1 20200227 (experimental) : gcc -flto -O2 demo2.c -c -g 
--record-gcc-command-line -DFORTIFY=2

Regards
Egeyar

libiberty:
2020-02-27  Egeyar Bagcioglu  

* simple-object.c (handle_lto_debug_sections): Name
".GCC.command.line" among debug sections to be copied over
from lto objects.
---
 libiberty/simple-object.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c
index d9c648a..3b3ca9c 100644
--- a/libiberty/simple-object.c
+++ b/libiberty/simple-object.c
@@ -298,6 +298,9 @@ handle_lto_debug_sections (const char *name, int rename)
  COMDAT sections in objects produced by GCC.  */
   else if (strcmp (name, ".comment") == 0)
 return strcpy (newname, name);
+  /* Copy over .GCC.command.line section under the same name if present.  */
+  else if (strcmp (name, ".GCC.command.line") == 0)
+return strcpy (newname, name);
   free (newname);
   return NULL;
 }
-- 
1.8.3.1