Re: [SFN+LVU+IEPM v4 9/9] [IEPM] Introduce inline entry point markers

[Alexandre Oliva <aoliva@redhat.com>]

On Feb 15, 2018, Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:

> i see assembler slow downs with these location view patches
> i opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84408


[LVU] reset view at function entry, omit views at line zero

Location views might be associated with locations that lack line
number information (line number zero), but since we omit .loc
directives that would have been issued with line number zero, we also
omit the symbolic view numbers that would have been issued at such
points.

Resetting views at function entry points address some of these issues,
and alleviate the huge chains of symbolic views that have burdened
assemblers since we disabled -ginternal-reset-location-views by
default, but other problems of undefined views remain when it's not
the whole function that lacks line number info, just parts of it.

So, when we encounter a request to output a view that may have been
referenced, but we decide to omit the .loc because the line is zero,
we will now omit the view as well, i.e., we will internally regard
that view as zero-numbered.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

Uros, could you please confirm whether this fixes the 84404 go problem
you reported on alpha?  I'm guessing it's the same issue.  TIA,

for  gcc/ChangeLog

	PR debug/84404
	PR debug/84408
	* dwarf2out.c (struct dw_line_info_table): Update comments for
	view == -1.
	(FORCE_RESET_NEXT_VIEW): New.
	(FORCE_RESETTING_VIEW_P): New.
	(RESETTING_VIEW_P): Check for -1 too.
	(ZERO_VIEW_P): Likewise.
	(new_line_info_table): Force-reset next view.
	(dwarf2out_begin_function): Likewise.
	(dwarf2out_source_line): Simplify zero_view_p initialization.
	Test FORCE_RESETTING_VIEW_P and RESETTING_VIEW_P instead of
	view directly.  Omit view when omitting .loc at line 0.

for  gcc/testsuite/ChangeLog

	PR debug/84404
	PR debug/84408
	* gcc.dg/graphite/pr84404.c: New.
---
 gcc/dwarf2out.c                         |   89 ++++++++++++++++++++++++-------
 gcc/testsuite/gcc.dg/graphite/pr84404.c |   18 ++++++
 2 files changed, 87 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/graphite/pr84404.c

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 5e88c7bacf06..7bbe20e495ac 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -2940,8 +2940,8 @@ 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.
+     If it is -1, we're forcing the view number to be reset, e.g. at a
+     function entry.
 
      The meaning of other nonzero values depends on whether we're
      computing views internally or leaving it for the assembler to do
@@ -2951,8 +2951,10 @@ struct GTY(()) dw_line_info_table {
      going to ask the assembler to assign.  */
   var_loc_view view;
 
+#define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1)
 #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0)
-#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0)
+#define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1)
+#define RESETTING_VIEW_P(x) ((x) == (var_loc_view)0 || FORCE_RESETTING_VIEW_P (x))
 
   vec<dw_line_info_entry, va_gc> *entries;
 };
@@ -2985,7 +2987,7 @@ maybe_reset_location_view (rtx_insn *insn, dw_line_info_table *table)
   else if (get_attr_min_length (insn) > 0)
     reset = 1;
 
-  if (reset > 0)
+  if (reset > 0 && !RESETTING_VIEW_P (table->view))
     RESET_NEXT_VIEW (table->view);
 }
 
@@ -3235,9 +3237,10 @@ static GTY(()) bitmap zero_view_p;
    that must be view number zero.  Otherwise, ZERO_VIEW_P is allocated
    and views label numbers recorded in it are the ones known to be
    zero.  */
-#define ZERO_VIEW_P(N) (zero_view_p				\
-			? bitmap_bit_p (zero_view_p, (N))	\
-			: (N) == 0)
+#define ZERO_VIEW_P(N) ((N) == (var_loc_view)0				\
+			|| (N) == (var_loc_view)-1			\
+			|| (zero_view_p					\
+			    && bitmap_bit_p (zero_view_p, (N))))
 
 /* Return true iff we're to emit .loc directives for the assembler to
    generate line number sections.
@@ -27210,6 +27213,18 @@ create_label:
 	  last_postcall_label = ggc_strdup (loclabel);
 	}
       newloc->label = last_postcall_label;
+      /* ??? This view is at last_label, not last_label-1, but we
+	 could only assume view at last_label-1 is zero if we could
+	 assume calls always have length greater than one.  This is
+	 probably true in general, though there might be a rare
+	 exception to this rule, e.g. if a call insn is optimized out
+	 by target magic.  Then, even the -1 in the label will be
+	 wrong, which might invalidate the range.  Anyway, using view,
+	 though technically possibly incorrect, will work as far as
+	 ranges go: since L-1 is in the middle of the call insn,
+	 (L-1).0 and (L-1).V shouldn't make any difference, and having
+	 the loclist entry refer to the .loc entry might be useful, so
+	 leave it like this.  */
       newloc->view = view;
     }
 
@@ -27391,7 +27406,7 @@ new_line_info_table (void)
   table->file_num = 1;
   table->line_num = 1;
   table->is_stmt = DWARF_LINE_DEFAULT_IS_STMT_START;
-  RESET_NEXT_VIEW (table->view);
+  FORCE_RESET_NEXT_VIEW (table->view);
 
   return table;
 }
@@ -27475,6 +27490,7 @@ dwarf2out_begin_function (tree fun)
   tail_call_site_count = 0;
 
   set_cur_line_info_table (sec);
+  FORCE_RESET_NEXT_VIEW (cur_line_info_table->view);
 }
 
 /* Helper function of dwarf2out_end_function, called only after emitting
@@ -27572,10 +27588,44 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 {
   unsigned int file_num;
   dw_line_info_table *table;
+  static var_loc_view lvugid;
 
-  if (debug_info_level < DINFO_LEVEL_TERSE || line == 0)
+  if (debug_info_level < DINFO_LEVEL_TERSE)
     return;
 
+  table = cur_line_info_table;
+
+  if (line == 0)
+    {
+      if (debug_variable_location_views
+	  && output_asm_line_debug_info ()
+	  && table && !RESETTING_VIEW_P (table->view))
+	{
+	  /* If we're using the assembler to compute view numbers, we
+	     can't issue a .loc directive for line zero, so we can't
+	     get a view number at this point.  We might attempt to
+	     compute it from the previous view, but since we're
+	     omitting the line number entry, we might as well omit the
+	     view number as well.  That means pretending it's a view
+	     number zero, which might very well turn out to be
+	     correct.  */
+	  if (!zero_view_p)
+	    zero_view_p = BITMAP_GGC_ALLOC ();
+	  bitmap_set_bit (zero_view_p, table->view);
+	  if (flag_debug_asm)
+	    {
+	      char label[MAX_ARTIFICIAL_LABEL_BYTES];
+	      ASM_GENERATE_INTERNAL_LABEL (label, "LVU", table->view);
+	      fprintf (asm_out_file, "\t%s line 0, omitted view ",
+		       ASM_COMMENT_START);
+	      assemble_name (asm_out_file, label);
+	      putc ('\n', asm_out_file);
+	    }
+	  table->view = ++lvugid;
+	}
+      return;
+    }
+
   /* The discriminator column was added in dwarf4.  Simplify the below
      by simply removing it if we're not supposed to output it.  */
   if (dwarf_version < 4 && dwarf_strict)
@@ -27584,7 +27634,6 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
   if (!debug_column_info)
     column = 0;
 
-  table = cur_line_info_table;
   file_num = maybe_emit_file (lookup_filename (filename));
 
   /* ??? TODO: Elide duplicate line number entries.  Traditionally,
@@ -27646,13 +27695,6 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	}
       if (debug_variable_location_views)
 	{
-	  static var_loc_view lvugid;
-	  if (!lvugid)
-	    {
-	      gcc_assert (!zero_view_p);
-	      zero_view_p = BITMAP_GGC_ALLOC ();
-	      bitmap_set_bit (zero_view_p, 0);
-	    }
 	  if (!RESETTING_VIEW_P (table->view))
 	    {
 	      /* When we're using the assembler to compute view
@@ -27673,7 +27715,7 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	    }
 	  else
 	    {
-	      if (!table->in_use)
+	      if (FORCE_RESETTING_VIEW_P (table->view))
 		fputs (" view -0", asm_out_file);
 	      else
 		fputs (" view 0", asm_out_file);
@@ -27684,6 +27726,8 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 		 known to be zero, because then we may be able to
 		 optimize out locviews that are all zeros, so take
 		 note of it in zero_view_p.  */
+	      if (!zero_view_p)
+		zero_view_p = BITMAP_GGC_ALLOC ();
 	      bitmap_set_bit (zero_view_p, lvugid);
 	      table->view = ++lvugid;
 	    }
@@ -27696,17 +27740,22 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 
       targetm.asm_out.internal_label (asm_out_file, LINE_CODE_LABEL, label_num);
 
-      if (debug_variable_location_views && table->view)
+      if (debug_variable_location_views && !RESETTING_VIEW_P (table->view))
 	push_dw_line_info_entry (table, LI_adv_address, label_num);
       else
 	push_dw_line_info_entry (table, LI_set_address, label_num);
       if (debug_variable_location_views)
 	{
+	  bool resetting = FORCE_RESETTING_VIEW_P (table->view);
+	  if (resetting)
+	    table->view = 0;
+
 	  if (flag_debug_asm)
 	    fprintf (asm_out_file, "\t%s view %s%d\n",
 		     ASM_COMMENT_START,
-		     table->in_use ? "" : "-",
+		     resetting ? "-" : "",
 		     table->view);
+
 	  table->view++;
 	}
       if (file_num != table->file_num)
diff --git a/gcc/testsuite/gcc.dg/graphite/pr84404.c b/gcc/testsuite/gcc.dg/graphite/pr84404.c
new file mode 100644
index 000000000000..858e651cfab7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/pr84404.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-parallelize-loops=2 -floop-nest-optimize -g" } */
+
+int te[9];
+
+void
+dt (int cz)
+{
+  while (cz < 1)
+    {
+      int xy;
+
+      for (xy = 0; xy < 9; ++xy)
+        te[xy] = 0;
+
+      ++cz;
+    }
+}


-- 
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