[LVU] use asm .loc and get gas to compute views on the side

I was shocked when you, richi, reported huge debug info growth after the
LVU and IEPM patches went in.  At first, I suspected IEPM, due to
possibly keeping more lexical blocks around, but my investigation showed
a lot of the growth was actually due to switching from asm-generated
line number sections to gcc-generated ones, which we do when locviews
are requested but the assembler has no support for view assignments in
.loc directives.

It then occurred to me that we could compute view numbers on the side,
and I jumped at it before recalling there were complications I was
already familiar with, namely that .loc directives (without views) don't
necessarily bind to '.': they bind to the next instruction, which might
even be in a different section.  This makes for some complications,
verbosely described in comments in dwarf2out.c.  I'm reasonably happy
with the result, that passed visual inspection on some reasonably tricky
cases, but there's unfortunately no way I can think of to test
mechanically that the views we get from gas-generated line number
programs correspond to the views we compute from the labels introduced
by the patch.  Although we might get view numbers wrong in some unknown
cases, that we'll only notice for real once we have loc view support in
debuggers, since actual insns are sync (reset) points for views, odds
are they will be in sync at most points of interest.

So, do you think the far more compact debug_line sections are worth the
risk of some out-of-sync view counts, for those who already use gas,
just not 2.30?  Or should we instead default to disabling views when the
assembler doesn't have native support for them, as you had suggested?

