Allow target to emit LTO early debug to a separate LTO file.

Hi Tom, Richi,

This is something I was experimenting with to try and solve platform problems with early debug.

Not a “finished patch” (I’ve removed the Darwin back-end parts) but would like your comments on the central idea.

This is to switch to the alternate LTO file (this process already exists for the actual LTO output) before the early debug is started and switch back to the regular output file after.  Therefore both the LTO early debug and the LTO streamed data end up in a separate file (this can be concatenated as we do now, guaranteeing that it appears after any referenced symbols, or could be handled in “some other way” if that was a useful solution).

Now the second part of this delays the output of the .file directives until the “regular” output of the asm (it could be that this could be simplified now there there’s a start/end function pair).

The idea is that the main output text is identical with/without the early debug (and, in fact, it’s broken without this change - since the .file directives would end up in the separate LTO stream).

thoughts?
Iain

On Wed, Aug 22, 2018 at 10:09 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi Tom, Richi,
>
> This is something I was experimenting with to try and solve platform problems with early debug.
>
> Not a “finished patch” (I’ve removed the Darwin back-end parts) but would like your comments on the central idea.
>
> This is to switch to the alternate LTO file (this process already exists for the actual LTO output) before the early debug is started and switch back to the regular output file after.  Therefore both the LTO early debug and the LTO streamed data end up in a separate file (this can be concatenated as we do now, guaranteeing that it appears after any referenced symbols, or could be handled in “some other way” if that was a useful solution).
>
> Now the second part of this delays the output of the .file directives until the “regular” output of the asm (it could be that this could be simplified now there there’s a start/end function pair).
>
> The idea is that the main output text is identical with/without the early debug (and, in fact, it’s broken without this change - since the .file directives would end up in the separate LTO stream).
>
> thoughts?

Hmm, I wonder how we build the file table for the LTO early debug
when output_asm_line_debug_info () is true.  I guess that's
completely broken right now?  But IIRC I verified it "works" ...

Re-checking it looks like for early LTO debug we're not relying
on the assembler somehow but for the FAT part we do.

Otherwise the patch makes sense for those targets that need
to use that separate temporary file thing.

Note I think it would be cleaner to stick that lto_start/end around
the block actually emitting the debug info in dwarf2out_early_finish.

I'd also initialize delay_emit_file to true and only clear it in
dwarf2out_early_finish.  Or clear it in dwarf2out_finish only?

Richard.

> Iain
>
>
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index ec490d75bd..1a7db6c353 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -2777,11 +2777,14 @@ symbol_table::finalize_compilation_unit (void)
>        FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (cnode)
>         (*debug_hooks->early_global_decl) (cnode->decl);
>
> +     targetm.asm_out.lto_start ();
>        /* Clean up anything that needs cleaning up after initial debug
>          generation.  */
>        debuginfo_early_start ();
>        (*debug_hooks->early_finish) (main_input_filename);
> +
>        debuginfo_early_stop ();
> +      targetm.asm_out.lto_end ();
>      }
>
>    /* Finally drive the pass manager.  */
>
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 74a5926524..5f166b7f42 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -3600,6 +3600,9 @@ static GTY(()) unsigned int poc_label_num;
>  /* The last file entry emitted by maybe_emit_file().  */
>  static GTY(()) struct dwarf_file_data * last_emitted_file;
>
> +/* Don't emit the .file directives for early debug.  */
> +static bool delay_emit_file = false;
> +
>  /* Number of internal labels generated by gen_internal_sym().  */
>  static GTY(()) int label_num;
>
> @@ -26939,7 +26942,7 @@ lookup_filename (const char *file_name)
>  static int
>  maybe_emit_file (struct dwarf_file_data * fd)
>  {
> -  if (! fd->emitted_number)
> +  if (! fd->emitted_number && ! delay_emit_file)
>      {
>        if (last_emitted_file)
>         fd->emitted_number = last_emitted_file->emitted_number + 1;
> @@ -31866,6 +31869,7 @@ dwarf2out_early_finish (const char *filename)
>    set_early_dwarf s;
>    char dl_section_ref[MAX_ARTIFICIAL_LABEL_BYTES];
>
> +  delay_emit_file = true;
>    /* PCH might result in DW_AT_producer string being restored from the
>       header compilation, so always fill it with empty string initially
>       and overwrite only here.  */
> @@ -31917,6 +31921,7 @@ dwarf2out_early_finish (const char *filename)
>           fprintf (dump_file, "LTO EARLY DWARF for %s\n", filename);
>           print_die (comp_unit_die (), dump_file);
>         }
> +      delay_emit_file = false;
>        return;
>      }
>
> @@ -32006,7 +32011,11 @@ dwarf2out_early_finish (const char *filename)
>          copy_lto_debug_sections operation of the simple object support in
>          libiberty is not implemented for them yet.  */
>        || TARGET_PECOFF || TARGET_COFF)
> -    return;
> +      || TARGET_PECOFF)
> +    {
> +      delay_emit_file = false;
> +      return;
> +    }
>
>    /* Now as we are going to output for LTO initialize sections and labels
>       to the LTO variants.  We don't need a random-seed postfix as other
> @@ -32128,6 +32137,8 @@ dwarf2out_early_finish (const char *filename)
>                                     output_indirect_string> (form);
>      }
>
> +  delay_emit_file = false;
> +
>    /* Switch back to the text section.  */
>    switch_to_section (text_section);
>  }
> --
> 2.17.1
>
>
>

On 08/22/2018 10:09 PM, Iain Sandoe wrote:
> Hi Tom, Richi,
> 
> This is something I was experimenting with to try and solve platform problems with early debug.
> 
> Not a “finished patch” 

Hmm, not a building patch either.

> (I’ve removed the Darwin back-end parts) but would like your comments on the central idea.
> 
> This is to switch to the alternate LTO file (this process already exists for the actual LTO output)

It took me a while to realize that you're talking about darwin here,
specifically the lto_start/end hooks.

> before the early debug is started and switch back to the regular output file after.  Therefore both the LTO early debug and the LTO streamed data end up in a separate file (this can be concatenated as we do now, guaranteeing that it appears after any referenced symbols, or could be handled in “some other way” if that was a useful solution).
> 
> Now the second part of this delays the output of the .file directives until the “regular” output of the asm (it could be that this could be simplified now there there’s a start/end function pair).
> 
> The idea is that the main output text is identical with/without the early debug (and, in fact, it’s broken without this change - since the .file directives would end up in the separate LTO stream).
> 
> thoughts?
> Iain
> 

I found it hard to understand the above with something concrete to look
at. So I've written attach patch (targeted for my regular x86_64 linux
development platform) that adds comments in the assembly when
transitioning from one generation phase to another.

So, the effect of the patch on vla-1.c -flto -g is:
...
$ diff -I '\.section' -I '\.byte' -u 1 2
--- 1   2018-08-23 12:24:37.426659159 +0200
+++ 2   2018-08-23 12:26:46.886658665 +0200
@@ -1,6 +1,6 @@
        .file   "vla-1.c"
+# LTO_START
 # DWARF2OUT_EARLY_FINISH START
-       .file 1
"/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
        .section        .gnu.debuglto_.debug_info,"e",@progbits
 .Ldebug_info0:
        .hidden vla_1.c.1d3f9cc3
@@ -330,15 +330,8 @@
        .byte   0
        .byte   0
        .byte   0x1
-       .ascii
"/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/gual"
-       .ascii  "ity"
        .byte   0
        .byte   0
-       .string "vla-1.c"
-       .uleb128 0x1
-       .uleb128 0
-       .uleb128 0
-       .byte   0
 .LELTP0:
 .LELT0:
        .section        .gnu.debuglto_.debug_str,"eMS",@progbits,1
@@ -358,8 +351,9 @@
        .string
"/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
        .text
 # DWARF2OUT_EARLY_FINISH END
+# LTO_END
 # LTO_START
-       .section        .gnu.lto_.profile.b9a985e7cc2d09b4,"e",@progbits
+       .section        .gnu.lto_.profile.922c9ad53427823c,"e",@progbits
        .string "x\234\343````\004b\006"
        .string ""
        .string "V"
@@ -635,6 +629,7 @@
        .type   bar, @function
 bar:
 .LFB0:
+       .file 1
"/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
        .loc 1 7 1
        .cfi_startproc
 .LVL0:
...

So, emitting the dwarf2 .file ALAP looks ok to me, and I think that can
be a separate patch. I wonder though if you really need a separate state
variable delay_emit_file to track this, and if you can't use
early_dwarf/early_dwarf_finished.

As for adding lto_start/end around dwarf2out_early_finish, the lto_start
hook is defined as:
...
Output to asm_out_file any text which the assembler expects to find at
the start of an LTO section.
...

Looking at the current implementation of this hook, adding this new call
pair shouldn't hurt, but perhaps we want to clarify in the documentation
that the "LTO section" can also be a debug LTO section?

Thanks,
- Tom
Add comments in .s file describing generation phases

---
 gcc/dwarf2out.c | 13 ++++++++++++-
 gcc/hooks.c     | 15 +++++++++++++++
 gcc/hooks.h     |  2 ++
 gcc/target.def  |  4 ++--
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index fb71ff349fa..334f9a3a901 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -31114,6 +31114,7 @@ reset_dies (dw_die_ref die)
 static void
 dwarf2out_finish (const char *filename)
 {
+  fprintf (asm_out_file, "%s DWARF2OUT_FINISH START\n", ASM_COMMENT_START);
   comdat_type_node *ctnode;
   dw_die_ref main_comp_unit_die;
   unsigned char checksum[16];
@@ -31565,6 +31566,7 @@ dwarf2out_finish (const char *filename)
   symview_upper_bound = 0;
   if (zero_view_p)
     bitmap_clear (zero_view_p);
+  fprintf (asm_out_file, "%s DWARF2OUT_FINISH END\n", ASM_COMMENT_START);
 }
 
 /* Returns a hash value for X (which really is a variable_value_struct).  */
@@ -31845,6 +31847,8 @@ note_variable_value (dw_die_ref die)
 static void
 dwarf2out_early_finish (const char *filename)
 {
+  fprintf (asm_out_file, "%s DWARF2OUT_EARLY_FINISH START\n",
+	   ASM_COMMENT_START);
   set_early_dwarf s;
   char dl_section_ref[MAX_ARTIFICIAL_LABEL_BYTES];
 
@@ -31899,6 +31903,8 @@ dwarf2out_early_finish (const char *filename)
 	  fprintf (dump_file, "LTO EARLY DWARF for %s\n", filename);
 	  print_die (comp_unit_die (), dump_file);
 	}
+      fprintf (asm_out_file, "%s DWARF2OUT_EARLY_FINISH END\n",
+	       ASM_COMMENT_START);
       return;
     }
 
@@ -31988,7 +31994,11 @@ dwarf2out_early_finish (const char *filename)
 	 copy_lto_debug_sections operation of the simple object support in
 	 libiberty is not implemented for them yet.  */
       || TARGET_PECOFF || TARGET_COFF)
-    return;
+    {
+      fprintf (asm_out_file, "%s DWARF2OUT_EARLY_FINISH END\n",
+	       ASM_COMMENT_START);
+      return;
+    }
 
   /* Now as we are going to output for LTO initialize sections and labels
      to the LTO variants.  We don't need a random-seed postfix as other
@@ -32112,6 +32122,7 @@ dwarf2out_early_finish (const char *filename)
 
   /* Switch back to the text section.  */
   switch_to_section (text_section);
+  fprintf (asm_out_file, "%s DWARF2OUT_EARLY_FINISH END\n", ASM_COMMENT_START);
 }
 
 /* Reset all state within dwarf2out.c so that we can rerun the compiler
diff --git a/gcc/hooks.c b/gcc/hooks.c
index 780cc1e0863..47b72c3d3a9 100644
--- a/gcc/hooks.c
+++ b/gcc/hooks.c
@@ -27,6 +27,7 @@
 #include "coretypes.h"
 #include "tm.h"
 #include "hooks.h"
+#include "output.h"
 
 /* Generic hook that does absolutely zappo.  */
 void
@@ -539,3 +540,17 @@ hook_optmode_mode_uhwi_none (machine_mode, unsigned HOST_WIDE_INT)
 {
   return opt_machine_mode ();
 }
+
+#pragma weak asm_out_file
+
+void
+default_lto_start (void)
+{
+  fprintf (asm_out_file, "%s LTO_START\n", ASM_COMMENT_START);
+}
+
+void
+default_lto_end (void)
+{
+  fprintf (asm_out_file, "%s LTO_END\n", ASM_COMMENT_START);
+}
diff --git a/gcc/hooks.h b/gcc/hooks.h
index 0ed5b952b48..9f78e4b204a 100644
--- a/gcc/hooks.h
+++ b/gcc/hooks.h
@@ -128,4 +128,6 @@ extern const char *hook_constcharptr_int_const_tree_const_tree_null (int, const_
 
 extern opt_machine_mode hook_optmode_mode_uhwi_none (machine_mode,
 						     unsigned HOST_WIDE_INT);
+extern void default_lto_end (void);
+extern void default_lto_start (void);
 #endif
diff --git a/gcc/target.def b/gcc/target.def
index c570f3825a5..1afc4844674 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -679,7 +679,7 @@ DEFHOOK
 to find at the start of an LTO section.  The default is to output\n\
 nothing.",
  void, (void),
- hook_void_void)
+ default_lto_start)
 
 /* Output any boilerplate text needed at the end of an
    LTO output stream.  */
@@ -689,7 +689,7 @@ DEFHOOK
 to find at the end of an LTO section.  The default is to output\n\
 nothing.",
  void, (void),
- hook_void_void)
+ default_lto_end)
 
 /* Output any boilerplace text needed at the end of a
    translation unit before debug and unwind info is emitted.  */

On Thu, Aug 23, 2018 at 1:19 PM Tom de Vries <tdevries@suse.de> wrote:
>
> On 08/22/2018 10:09 PM, Iain Sandoe wrote:
> > Hi Tom, Richi,
> >
> > This is something I was experimenting with to try and solve platform problems with early debug.
> >
> > Not a “finished patch”
>
> Hmm, not a building patch either.
>
> > (I’ve removed the Darwin back-end parts) but would like your comments on the central idea.
> >
> > This is to switch to the alternate LTO file (this process already exists for the actual LTO output)
>
> It took me a while to realize that you're talking about darwin here,
> specifically the lto_start/end hooks.
>
> > before the early debug is started and switch back to the regular output file after.  Therefore both the LTO early debug and the LTO streamed data end up in a separate file (this can be concatenated as we do now, guaranteeing that it appears after any referenced symbols, or could be handled in “some other way” if that was a useful solution).
> >
> > Now the second part of this delays the output of the .file directives until the “regular” output of the asm (it could be that this could be simplified now there there’s a start/end function pair).
> >
> > The idea is that the main output text is identical with/without the early debug (and, in fact, it’s broken without this change - since the .file directives would end up in the separate LTO stream).
> >
> > thoughts?
> > Iain
> >
>
> I found it hard to understand the above with something concrete to look
> at. So I've written attach patch (targeted for my regular x86_64 linux
> development platform) that adds comments in the assembly when
> transitioning from one generation phase to another.
>
> So, the effect of the patch on vla-1.c -flto -g is:
> ...
> $ diff -I '\.section' -I '\.byte' -u 1 2
> --- 1   2018-08-23 12:24:37.426659159 +0200
> +++ 2   2018-08-23 12:26:46.886658665 +0200
> @@ -1,6 +1,6 @@
>         .file   "vla-1.c"
> +# LTO_START
>  # DWARF2OUT_EARLY_FINISH START
> -       .file 1
> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
>         .section        .gnu.debuglto_.debug_info,"e",@progbits
>  .Ldebug_info0:
>         .hidden vla_1.c.1d3f9cc3
> @@ -330,15 +330,8 @@
>         .byte   0
>         .byte   0
>         .byte   0x1
> -       .ascii
> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/gual"
> -       .ascii  "ity"
>         .byte   0
>         .byte   0
> -       .string "vla-1.c"
> -       .uleb128 0x1
> -       .uleb128 0
> -       .uleb128 0
> -       .byte   0
>  .LELTP0:
>  .LELT0:
>         .section        .gnu.debuglto_.debug_str,"eMS",@progbits,1
> @@ -358,8 +351,9 @@
>         .string
> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
>         .text
>  # DWARF2OUT_EARLY_FINISH END
> +# LTO_END
>  # LTO_START
> -       .section        .gnu.lto_.profile.b9a985e7cc2d09b4,"e",@progbits
> +       .section        .gnu.lto_.profile.922c9ad53427823c,"e",@progbits
>         .string "x\234\343````\004b\006"
>         .string ""
>         .string "V"
> @@ -635,6 +629,7 @@
>         .type   bar, @function
>  bar:
>  .LFB0:
> +       .file 1
> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
>         .loc 1 7 1
>         .cfi_startproc
>  .LVL0:
> ...
>
> So, emitting the dwarf2 .file ALAP looks ok to me, and I think that can
> be a separate patch. I wonder though if you really need a separate state
> variable delay_emit_file to track this, and if you can't use
> early_dwarf/early_dwarf_finished.
>
> As for adding lto_start/end around dwarf2out_early_finish, the lto_start
> hook is defined as:
> ...
> Output to asm_out_file any text which the assembler expects to find at
> the start of an LTO section.
> ...
>
> Looking at the current implementation of this hook, adding this new call
> pair shouldn't hurt, but perhaps we want to clarify in the documentation
> that the "LTO section" can also be a debug LTO section?

I think we should adjust the documentation since they now called multiple
times, bracketing LTO section output.  So targets implementing the hook
would need to take care of keeping track of whether it's the first call
to lto_begin.

Richard.

>
> Thanks,
> - Tom

