diff mbox series

bootstrap-debug-lean + flags in producer vs compare

Message ID or1smh5b6i.fsf@lxoliva.fsfla.org
State New
Headers show
Series bootstrap-debug-lean + flags in producer vs compare | expand

Commit Message

Alexandre Oliva Oct. 6, 2017, 1:15 a.m. UTC
Unlike bootstrap-debug, bootstrap-debug-lean used to pass compare using
the traditional compare command, because it compiled both stage2 and
stage3 with options that used to generate identical output
(-fcompare-debug= in stage2 vs -fcompare-debug in stage3).

Since we started adding relevant command-line flags to DW_AT_producer,
this is no longer the case, and stages 2 and 3 object files that differ
in nothing but the DW_AT_producer strings.


-fcompare-debug is short for -fcompare-debug=-gtoggle, so stage3
compiles twice, once with the normal options, once with toggled -g, to
then compare the temporary final dumps.  When enabled, both compilations
get from the driver an additional -frandom-seed flag (if none is given
explicitly).

-fcompare-debug= is short for -fno-compare-debug, disabling the second
compilation.


The difference between the DW_AT_producer lines are the different
-fcompare-debug flags, and the presence of the -frandom-seed flag in the
stage3 compilation.

It should be easy and sensible enough to filter the -fcompare-debug
flags out of the DW_AT_producer string.  This option should never affect
the compilation output, it just determines whether or not to perform an
additional compilation that should produce the same executable output.

However, doing that but that won't get us rid of the -frandom-seed
option.  We could drop -frandom-seed from the DW_AT_producer output too,
but I don't think we should do that when the option is given by the
user, rather than implicitly introduced by -fcompare-debug.  We could
introduce an option that causes the subsequent option to be omitted from
the DW_AT_producer string, and arrange for -fcompare-debug to issue that
option before the -frandom-seed option it issues.  The problem with this
approach is that I can't decide whether it's an option prone to abuse,
or one that can have other legitimate uses.


Another approach to fix (or rather hide) this failure mode is to allow
the debug information to differ under bootstrap-debug-lean, by using the
same compare-debug script we use for bootstrap-debug.  The first
patchlet below does just that.  The second drops -fcompare-debug from
DW_AT_producer.  A third (not written yet :-) might somehow deal with
the -frandom-seed added by -fcompare-debug, and either the first or the
others would be posted as a single, fully tested patch, once we decide
how we want to deal with -frandom-seed.


Thoughts?  Thanks in advance,

Comments

Jeff Law Oct. 30, 2017, 11:12 p.m. UTC | #1
On 10/05/2017 07:15 PM, Alexandre Oliva wrote:
> Unlike bootstrap-debug, bootstrap-debug-lean used to pass compare using
> the traditional compare command, because it compiled both stage2 and
> stage3 with options that used to generate identical output
> (-fcompare-debug= in stage2 vs -fcompare-debug in stage3).
> 
> Since we started adding relevant command-line flags to DW_AT_producer,
> this is no longer the case, and stages 2 and 3 object files that differ
> in nothing but the DW_AT_producer strings.
> 
> 
> -fcompare-debug is short for -fcompare-debug=-gtoggle, so stage3
> compiles twice, once with the normal options, once with toggled -g, to
> then compare the temporary final dumps.  When enabled, both compilations
> get from the driver an additional -frandom-seed flag (if none is given
> explicitly).
> 
> -fcompare-debug= is short for -fno-compare-debug, disabling the second
> compilation.
> 
> 
> The difference between the DW_AT_producer lines are the different
> -fcompare-debug flags, and the presence of the -frandom-seed flag in the
> stage3 compilation.
> 
> It should be easy and sensible enough to filter the -fcompare-debug
> flags out of the DW_AT_producer string.  This option should never affect
> the compilation output, it just determines whether or not to perform an
> additional compilation that should produce the same executable output.
> 
> However, doing that but that won't get us rid of the -frandom-seed
> option.  We could drop -frandom-seed from the DW_AT_producer output too,
> but I don't think we should do that when the option is given by the
> user, rather than implicitly introduced by -fcompare-debug.  We could
> introduce an option that causes the subsequent option to be omitted from
> the DW_AT_producer string, and arrange for -fcompare-debug to issue that
> option before the -frandom-seed option it issues.  The problem with this
> approach is that I can't decide whether it's an option prone to abuse,
> or one that can have other legitimate uses.
> 
> 
> Another approach to fix (or rather hide) this failure mode is to allow
> the debug information to differ under bootstrap-debug-lean, by using the
> same compare-debug script we use for bootstrap-debug.  The first
> patchlet below does just that.  The second drops -fcompare-debug from
> DW_AT_producer.  A third (not written yet :-) might somehow deal with
> the -frandom-seed added by -fcompare-debug, and either the first or the
> others would be posted as a single, fully tested patch, once we decide
> how we want to deal with -frandom-seed.
> 
> 
> Thoughts?  Thanks in advance,
I'd support either or both.  Your call.