(I haven't looked yet into the other debug_info growth, most certainly
caused by IEPM-preserved lexical blocks; that's next up, but I'll need
another sleep cycle first, I'm afraid)

WDYT?


introduce asm-fallback view computation mode


---
 gcc/config.in    |    6 +
 gcc/configure    |   55 ++++++++++-
 gcc/configure.ac |   23 ++++
 gcc/dwarf2out.c  |  282 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 gcc/final.c      |   15 +++
 5 files changed, 360 insertions(+), 21 deletions(-)

On February 10, 2018 1:53:41 PM GMT+01:00, Alexandre Oliva <aoliva@redhat.com> wrote:
>I was shocked when you, richi, reported huge debug info growth after
>the
>LVU and IEPM patches went in.  At first, I suspected IEPM, due to
>possibly keeping more lexical blocks around, but my investigation
>showed
>a lot of the growth was actually due to switching from asm-generated
>line number sections to gcc-generated ones, which we do when locviews
>are requested but the assembler has no support for view assignments in
>.loc directives.
>
>It then occurred to me that we could compute view numbers on the side,
>and I jumped at it before recalling there were complications I was
>already familiar with, namely that .loc directives (without views)
>don't
>necessarily bind to '.': they bind to the next instruction, which might
>even be in a different section.  This makes for some complications,
>verbosely described in comments in dwarf2out.c.  I'm reasonably happy
>with the result, that passed visual inspection on some reasonably
>tricky
>cases, but there's unfortunately no way I can think of to test
>mechanically that the views we get from gas-generated line number
>programs correspond to the views we compute from the labels introduced
>by the patch.  Although we might get view numbers wrong in some unknown
>cases, that we'll only notice for real once we have loc view support in
>debuggers, since actual insns are sync (reset) points for views, odds
>are they will be in sync at most points of interest.
>
>So, do you think the far more compact debug_line sections are worth the
>risk of some out-of-sync view counts, for those who already use gas,
>just not 2.30?  Or should we instead default to disabling views when
>the
>assembler doesn't have native support for them, as you had suggested?
>
>(I haven't looked yet into the other debug_info growth, most certainly
>caused by IEPM-preserved lexical blocks; that's next up, but I'll need
>another sleep cycle first, I'm afraid)
>
>WDYT?

I think it makes sense to disable them by default if gas doesn't support them with that ability to override the auto detection by a configure switch (get the GCC generated ones). The patch looks too heavy for this stage and as you say we lack support on the consumer side anyways. We can revisit this patch for GCC 9 or simply document the recommendation of using gas 2.30 or newer. 

Richard. 
>
>
>introduce asm-fallback view computation mode
>
>
>---
> gcc/config.in    |    6 +
> gcc/configure    |   55 ++++++++++-
> gcc/configure.ac |   23 ++++
>gcc/dwarf2out.c  |  282
>++++++++++++++++++++++++++++++++++++++++++++++++++----
> gcc/final.c      |   15 +++
> 5 files changed, 360 insertions(+), 21 deletions(-)
>
>diff --git a/gcc/config.in b/gcc/config.in
>index 5bccb408016b..f31f79763fb4 100644
>--- a/gcc/config.in
>+++ b/gcc/config.in
>@@ -315,6 +315,12 @@
> #endif
> 
> 
>+/* Define if your assembler supports computing views from labels. */
>+#ifndef USED_FOR_TARGET
>+#undef HAVE_AS_COMPUTED_DEBUG_VIEW
>+#endif
>+
>+
> /* Define if your assembler supports the DCI/ICI instructions. */
> #ifndef USED_FOR_TARGET
> #undef HAVE_AS_DCI
>diff --git a/gcc/configure b/gcc/configure
>index b12628725b67..758737daf296 100755
>--- a/gcc/configure
>+++ b/gcc/configure
>@@ -27843,7 +27843,7 @@ else
>   gcc_cv_as_dwarf2_debug_view=no
>     if test $in_tree_gas = yes; then
>     if test $in_tree_gas_is_elf = yes \
>-  && test $gcc_cv_gas_vers -ge `expr \( \( 2 \* 1000 \) + 27 \) \*
>1000 + 0`
>+  && test $gcc_cv_gas_vers -ge `expr \( \( 2 \* 1000 \) + 30 \) \*
>1000 + 0`
>   then gcc_cv_as_dwarf2_debug_view=yes
> fi
>   elif test x$gcc_cv_as != x; then
>@@ -27870,6 +27870,59 @@ if test $gcc_cv_as_dwarf2_debug_view = yes;
>then
> $as_echo "#define HAVE_AS_DWARF2_DEBUG_VIEW 1" >>confdefs.h
> 
> fi
>+
>+
>+	conftest_s="\
>+	.data
>+	.LVUL0:
>+	.set .LVU0, 0
>+	.LVUL1:
>+	.set .LVU1, (.LVU0 + 1) * !(.LVUL1 > .LVUL0)
>+	.space 1
>+	.LVUL2:
>+	.set .LVU2, (.LVU1 + 1) * !(.LVUL2 > .LVUL1)
>+
>+	.uleb128 .LVU0
>+	.uleb128 .LVU1
>+	.uleb128 .LVU2
>+"
>+	{ $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
>dwarf2 debug_view support fallback" >&5
>+$as_echo_n "checking assembler for dwarf2 debug_view support
>fallback... " >&6; }
>+if test "${gcc_cv_as_dwarf2_debug_view_fallback+set}" = set; then :
>+  $as_echo_n "(cached) " >&6
>+else
>+  gcc_cv_as_dwarf2_debug_view_fallback=no
>+    if test $in_tree_gas = yes; then
>+    if test $in_tree_gas_is_elf = yes \
>+  && test $gcc_cv_gas_vers -ge `expr \( \( 2 \* 1000 \) + 30 \) \*
>1000 + 0`
>+  then gcc_cv_as_dwarf2_debug_view_fallback=yes
>+fi
>+  elif test x$gcc_cv_as != x; then
>+    $as_echo "$conftest_s" > conftest.s
>+    if { ac_try='$gcc_cv_as $gcc_cv_as_flags  -o conftest.o conftest.s
>>&5'
>+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
>+  (eval $ac_try) 2>&5
>+  ac_status=$?
>+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
>+  test $ac_status = 0; }; }
>+    then
>+	gcc_cv_as_dwarf2_debug_view_fallback=yes
>+    else
>+      echo "configure: failed program was" >&5
>+      cat conftest.s >&5
>+    fi
>+    rm -f conftest.o conftest.s
>+  fi
>+fi
>+{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
>$gcc_cv_as_dwarf2_debug_view_fallback" >&5
>+$as_echo "$gcc_cv_as_dwarf2_debug_view_fallback" >&6; }
>+if test $gcc_cv_as_dwarf2_debug_view_fallback = yes; then
>+
>+$as_echo "#define HAVE_AS_COMPUTED_DEBUG_VIEW 1" >>confdefs.h
>+
>+fi
>+
>+
>     fi
>  fi
> 
>diff --git a/gcc/configure.ac b/gcc/configure.ac
>index 140c804412a7..4a6b76a83402 100644
>--- a/gcc/configure.ac
>+++ b/gcc/configure.ac
>@@ -4917,9 +4917,30 @@ if test x"$insn" != x; then
> "
> 	gcc_GAS_CHECK_FEATURE([dwarf2 debug_view support],
> 	  gcc_cv_as_dwarf2_debug_view,
>-	  [elf,2,27,0],,[$conftest_s],,
>+	  [elf,2,30,0],,[$conftest_s],,
> 	  [AC_DEFINE(HAVE_AS_DWARF2_DEBUG_VIEW, 1,
>[Define if your assembler supports views in dwarf2 .loc directives.])])
>+
>+	conftest_s="\
>+	.data
>+	.LVUL0:
>+	.set .LVU0, 0
>+	.LVUL1:
>+	.set .LVU1, (.LVU0 + 1) * !(.LVUL1 > .LVUL0)
>+	.space 1
>+	.LVUL2:
>+	.set .LVU2, (.LVU1 + 1) * !(.LVUL2 > .LVUL1)
>+
>+	.uleb128 .LVU0
>+	.uleb128 .LVU1
>+	.uleb128 .LVU2
>+"
>+	gcc_GAS_CHECK_FEATURE([dwarf2 debug_view support fallback],
>+	  gcc_cv_as_dwarf2_debug_view_fallback,
>+	  [elf,2,30,0],,[$conftest_s],,
>+	  [AC_DEFINE(HAVE_AS_COMPUTED_DEBUG_VIEW, 1,
>+  [Define if your assembler supports computing views from labels.])])
>+
>     fi
>  fi
> 
>diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>index c48c117c8a16..c7bf8b33c093 100644
>--- a/gcc/dwarf2out.c
>+++ b/gcc/dwarf2out.c
>@@ -2941,9 +2941,6 @@ struct GTY(()) dw_line_info_table {
>      If it is 0, it is known that the NEXT view will be the first view
>      at the given PC.
> 
>-     If it is -1, we've advanced PC but we haven't emitted a line
>location yet,
>-     so we shouldn't use this view number.
>-
>      The meaning of other nonzero values depends on whether we're
>      computing views internally or leaving it for the assembler to do
>      so.  If we're emitting them internally, view denotes the view
>@@ -2952,6 +2949,11 @@ struct GTY(()) dw_line_info_table {
>      going to ask the assembler to assign.  */
>   var_loc_view view;
> 
>+  /* This denotes the last emitted view number in the current table.
>+     It is only really used when we're computing view numbers through
>+     the assembler, but without .loc view support.  */
>+  var_loc_view prev_view;
>+
> #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0)
> #define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0)
> 
>@@ -3165,6 +3167,92 @@ skeleton_chain_node;
> #endif
> #endif
> 
>+/* Even if the assembler does not support .loc views, we might still
>+   be able to use it to compute views, using such expressions as:
>+
>+	.LVUL0:
>+	.set .LVU0, 0
>+	.LVUL1:
>+	.set .LVU1, (.LVU0 + 1) * !(.LVUL1 > .LVUL0)
>+	.space 1
>+	.LVUL2:
>+	.set .LVU2, (.LVU1 + 1) * !(.LVUL2 > .LVUL1)
>+
>+   This may works as long as we can be sure the so-computed view
>+   numbers will match the line number program generated by the
>+   assembler.  Unfortunately, it's not as trivial as it might seem,
>+   because, at least with GNU as, a .loc directive (without a view)
>+   binds to the PC of the next *instruction*, even if it is only
>+   emitted long after the .loc directive itself.  Say, there could be
>+   alignment directives, and even section changes, which makes keeping
>+   views in sync a bit more complicated.
>+
>+   On the good side, at least GNU as will also flush a .loc directive
>+   when it sees the next .loc directive.  This provides us with a way
>+   to get .loc directives at the expected PC in the original section:
>+   after a section switch, when we encounter the first .loc in the new
>+   section, we switch back to the earlier section, output the new .loc
>+   to flush the previous one, then switch back.  This will get a
>+   previous unflushed .loc, if any, flushed in the correct section and
>+   address, because it would only be unflushed if it was a trailing
>+   .loc, issued right before the section switch.  If there wasn't any
>+   unflushed .loc, it won't matter, the section switching will be just
>+   a small waste of asm output.
>+
>+   As for alignment requests, we deal with alignment within functions
>+   by emitting a duplicate .loc directive before the alignment
>+   request, so that the previous .loc directive is flushed at the
>+   correct address, and emitting the label associated with this
>+   duplicate at the address after the alignment, because that's the
>+   address it will take in the line number table, so view computation
>+   will be in sync between compiler/assembler and debug info consumer.
>+   See next_locs_for_align for details.
>+
>+   It is be that other assemblers behave in the same way.  If that is
>+   so, we may introduce a target bool that defaults to HAVE_GNU_AS,
>+   and that other targets may then set if their native assemblers
>+   behave similarly.  For now, let's be conservative and fallback to
>+   internal line programs on them.  Ideally, we'd have some means to
>+   test for these assembler properties at configure time, but this
>+   will do for now.  */
>+#ifndef DWARF2_ASM_FALLBACK_VIEW
>+#if defined HAVE_AS_COMPUTED_DEBUG_VIEW && HAVE_GNU_AS
>+#define DWARF2_ASM_FALLBACK_DEBUG_VIEW 1
>+#else
>+#define DWARF2_ASM_FALLBACK_DEBUG_VIEW 0
>+#endif
>+#endif
>+
>+/* These record the table and section of the last .loc directive, when
>+   we're using DWARF2_ASM_FALLBACK_DEBUG_VIEW.  See above.  */
>+static GTY(()) dw_line_info_table *prev_loc_table;
>+static GTY(()) section *prev_loc_section;
>+
>+/* This records the state of processing a label alignment.  When final
>+   sees a CODE_LABEL that requests extra alignment, and location views
>+   are enabled, it will call the var_location hook with the label
>+   once, and then source_line twice, before and after the alignment
>+   directives.
>+
>+   So, when var_location is called with a label (we assume location
>+   views are enabled), we set next_locs_for_align to 2, if we are
>+   outputting viewless .locs and our last .loc was in the current
>+   table/section, or -2 otherwise, indicating the next two calls of
>+   source_line are related with that label.
>+
>+   Within source_line, if next_locs_for_align is negative, we know to
>+   just increment it and ignore the calls until it reaches zero: only
>+   fallback asm view computation needs these calls.
>+
>+.  Now, if it's positive, we decrement it.  The first time around, we
>+   proceed to outputting the .loc and its view computation, but not
>+   its label, that we defer when next_locs_for_align is still nonzero.
>+   Upon the subsequent call, after the align directive,
>+   next_locs_for_align will be 1, so we just decrement it down to zero
>+   and output the label, completing the handling of the alignment
>+   sequence.  */
>+static int next_locs_for_align;
>+
> /* A bit is set in ZERO_VIEW_P if we are using the assembler-supported
>    view computation, and it refers to a view identifier for which we
>    will not emit a label because it is known to map to a view number
>@@ -3217,10 +3305,19 @@ static bool
> output_asm_line_debug_info (void)
> {
>   return (DWARF2_ASM_VIEW_DEBUG_INFO
>+	  || DWARF2_ASM_FALLBACK_DEBUG_VIEW
> 	  || (DWARF2_ASM_LINE_DEBUG_INFO
> 	      && !debug_variable_location_views));
> }
> 
>+/* Return true if we are to use the assembler's .loc view support.  */
>+
>+static bool
>+output_asm_line_view_debug_info (void)
>+{
>+  return DWARF2_ASM_VIEW_DEBUG_INFO;
>+}
>+
> /* Minimum line offset in a special line info. opcode.
>    This value was chosen to give a reasonable range of values.  */
> #define DWARF_LINE_BASE  -10
>@@ -9951,7 +10048,7 @@ dwarf2out_maybe_output_loclist_view_pair
>(dw_loc_list_ref curr)
> #ifdef DW_LLE_view_pair
>   dw2_asm_output_data (1, DW_LLE_view_pair, "DW_LLE_view_pair");
> 
>-# if DWARF2_ASM_VIEW_DEBUG_INFO
>+# if DWARF2_ASM_VIEW_DEBUG_INFO || DWARF2_ASM_FALLBACK_DEBUG_VIEW
>   if (ZERO_VIEW_P (curr->vbegin))
>     dw2_asm_output_data_uleb128 (0, "Location view begin");
>   else
>@@ -10002,7 +10099,7 @@ output_loc_list (dw_loc_list_ref list_head)
> 	  vcount++;
> 
> 	  /* ?? dwarf_split_debug_info?  */
>-#if DWARF2_ASM_VIEW_DEBUG_INFO
>+#if DWARF2_ASM_VIEW_DEBUG_INFO || DWARF2_ASM_FALLBACK_DEBUG_VIEW
> 	  char label[MAX_ARTIFICIAL_LABEL_BYTES];
> 
> 	  if (!ZERO_VIEW_P (curr->vbegin))
>@@ -10028,7 +10125,7 @@ output_loc_list (dw_loc_list_ref list_head)
> 	    dw2_asm_output_data_uleb128 (0,
> 					 "View list end (%s)",
> 					 list_head->vl_symbol);
>-#else /* !DWARF2_ASM_VIEW_DEBUG_INFO */
>+#else /* !DWARF2_ASM_VIEW_DEBUG_INFO || DWARF2_ASM_FALLBACK_DEBUG_VIEW
>*/
> 	  dw2_asm_output_data_uleb128 (curr->vbegin,
> 				       "View list begin (%s)",
> 				       list_head->vl_symbol);
>@@ -26944,6 +27041,16 @@ dwarf2out_var_location (rtx_insn *loc_note)
> 	}
>       else if (!debug_variable_location_views)
> 	gcc_unreachable ();
>+      else if (LABEL_P (loc_note))
>+	{
>+	  if (output_asm_line_debug_info ()
>+	      && !output_asm_line_view_debug_info ()
>+	      && cur_line_info_table && cur_line_info_table == prev_loc_table
>+	      && cur_line_info_table->in_use)
>+	    next_locs_for_align = 2;
>+	  else
>+	    next_locs_for_align = -2;
>+	}
>       else if (JUMP_TABLE_DATA_P (loc_note))
> 	RESET_NEXT_VIEW (cur_line_info_table->view);
>       else if (unreliable_attr_length_p ()
>@@ -27325,6 +27432,7 @@ new_line_info_table (void)
>   table->line_num = 1;
>   table->is_stmt = DWARF_LINE_DEFAULT_IS_STMT_START;
>   RESET_NEXT_VIEW (table->view);
>+  table->prev_view = -1;
> 
>   return table;
> }
>@@ -27374,9 +27482,22 @@ set_cur_line_info_table (section *sec)
>     }
> 
>   if (output_asm_line_debug_info ())
>-    table->is_stmt = (cur_line_info_table
>-		      ? cur_line_info_table->is_stmt
>-		      : DWARF_LINE_DEFAULT_IS_STMT_START);
>+    {
>+      table->is_stmt = (cur_line_info_table
>+			? cur_line_info_table->is_stmt
>+			: DWARF_LINE_DEFAULT_IS_STMT_START);
>+
>+      /* If we're switching sections, we've already output any pending
>+	 .loc views for the section we're leaving.  Since we're
>+	 keeping a global view (label) count, the next view, that was
>+	 assigned to that table then, should be the next view for the
>+	 newly-selected table instead.  When we're computing views
>+	 internally in the compiler, the views numbers are reset and
>+	 incremented on a per-table basis already.  */
>+      if (cur_line_info_table)
>+	table->view = cur_line_info_table->view;
>+    }
>+
>   cur_line_info_table = table;
> }
> 
>@@ -27506,8 +27627,38 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
>   unsigned int file_num;
>   dw_line_info_table *table;
> 
>+  if (next_locs_for_align)
>+    {
>+      /* If we're getting these calls but we're not interested
>+	 (alignment only needs special handling in fallback asm view
>+	 computation mode without a section change), just count them
>+	 down to zero.  */
>+      if (next_locs_for_align < 0)
>+	{
>+	  next_locs_for_align++;
>+	  return;
>+	}
>+
>+      /* Otherwise, count down; if this is the last of the two calls,
>+	 just output the label for the view computation of the
>+	 previous call.  The previous call was before the alignment
>+	 directive, to flush an earlier .loc directive; this one is
>+	 after, where the PC for this unused view will land.  */
>+      next_locs_for_align--;
>+      if (!next_locs_for_align)
>+	{
>+	  ASM_OUTPUT_DEBUG_LABEL (asm_out_file, "LVUL",
>+				  cur_line_info_table->prev_view);
>+	  return;
>+	}
>+    }
>+
>   if (debug_info_level < DINFO_LEVEL_TERSE || line == 0)
>-    return;
>+    {
>+      if (next_locs_for_align > 0)
>+	next_locs_for_align = -next_locs_for_align;
>+      return;
>+    }
> 
>   /* The discriminator column was added in dwarf4.  Simplify the below
>      by simply removing it if we're not supposed to output it.  */
>@@ -27556,6 +27707,16 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
> 
>   if (output_asm_line_debug_info ())
>     {
>+      section *switch_back_to_section = NULL;
>+
>+      if (prev_loc_table && prev_loc_table != table
>+	  && debug_variable_location_views
>+	  && !output_asm_line_view_debug_info ())
>+	{
>+	  switch_back_to_section = current_function_section ();
>+	  switch_to_section (prev_loc_section);
>+	}
>+
>       /* Emit the .loc directive understood by GNU as.  */
>       /* "\t.loc %u %u 0 is_stmt %u discriminator %u",
> 	 file_num, line, is_stmt, discriminator */
>@@ -27577,6 +27738,7 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
> 	  fputs (" discriminator ", asm_out_file);
> 	  fprint_ul (asm_out_file, (unsigned long) discriminator);
> 	}
>+      bool pending_line_break = true;
>       if (debug_variable_location_views)
> 	{
> 	  static var_loc_view lvugid;
>@@ -27586,6 +27748,10 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
> 	      zero_view_p = BITMAP_GGC_ALLOC ();
> 	      bitmap_set_bit (zero_view_p, 0);
> 	    }
>+	  if (!table->in_use)
>+	    RESET_NEXT_VIEW (table->view);
>+	  if (!output_asm_line_view_debug_info () && flag_debug_asm)
>+	    fprintf (asm_out_file, "\n%s ...", ASM_COMMENT_START);
> 	  if (!RESETTING_VIEW_P (table->view))
> 	    {
> 	      /* When we're using the assembler to compute view
>@@ -27598,18 +27764,82 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
> 		 the assembler to check that there was a PC change
> 		 since the previous view, in a way that implicitly
> 		 resets the next view.  */
>-	      fputs (" view ", asm_out_file);
>-	      char label[MAX_ARTIFICIAL_LABEL_BYTES];
>-	      ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view);
>-	      assemble_name (asm_out_file, label);
>+	      if (output_asm_line_view_debug_info () || flag_debug_asm)
>+		{
>+		  fputs (" view ", asm_out_file);
>+		  char label[MAX_ARTIFICIAL_LABEL_BYTES];
>+		  ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view);
>+		  assemble_name (asm_out_file, label);
>+		}
>+	      if (!output_asm_line_view_debug_info ())
>+		{
>+		  putc ('\n', asm_out_file);
>+
>+		  if (switch_back_to_section)
>+		    {
>+		      switch_to_section (switch_back_to_section);
>+		      prev_loc_table = table;
>+		      prev_loc_section = switch_back_to_section;
>+		    }
>+
>+		  /* Skip the label if we're in the first of a pair of
>+		     source_line calls around a alignment request.
>+		     The label will be output in the second call.  */
>+		  if (!next_locs_for_align)
>+		    ASM_OUTPUT_DEBUG_LABEL (asm_out_file, "LVUL", table->view);
>+
>+		  fputs (".set ", asm_out_file);
>+		  char label[MAX_ARTIFICIAL_LABEL_BYTES];
>+		  ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view);
>+		  assemble_name (asm_out_file, label);
>+		  fputs (", ", asm_out_file);
>+		  if (!ZERO_VIEW_P (table->prev_view))
>+		    {
>+		      putc ('(', asm_out_file);
>+		      ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->prev_view);
>+		      assemble_name (asm_out_file, label);
>+		      fputs (" + 1) * ", asm_out_file);
>+		    }
>+		  fputs ("!(", asm_out_file);
>+		  ASM_GENERATE_INTERNAL_LABEL (label, "LVUL", table->view);
>+		  assemble_name (asm_out_file, label);
>+		  fputs (" > ", asm_out_file);
>+		  ASM_GENERATE_INTERNAL_LABEL (label, "LVUL", table->prev_view);
>+		  assemble_name (asm_out_file, label);
>+		  putc (')', asm_out_file);
>+		}
>+	      table->prev_view = table->view;
> 	      table->view = ++lvugid;
> 	    }
> 	  else
> 	    {
>-	      if (!table->in_use)
>-		fputs (" view -0", asm_out_file);
>-	      else
>-		fputs (" view 0", asm_out_file);
>+	      if (output_asm_line_view_debug_info () || flag_debug_asm)
>+		{
>+		  if (!table->in_use)
>+		    fputs (" view -0", asm_out_file);
>+		  else
>+		    fputs (" view 0", asm_out_file);
>+		}
>+
>+	      if (!output_asm_line_view_debug_info ())
>+		{
>+		  putc ('\n', asm_out_file);
>+		  pending_line_break = false;
>+
>+		  if (switch_back_to_section)
>+		    {
>+		      switch_to_section (switch_back_to_section);
>+		      prev_loc_table = table;
>+		      prev_loc_section = switch_back_to_section;
>+		    }
>+
>+		  /* Skip the label if we're in the first of a pair of
>+		     source_line calls around a alignment request.
>+		     The label will be output in the second call.  */
>+		  if (!next_locs_for_align)
>+		    ASM_OUTPUT_DEBUG_LABEL (asm_out_file, "LVUL", lvugid);
>+		}
>+
> 	      /* Mark the present view as a zero view.  Earlier debug
> 		 binds may have already added its id to loclists to be
> 		 emitted later, so we can't reuse the id for something
>@@ -27618,10 +27848,22 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
> 		 optimize out locviews that are all zeros, so take
> 		 note of it in zero_view_p.  */
> 	      bitmap_set_bit (zero_view_p, lvugid);
>+	      table->prev_view = lvugid;
> 	      table->view = ++lvugid;
> 	    }
>+
>+	  /* This test will probably only pass for the first insn.
>+	     Later on, we'll have already recorded table and section
>+	     after switching back, above, so we won't call
>+	     current_function_section over and over.  */
>+	  if (prev_loc_table != table)
>+	    {
>+	      prev_loc_table = table;
>+	      prev_loc_section = current_function_section ();
>+	    }
> 	}
>-      putc ('\n', asm_out_file);
>+      if (pending_line_break)
>+	putc ('\n', asm_out_file);
>     }
>   else
>     {
>@@ -27629,6 +27871,8 @@ dwarf2out_source_line (unsigned int line,
>unsigned int column,
> 
>targetm.asm_out.internal_label (asm_out_file, LINE_CODE_LABEL,
>label_num);
> 
>+      table->prev_view = table->view;
>+
>       if (debug_variable_location_views && table->view)
> 	push_dw_line_info_entry (table, LI_adv_address, label_num);
>       else
>diff --git a/gcc/final.c b/gcc/final.c
>index 99a7cadd7c9f..c311c198da67 100644
>--- a/gcc/final.c
>+++ b/gcc/final.c
>@@ -2521,6 +2521,15 @@ final_scan_insn (rtx_insn *insn, FILE *file, int
>optimize_p ATTRIBUTE_UNUSED,
> 
> 	  if (align && NEXT_INSN (insn))
> 	    {
>+	      if (!DECL_IGNORED_P (current_function_decl)
>+		  && debug_variable_location_views)
>+		{
>+		  (*debug_hooks->var_location) (insn);
>+		  (*debug_hooks->source_line) (last_linenum, last_columnnum,
>+					       last_filename, last_discriminator,
>+					       false);
>+		}
>+
> #ifdef ASM_OUTPUT_MAX_SKIP_ALIGN
> 	      ASM_OUTPUT_MAX_SKIP_ALIGN (file, align, max_skip);
> #else
>@@ -2530,6 +2539,12 @@ final_scan_insn (rtx_insn *insn, FILE *file, int
>optimize_p ATTRIBUTE_UNUSED,
> 	      ASM_OUTPUT_ALIGN (file, align);
> #endif
> #endif
>+
>+	      if (!DECL_IGNORED_P (current_function_decl)
>+		  && debug_variable_location_views)
>+		(*debug_hooks->source_line) (last_linenum, last_columnnum,
>+					     last_filename, last_discriminator,
>+					     false);
> 	    }
> 	}
>       CC_STATUS_INIT;

On 02/10/2018 06:34 AM, Richard Biener wrote:
> On February 10, 2018 1:53:41 PM GMT+01:00, Alexandre Oliva <aoliva@redhat.com> wrote:
>> I was shocked when you, richi, reported huge debug info growth after
>> the
>> LVU and IEPM patches went in.  At first, I suspected IEPM, due to
>> possibly keeping more lexical blocks around, but my investigation
>> showed
>> a lot of the growth was actually due to switching from asm-generated
>> line number sections to gcc-generated ones, which we do when locviews
>> are requested but the assembler has no support for view assignments in
>> .loc directives.
>>
>> It then occurred to me that we could compute view numbers on the side,
>> and I jumped at it before recalling there were complications I was
>> already familiar with, namely that .loc directives (without views)
>> don't
>> necessarily bind to '.': they bind to the next instruction, which might
>> even be in a different section.  This makes for some complications,
>> verbosely described in comments in dwarf2out.c.  I'm reasonably happy
>> with the result, that passed visual inspection on some reasonably
>> tricky
>> cases, but there's unfortunately no way I can think of to test
>> mechanically that the views we get from gas-generated line number
>> programs correspond to the views we compute from the labels introduced
>> by the patch.  Although we might get view numbers wrong in some unknown
>> cases, that we'll only notice for real once we have loc view support in
>> debuggers, since actual insns are sync (reset) points for views, odds
>> are they will be in sync at most points of interest.
>>
>> So, do you think the far more compact debug_line sections are worth the
>> risk of some out-of-sync view counts, for those who already use gas,
>> just not 2.30?  Or should we instead default to disabling views when
>> the
>> assembler doesn't have native support for them, as you had suggested?
>>
>> (I haven't looked yet into the other debug_info growth, most certainly
>> caused by IEPM-preserved lexical blocks; that's next up, but I'll need
>> another sleep cycle first, I'm afraid)
>>
>> WDYT?
> 
> I think it makes sense to disable them by default if gas doesn't support them with that ability to override the auto detection by a configure switch (get the GCC generated ones). The patch looks too heavy for this stage and as you say we lack support on the consumer side anyways. We can revisit this patch for GCC 9 or simply document the recommendation of using gas 2.30 or newer. 
Seems reasonable to me as well.
jeff

On Sat, Feb 10, 2018 at 09:26:47AM -0700, Jeff Law wrote:
> > I think it makes sense to disable them by default if gas doesn't support
> > them with that ability to override the auto detection by a configure
> > switch (get the GCC generated ones).  The patch looks too heavy for this
> > stage and as you say we lack support on the consumer side anyways.  We
> > can revisit this patch for GCC 9 or simply document the recommendation
> > of using gas 2.30 or newer.

> Seems reasonable to me as well.

We should have a switch to force gcc generated .debug_line in any case,
similarly how we have -fno-dwarf2-cfi-asm.
E.g. only the gcc generated .debug_line can do DWARF5 .debug_line right now,
gas generated one can't.

	Jakub