> On 23 Aug 2018, at 12:37, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Thu, Aug 23, 2018 at 1:19 PM Tom de Vries <tdevries@suse.de> wrote:
>> 
>> On 08/22/2018 10:09 PM, Iain Sandoe wrote:
>>> Hi Tom, Richi,
>>> 
>>> This is something I was experimenting with to try and solve platform problems with early debug.
>>> 
>>> Not a “finished patch”
>> 
>> Hmm, not a building patch either.
>> 
>>> (I’ve removed the Darwin back-end parts) but would like your comments on the central idea.
>>> 
>>> This is to switch to the alternate LTO file (this process already exists for the actual LTO output)
>> 
>> It took me a while to realize that you're talking about darwin here,
>> specifically the lto_start/end hooks.
>> 
>>> before the early debug is started and switch back to the regular output file after.  Therefore both the LTO early debug and the LTO streamed data end up in a separate file (this can be concatenated as we do now, guaranteeing that it appears after any referenced symbols, or could be handled in “some other way” if that was a useful solution).
>>> 
>>> Now the second part of this delays the output of the .file directives until the “regular” output of the asm (it could be that this could be simplified now there there’s a start/end function pair).
>>> 
>>> The idea is that the main output text is identical with/without the early debug (and, in fact, it’s broken without this change - since the .file directives would end up in the separate LTO stream).
>>> 
>>> thoughts?
>>> Iain
>>> 
>> 
>> I found it hard to understand the above with something concrete to look
>> at. So I've written attach patch (targeted for my regular x86_64 linux
>> development platform) that adds comments in the assembly when
>> transitioning from one generation phase to another.
>> 
>> So, the effect of the patch on vla-1.c -flto -g is:
>> ...
>> $ diff -I '\.section' -I '\.byte' -u 1 2
>> --- 1   2018-08-23 12:24:37.426659159 +0200
>> +++ 2   2018-08-23 12:26:46.886658665 +0200
>> @@ -1,6 +1,6 @@
>>        .file   "vla-1.c"
>> +# LTO_START
>> # DWARF2OUT_EARLY_FINISH START
>> -       .file 1
>> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
>>        .section        .gnu.debuglto_.debug_info,"e",@progbits
>> .Ldebug_info0:
>>        .hidden vla_1.c.1d3f9cc3
>> @@ -330,15 +330,8 @@
>>        .byte   0
>>        .byte   0
>>        .byte   0x1
>> -       .ascii
>> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/gual"
>> -       .ascii  "ity"
>>        .byte   0
>>        .byte   0
>> -       .string "vla-1.c"
>> -       .uleb128 0x1
>> -       .uleb128 0
>> -       .uleb128 0
>> -       .byte   0
>> .LELTP0:
>> .LELT0:
>>        .section        .gnu.debuglto_.debug_str,"eMS",@progbits,1
>> @@ -358,8 +351,9 @@
>>        .string
>> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
>>        .text
>> # DWARF2OUT_EARLY_FINISH END
>> +# LTO_END
>> # LTO_START
>> -       .section        .gnu.lto_.profile.b9a985e7cc2d09b4,"e",@progbits
>> +       .section        .gnu.lto_.profile.922c9ad53427823c,"e",@progbits
>>        .string "x\234\343````\004b\006"
>>        .string ""
>>        .string "V"
>> @@ -635,6 +629,7 @@
>>        .type   bar, @function
>> bar:
>> .LFB0:
>> +       .file 1
>> "/home/vries/gcc_versions/devel/src/gcc/testsuite/gcc.dg/guality/vla-1.c"
>>        .loc 1 7 1
>>        .cfi_startproc
>> .LVL0:
>> ...
>> 
>> So, emitting the dwarf2 .file ALAP looks ok to me, and I think that can
>> be a separate patch. I wonder though if you really need a separate state
>> variable delay_emit_file to track this, and if you can't use
>> early_dwarf/early_dwarf_finished.
>> 
>> As for adding lto_start/end around dwarf2out_early_finish, the lto_start
>> hook is defined as:
>> ...
>> Output to asm_out_file any text which the assembler expects to find at
>> the start of an LTO section.
>> ...
>> 
>> Looking at the current implementation of this hook, adding this new call
>> pair shouldn't hurt, but perhaps we want to clarify in the documentation
>> that the "LTO section" can also be a debug LTO section?
> 
> I think we should adjust the documentation since they now called multiple
> times, bracketing LTO section output.  So targets implementing the hook
> would need to take care of keeping track of whether it's the first call
> to lto_begin.

Thanks for looking at this, apologies my question was phrased badly
 (I was trying to make sure that it was conceptually sound, i.e that the
FAT portion of the asm output could be handled distinctly from the LTO).

It seemed to make sense, since the LTO stuff can stand alone.

This is still experimental; it’s not clear if it will provide a mechanism to 
solve the problems, but your suggestions are noted and I’ll incorporate
in my next update.

thanks
Iain