jeff
Alexandre Oliva Nov. 11, 2017, 12:15 a.m. UTC | #2
On Oct 30, 2017, Jeff Law <law@redhat.com> wrote:

> I'd support either or both.  Your call.

Thanks.  I think I'll go with both, then.

Ok to install as follows? (edited the comments and added a ChangeLog)


Use compare-debug for bootstrap-debug-lean to ignore != DW_AT_producer

Unlike bootstrap-debug, bootstrap-debug-lean used to pass compare using
the traditional compare command, because it compiled both stage2 and
stage3 with options that used to generate identical output
(-fcompare-debug= in stage2 vs -fcompare-debug in stage3).

Since we started adding relevant command-line flags to DW_AT_producer,
this is no longer the case, and stages 2 and 3 object files that differ
in nothing but the DW_AT_producer strings.


-fcompare-debug is short for -fcompare-debug=-gtoggle, so stage3
compiles twice, once with the normal options, once with toggled -g, to
then compare the temporary final dumps.  When enabled, both compilations
get from the driver an additional -frandom-seed flag (if none is given
explicitly).

-fcompare-debug= is short for -fno-compare-debug, disabling the second
compilation.


The difference between the DW_AT_producer lines are the different
-fcompare-debug flags, and the presence of the -frandom-seed flag in the
stage3 compilation.

It is easy and sensible enough to filter the -fcompare-debug flags out
of the DW_AT_producer string.  This option should never affect the
compilation output, it just determines whether or not to perform an
additional compilation that should produce the same executable output.

However, dropping -frandom-seed is not quite correct, in that it might
have other consequences on the compilation.  So, it makes little sense
to make the effort to drop it when it's implicit; for those comparing
compiler output differences, it might even hint at what causes, and
could fix, difficult to explain differences, namely, explicitly
supplying -frandom-seed options.

They don't seem to matter for compiler bootstraps, though, at least
for now, so we can safely refrain from issuing -frandom-seed (or maybe
we already issue it where needed :-), and then, for
bootstrap-debug-lean, use the compare-debug script, that strips out
debug information before comparing the object files.


for  config/ChangeLog

	* bootstrap-debug-lean.mk (do-compare): Use the
	contrib/compare-debug script.

for  gcc/ChangeLog

	* dwarf2out.c (gen_producer_string): Discard
	OPT_fcompare_debug.

---
 config/bootstrap-debug-lean.mk |    1 +
 gcc/dwarf2out.c                |    1 +
 2 files changed, 2 insertions(+)

diff --git a/config/bootstrap-debug-lean.mk b/config/bootstrap-debug-lean.mk
index e215280b09f3..5f2db80687fa 100644
--- a/config/bootstrap-debug-lean.mk
+++ b/config/bootstrap-debug-lean.mk
@@ -9,3 +9,4 @@
 
 STAGE2_CFLAGS += -fcompare-debug=
 STAGE3_CFLAGS += -fcompare-debug
+do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index b8f4e4888f1b..01d90808fd3b 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -23403,6 +23403,7 @@ gen_producer_string (void)
       case OPT_fltrans_output_list_:
       case OPT_fresolution_:
       case OPT_fdebug_prefix_map_:
+      case OPT_fcompare_debug:
 	/* Ignore these.  */
 	continue;
       default:
Richard Biener Nov. 13, 2017, 8:46 a.m. UTC | #3
On Sat, Nov 11, 2017 at 1:15 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Oct 30, 2017, Jeff Law <law@redhat.com> wrote:
>
>> I'd support either or both.  Your call.
>
> Thanks.  I think I'll go with both, then.
>
> Ok to install as follows? (edited the comments and added a ChangeLog)

Ok.

>
> Use compare-debug for bootstrap-debug-lean to ignore != DW_AT_producer
>
> Unlike bootstrap-debug, bootstrap-debug-lean used to pass compare using
> the traditional compare command, because it compiled both stage2 and
> stage3 with options that used to generate identical output
> (-fcompare-debug= in stage2 vs -fcompare-debug in stage3).
>
> Since we started adding relevant command-line flags to DW_AT_producer,
> this is no longer the case, and stages 2 and 3 object files that differ
> in nothing but the DW_AT_producer strings.
>
>
> -fcompare-debug is short for -fcompare-debug=-gtoggle, so stage3
> compiles twice, once with the normal options, once with toggled -g, to
> then compare the temporary final dumps.  When enabled, both compilations
> get from the driver an additional -frandom-seed flag (if none is given
> explicitly).
>
> -fcompare-debug= is short for -fno-compare-debug, disabling the second
> compilation.
>
>
> The difference between the DW_AT_producer lines are the different
> -fcompare-debug flags, and the presence of the -frandom-seed flag in the
> stage3 compilation.
>
> It is easy and sensible enough to filter the -fcompare-debug flags out
> of the DW_AT_producer string.  This option should never affect the
> compilation output, it just determines whether or not to perform an
> additional compilation that should produce the same executable output.
>
> However, dropping -frandom-seed is not quite correct, in that it might
> have other consequences on the compilation.  So, it makes little sense
> to make the effort to drop it when it's implicit; for those comparing
> compiler output differences, it might even hint at what causes, and
> could fix, difficult to explain differences, namely, explicitly
> supplying -frandom-seed options.
>
> They don't seem to matter for compiler bootstraps, though, at least
> for now, so we can safely refrain from issuing -frandom-seed (or maybe
> we already issue it where needed :-), and then, for
> bootstrap-debug-lean, use the compare-debug script, that strips out
> debug information before comparing the object files.
>
>
> for  config/ChangeLog
>
>         * bootstrap-debug-lean.mk (do-compare): Use the
>         contrib/compare-debug script.
>
> for  gcc/ChangeLog
>
>         * dwarf2out.c (gen_producer_string): Discard
>         OPT_fcompare_debug.
>
> ---
>  config/bootstrap-debug-lean.mk |    1 +
>  gcc/dwarf2out.c                |    1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/config/bootstrap-debug-lean.mk b/config/bootstrap-debug-lean.mk
> index e215280b09f3..5f2db80687fa 100644
> --- a/config/bootstrap-debug-lean.mk
> +++ b/config/bootstrap-debug-lean.mk
> @@ -9,3 +9,4 @@
>
>  STAGE2_CFLAGS += -fcompare-debug=
>  STAGE3_CFLAGS += -fcompare-debug
> +do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index b8f4e4888f1b..01d90808fd3b 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -23403,6 +23403,7 @@ gen_producer_string (void)
>        case OPT_fltrans_output_list_:
>        case OPT_fresolution_:
>        case OPT_fdebug_prefix_map_:
> +      case OPT_fcompare_debug:
>         /* Ignore these.  */
>         continue;
>        default:
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
diff mbox series

Patch

diff --git a/config/bootstrap-debug-lean.mk b/config/bootstrap-debug-lean.mk
index e215280..5f2db80 100644
--- a/config/bootstrap-debug-lean.mk
+++ b/config/bootstrap-debug-lean.mk
@@ -9,3 +9,4 @@ 
 
 STAGE2_CFLAGS += -fcompare-debug=
 STAGE3_CFLAGS += -fcompare-debug
+do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2


diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index ed7a85a..f077b35 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -23846,6 +23846,7 @@  gen_producer_string (void)
       case OPT_fltrans_output_list_:
       case OPT_fresolution_:
       case OPT_fdebug_prefix_map_:
+      case OPT_fcompare_debug:
 	/* Ignore these.  */
 	continue;
       default: