[PR84620] output symbolic entry_view as data2, not addr

[PR84620] output symbolic entry_view as data2, not addr

[Alexandre Oliva]

Mark Wielaard is implementing support for LVU and IEPM in elfutils, and
he was surprised by the encoding of DW_AT_GNU_entry_view; so was I!
When GCC computes and outputs views internally (broken without internal
view resets), it outputs entry_view attributes as data: it knows the
view numbers.  However, when it uses the assembler to compute the views
symbolically, it has to output the view symbolically.

I'd chosen lbl_id to that end, without giving it much thought, but that
was no good: it's not just space-inefficient, it uses an addr encoding,
indirect in some cases, for something that's not a real label, but
rather a a small assembler data constant.  Oops.

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


[IEPM] [PR debug/84620] use constant form for DW_AT_GNU_entry_view

When outputting entry views in symbolic mode, we used to use a lbl_id,
but that outputs the view as an addr, perhaps even in an indirect one,
which is all excessive and undesirable for a small assembler-computed
constant.

Introduce a new value class for symbolic views, so that we can output
the labels as constant data, using small forms that should suffice for
any symbolic views.

Ideally, we'd use uleb128, but then the compiler would have to defer
.debug_info offset computation to the assembler.  I'm not going there
for now, so a symbolic uleb128 assembler constant in an attribute is
not something GCC can deal with ATM.

for  gcc/ChangeLog

	PR debug/84620
	* dwarf2out.h (dw_val_class): Add dw_val_class_symview.
	(dw_val_node): Add val_symbolic_view.
	* dwarf2out.c (dw_val_equal_p): Handle symview.
	(add_AT_symview): New.
	(print_dw_val): Handle symview.
	(attr_checksum, attr_checksum_ordered): Likewise.
	(same_dw_val_p, size_of_die): Likewise.
	(value_format, output_die): Likewise.
	(add_high_low_attributes): Use add_AT_symview for entry_view.
---
 gcc/dwarf2out.c |   40 +++++++++++++++++++++++++++++++++++++++-
 gcc/dwarf2out.h |    4 +++-
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 41bb11558f69..b52edee845f2 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1434,6 +1434,8 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b)
       return a->v.val_die_ref.die == b->v.val_die_ref.die;
     case dw_val_class_fde_ref:
       return a->v.val_fde_index == b->v.val_fde_index;
+    case dw_val_class_symview:
+      return strcmp (a->v.val_symbolic_view, b->v.val_symbolic_view) == 0;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -3600,6 +3602,7 @@ static addr_table_entry *add_addr_table_entry (void *, enum ate_kind);
 static void remove_addr_table_entry (addr_table_entry *);
 static void add_AT_addr (dw_die_ref, enum dwarf_attribute, rtx, bool);
 static inline rtx AT_addr (dw_attr_node *);
+static void add_AT_symview (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lineptr (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_macptr (dw_die_ref, enum dwarf_attribute, const char *);
@@ -5114,6 +5117,21 @@ add_AT_vms_delta (dw_die_ref die, enum dwarf_attribute attr_kind,
   add_dwarf_attr (die, &attr);
 }
 
+/* Add a symbolic view identifier attribute value to a DIE.  */
+
+static inline void
+add_AT_symview (dw_die_ref die, enum dwarf_attribute attr_kind,
+               const char *view_label)
+{
+  dw_attr_node attr;
+
+  attr.dw_attr = attr_kind;
+  attr.dw_attr_val.val_class = dw_val_class_symview;
+  attr.dw_attr_val.val_entry = NULL;
+  attr.dw_attr_val.v.val_symbolic_view = xstrdup (view_label);
+  add_dwarf_attr (die, &attr);
+}
+
 /* Add a label identifier attribute value to a DIE.  */
 
 static inline void
@@ -6457,6 +6475,9 @@ print_dw_val (dw_val_node *val, bool recurse, FILE *outfile)
       fprintf (outfile, "delta: @slotcount(%s-%s)",
 	       val->v.val_vms_delta.lbl2, val->v.val_vms_delta.lbl1);
       break;
+    case dw_val_class_symview:
+      fprintf (outfile, "view: %s", val->v.val_symbolic_view);
+      break;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -6828,6 +6849,7 @@ attr_checksum (dw_attr_node *at, struct md5_ctx *ctx, int *mark)
 
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7124,6 +7146,7 @@ attr_checksum_ordered (enum dwarf_tag tag, dw_attr_node *at,
       break;
 
     case dw_val_class_fde_ref:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7624,6 +7647,9 @@ same_dw_val_p (const dw_val_node *v1, const dw_val_node *v2, int *mark)
     case dw_val_class_die_ref:
       return same_die_p (v1->v.val_die_ref.die, v2->v.val_die_ref.die, mark);
 
+    case dw_val_class_symview:
+      return strcmp (v1->v.val_symbolic_view, v2->v.val_symbolic_view) == 0;
+
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
     case dw_val_class_lbl_id:
@@ -9284,6 +9310,10 @@ size_of_die (dw_die_ref die)
 	      size += csize;
 	  }
 	  break;
+	case dw_val_class_symview:
+	  /* FIXME: Use uleb128 rather than data2.  */
+	  size += 2;
+	  break;
 	case dw_val_class_const_implicit:
 	case dw_val_class_unsigned_const_implicit:
 	case dw_val_class_file_implicit:
@@ -9732,6 +9762,9 @@ value_format (dw_attr_node *a)
 	default:
 	  return DW_FORM_block1;
 	}
+    case dw_val_class_symview:
+      /* FIXME: Use uleb128 rather than data2.  */
+      return DW_FORM_data2;
     case dw_val_class_vec:
       switch (constant_size (a->dw_attr_val.v.val_vec.length
 			     * a->dw_attr_val.v.val_vec.elt_size))
@@ -10497,6 +10530,11 @@ output_die (dw_die_ref die)
 	  }
 	  break;
 
+	case dw_val_class_symview:
+	  /* FIXME: Use uleb128 rather than data2.  */
+	  dw2_asm_output_addr (2, a->dw_attr_val.v.val_symbolic_view, "%s", name);
+	  break;
+
 	case dw_val_class_const_implicit:
 	  if (flag_debug_asm)
 	    fprintf (asm_out_file, "\t\t\t%s %s ("
@@ -23809,7 +23847,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
 		 indirecting them through a table might make things
 		 easier, but even that would be more wasteful,
 		 space-wise, than what we have now.  */
-	      add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);
+	      add_AT_symview (die, DW_AT_GNU_entry_view, label);
 	    }
 	}
 
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index a1856a5185e2..f2240af85375 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -161,7 +161,8 @@ enum dw_val_class
   dw_val_class_const_implicit,
   dw_val_class_unsigned_const_implicit,
   dw_val_class_file_implicit,
-  dw_val_class_view_list
+  dw_val_class_view_list,
+  dw_val_class_symview
 };
 
 /* Describe a floating point constant value, or a vector constant value.  */
@@ -209,6 +210,7 @@ struct GTY(()) dw_val_node {
       HOST_WIDE_INT GTY ((default)) val_int;
       unsigned HOST_WIDE_INT
 	GTY ((tag ("dw_val_class_unsigned_const"))) val_unsigned;
+      char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view;
       double_int GTY ((tag ("dw_val_class_const_double"))) val_double;
       wide_int_ptr GTY ((tag ("dw_val_class_wide_int"))) val_wide;
       dw_vec_const GTY ((tag ("dw_val_class_vec"))) val_vec;



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

Re: [PR84620] output symbolic entry_view as data2, not addr

[Alexandre Oliva]

On Mar  2, 2018, Alexandre Oliva <aoliva@redhat.com> wrote:

> Mark Wielaard is implementing support for LVU and IEPM in elfutils, and
> he was surprised by the encoding of DW_AT_GNU_entry_view; so was I!
> When GCC computes and outputs views internally (broken without internal
> view resets), it outputs entry_view attributes as data: it knows the
> view numbers.  However, when it uses the assembler to compute the views
> symbolically, it has to output the view symbolically.

> I'd chosen lbl_id to that end, without giving it much thought, but that
> was no good: it's not just space-inefficient, it uses an addr encoding,
> indirect in some cases, for something that's not a real label, but
> rather a a small assembler data constant.  Oops.

Jakub wrote (in the BZ):

> I meant to say:
> The char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view; line 
> should come at the and of the union, not before the other classes.

I've moved the union field down in the revised patch below, but I don't
see the point, and I thought it would be better to keep it close to
logically-similar entries.  If the point is just to make it parallel to
the order of the enum (which manh other entries don't seem to have cared
about), maybe moving the enum would be better?


> What guarantees the symview symbols always fit into 16 bits?  Does 
> something    punt if it needs more than 65536 views?

There's no guarantee it will fit in 16 bits; the assembler will warn if
it truncates a view number.  There's no real upper limit, so uleb128
would be ideal, but since that's not viable ATM, I had to pick
something, and 16 bits would cover all really useful cases than 32 or
even 64 bits would, while being more compact.  I was even tempted to go
with 8 bits, but thought that was pushing it.  I can make it 32 if you
consider it better.  Or maybe a param?


> The FIXMEs don't really look helpful, we are not going to change the 
> offset computation from compile time to assemble time, that would be 
> terribly expensive.

Why do you say it would be terribly expensive to let the assembler compute
offsets in .debug_info?


[IEPM] [PR debug/84620] use constant form for DW_AT_GNU_entry_view

When outputting entry views in symbolic mode, we used to use a lbl_id,
but that outputs the view as an addr, perhaps even in an indirect one,
which is all excessive and undesirable for a small assembler-computed
constant.

Introduce a new value class for symbolic views, so that we can output
the labels as constant data, using small forms that should suffice for
any symbolic views.

Ideally, we'd use uleb128, but then the compiler would have to defer
.debug_info offset computation to the assembler.  I'm not going there
for now, so a symbolic uleb128 assembler constant in an attribute is
not something GCC can deal with ATM.

for  gcc/ChangeLog

	PR debug/84620
	* dwarf2out.h (dw_val_class): Add dw_val_class_symview.
	(dw_val_node): Add val_symbolic_view.
	* dwarf2out.c (dw_val_equal_p): Handle symview.
	(add_AT_symview): New.
	(print_dw_val): Handle symview.
	(attr_checksum, attr_checksum_ordered): Likewise.
	(same_dw_val_p, size_of_die): Likewise.
	(value_format, output_die): Likewise.
	(add_high_low_attributes): Use add_AT_symview for entry_view.
---
 gcc/dwarf2out.c |   40 +++++++++++++++++++++++++++++++++++++++-
 gcc/dwarf2out.h |    4 +++-
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 41bb11558f69..b52edee845f2 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1434,6 +1434,8 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b)
       return a->v.val_die_ref.die == b->v.val_die_ref.die;
     case dw_val_class_fde_ref:
       return a->v.val_fde_index == b->v.val_fde_index;
+    case dw_val_class_symview:
+      return strcmp (a->v.val_symbolic_view, b->v.val_symbolic_view) == 0;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -3600,6 +3602,7 @@ static addr_table_entry *add_addr_table_entry (void *, enum ate_kind);
 static void remove_addr_table_entry (addr_table_entry *);
 static void add_AT_addr (dw_die_ref, enum dwarf_attribute, rtx, bool);
 static inline rtx AT_addr (dw_attr_node *);
+static void add_AT_symview (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lineptr (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_macptr (dw_die_ref, enum dwarf_attribute, const char *);
@@ -5114,6 +5117,21 @@ add_AT_vms_delta (dw_die_ref die, enum dwarf_attribute attr_kind,
   add_dwarf_attr (die, &attr);
 }
 
+/* Add a symbolic view identifier attribute value to a DIE.  */
+
+static inline void
+add_AT_symview (dw_die_ref die, enum dwarf_attribute attr_kind,
+               const char *view_label)
+{
+  dw_attr_node attr;
+
+  attr.dw_attr = attr_kind;
+  attr.dw_attr_val.val_class = dw_val_class_symview;
+  attr.dw_attr_val.val_entry = NULL;
+  attr.dw_attr_val.v.val_symbolic_view = xstrdup (view_label);
+  add_dwarf_attr (die, &attr);
+}
+
 /* Add a label identifier attribute value to a DIE.  */
 
 static inline void
@@ -6457,6 +6475,9 @@ print_dw_val (dw_val_node *val, bool recurse, FILE *outfile)
       fprintf (outfile, "delta: @slotcount(%s-%s)",
 	       val->v.val_vms_delta.lbl2, val->v.val_vms_delta.lbl1);
       break;
+    case dw_val_class_symview:
+      fprintf (outfile, "view: %s", val->v.val_symbolic_view);
+      break;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -6828,6 +6849,7 @@ attr_checksum (dw_attr_node *at, struct md5_ctx *ctx, int *mark)
 
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7124,6 +7146,7 @@ attr_checksum_ordered (enum dwarf_tag tag, dw_attr_node *at,
       break;
 
     case dw_val_class_fde_ref:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7624,6 +7647,9 @@ same_dw_val_p (const dw_val_node *v1, const dw_val_node *v2, int *mark)
     case dw_val_class_die_ref:
       return same_die_p (v1->v.val_die_ref.die, v2->v.val_die_ref.die, mark);
 
+    case dw_val_class_symview:
+      return strcmp (v1->v.val_symbolic_view, v2->v.val_symbolic_view) == 0;
+
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
     case dw_val_class_lbl_id:
@@ -9284,6 +9310,10 @@ size_of_die (dw_die_ref die)
 	      size += csize;
 	  }
 	  break;
+	case dw_val_class_symview:
+	  /* FIXME: Use uleb128 rather than data2.  */
+	  size += 2;
+	  break;
 	case dw_val_class_const_implicit:
 	case dw_val_class_unsigned_const_implicit:
 	case dw_val_class_file_implicit:
@@ -9732,6 +9762,9 @@ value_format (dw_attr_node *a)
 	default:
 	  return DW_FORM_block1;
 	}
+    case dw_val_class_symview:
+      /* FIXME: Use uleb128 rather than data2.  */
+      return DW_FORM_data2;
     case dw_val_class_vec:
       switch (constant_size (a->dw_attr_val.v.val_vec.length
 			     * a->dw_attr_val.v.val_vec.elt_size))
@@ -10497,6 +10530,11 @@ output_die (dw_die_ref die)
 	  }
 	  break;
 
+	case dw_val_class_symview:
+	  /* FIXME: Use uleb128 rather than data2.  */
+	  dw2_asm_output_addr (2, a->dw_attr_val.v.val_symbolic_view, "%s", name);
+	  break;
+
 	case dw_val_class_const_implicit:
 	  if (flag_debug_asm)
 	    fprintf (asm_out_file, "\t\t\t%s %s ("
@@ -23809,7 +23847,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
 		 indirecting them through a table might make things
 		 easier, but even that would be more wasteful,
 		 space-wise, than what we have now.  */
-	      add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);
+	      add_AT_symview (die, DW_AT_GNU_entry_view, label);
 	    }
 	}
 
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index a1856a5185e2..a0ba414014d4 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -161,7 +161,8 @@ enum dw_val_class
   dw_val_class_const_implicit,
   dw_val_class_unsigned_const_implicit,
   dw_val_class_file_implicit,
-  dw_val_class_view_list
+  dw_val_class_view_list,
+  dw_val_class_symview
 };
 
 /* Describe a floating point constant value, or a vector constant value.  */
@@ -233,6 +234,7 @@ struct GTY(()) dw_val_node {
 	} GTY ((tag ("dw_val_class_vms_delta"))) val_vms_delta;
       dw_discr_value GTY ((tag ("dw_val_class_discr_value"))) val_discr_value;
       dw_discr_list_ref GTY ((tag ("dw_val_class_discr_list"))) val_discr_list;
+      char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view;
     }
   GTY ((desc ("%1.val_class"))) v;
 };


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

Re: [PR84620] output symbolic entry_view as data2, not addr

[Jakub Jelinek]

On Tue, Mar 06, 2018 at 03:13:11AM -0300, Alexandre Oliva wrote:
> On Mar  2, 2018, Alexandre Oliva <aoliva@redhat.com> wrote:
> 
> > Mark Wielaard is implementing support for LVU and IEPM in elfutils, and
> > he was surprised by the encoding of DW_AT_GNU_entry_view; so was I!
> > When GCC computes and outputs views internally (broken without internal
> > view resets), it outputs entry_view attributes as data: it knows the
> > view numbers.  However, when it uses the assembler to compute the views
> > symbolically, it has to output the view symbolically.
> 
> > I'd chosen lbl_id to that end, without giving it much thought, but that
> > was no good: it's not just space-inefficient, it uses an addr encoding,
> > indirect in some cases, for something that's not a real label, but
> > rather a a small assembler data constant.  Oops.
> 
> Jakub wrote (in the BZ):
> 
> > I meant to say:
> > The char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view; line 
> > should come at the and of the union, not before the other classes.
> 
> I've moved the union field down in the revised patch below, but I don't
> see the point, and I thought it would be better to keep it close to
> logically-similar entries.  If the point is just to make it parallel to
> the order of the enum (which manh other entries don't seem to have cared
> about), maybe moving the enum would be better?

I think the order should match the order of the dw_val_class entries and
should be sorted from the most commonly used ones (ones used by most
different attributes etc.), so that somebody trying to learn dwarf2out
stuff can learn it more easily (say the dw_val_class_const,
dw_val_class_const_unsigned, dw_val_class_flag first etc.), but apparently
it is completely random already, I'll defer to Jason what he wants.

> > What guarantees the symview symbols always fit into 16 bits?  Does 
> > something    punt if it needs more than 65536 views?
> 
> There's no guarantee it will fit in 16 bits; the assembler will warn if
> it truncates a view number.  There's no real upper limit, so uleb128
> would be ideal, but since that's not viable ATM, I had to pick
> something, and 16 bits would cover all really useful cases than 32 or
> even 64 bits would, while being more compact.  I was even tempted to go
> with 8 bits, but thought that was pushing it.  I can make it 32 if you
> consider it better.  Or maybe a param?

I'm afraid I haven't understood yet why we need labels instead of compiler
assigned numbers for the views, so not really sure I can answer this.
Can the compiler count how many different view labels it has produced or
will produce, or at least compute easily an upper bound, and decide on the
form based on that?  Or what will exactly happen if you use too small form?
Silent wrong-debug, assembler error, something else?

> > The FIXMEs don't really look helpful, we are not going to change the 
> > offset computation from compile time to assemble time, that would be 
> > terribly expensive.
> 
> Why do you say it would be terribly expensive to let the assembler compute
> offsets in .debug_info?

Because people are already complaining about slow assembly times of debug
info, and offsets are used pretty much everywhere in .debug_info.
With some assemblers that don't allow .uleb128/.sleb128 that is impossible
to do, with others you'd need to emit a new label before every DIE and all
DW_AT_type and many others would need to use .LD123456-.LD1092456 kind of
expressions which assembler would need to parse and resolve
(DW_FORM_ref{1,2,4,8,_udata}).

	Jakub

Re: [PR84620] output symbolic entry_view as data2, not addr

[Jason Merrill]

On Tue, Mar 6, 2018 at 2:28 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Mar 06, 2018 at 03:13:11AM -0300, Alexandre Oliva wrote:
>> Jakub wrote (in the BZ):
>>
>> > I meant to say:
>> > The char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view; line
>> > should come at the and of the union, not before the other classes.
>>
>> I've moved the union field down in the revised patch below, but I don't
>> see the point, and I thought it would be better to keep it close to
>> logically-similar entries.  If the point is just to make it parallel to
>> the order of the enum (which manh other entries don't seem to have cared
>> about), maybe moving the enum would be better?
>
> I think the order should match the order of the dw_val_class entries and
> should be sorted from the most commonly used ones (ones used by most
> different attributes etc.), so that somebody trying to learn dwarf2out
> stuff can learn it more easily (say the dw_val_class_const,
> dw_val_class_const_unsigned, dw_val_class_flag first etc.), but apparently
> it is completely random already, I'll defer to Jason what he wants.

I don't think the order of the GTY tags matters much, it's just boilerplate.

Jason

Re: [PR84620] output symbolic entry_view as data2, not addr

[Alexandre Oliva]

On Mar  6, 2018, Jakub Jelinek <jakub@redhat.com> wrote:

> I'm afraid I haven't understood yet why we need labels instead of compiler
> assigned numbers for the views,

The compiler can't determine points at which the insn address changes
reliably (or at all).  It's impossible in the general case, considering
that alignment requests might be fulfilled by an assembler without any
address change, if the current address was just right, and only the
assembler would know that.

> Can the compiler count how many different view labels it has produced or
> will produce

It can do that, yeah.  We could even count the max estimated view
number, which, in the absence of view reset points (the compiler has
that disabled except on ports that define a target hook to enable it,
and none do), is equivalent to the number of views.  I guess this is a
reasonable upper bound to use, in the absence of a closer estimate.
I'll give it a shot.


> Or what will exactly happen if you use too small form?
> Silent wrong-debug, assembler error, something else?

>> the assembler will warn if it truncates a view number

Wrong-debug, but not silent.  Not an error either.


>> Why do you say it would be terribly expensive to let the assembler compute
>> offsets in .debug_info?

> Because people are already complaining about slow assembly times of debug
> info, and offsets are used pretty much everywhere in .debug_info.

They're used pretty much everywhere in code as well; I suspect we have a
lot more labels in code than we'd have within debug info.

As to complaints...  are those recent, possibly related with
https://sourceware.org/bugzilla/show_bug.cgi?id=22870
or were the complains already in place before LVU went in?

> With some assemblers that don't allow .uleb128/.sleb128 that is impossible
> to do, with others you'd need to emit a new label before every DIE and all
> DW_AT_type and many others would need to use .LD123456-.LD1092456 kind of
> expressions which assembler would need to parse and resolve
> (DW_FORM_ref{1,2,4,8,_udata}).

Assemblers determine addresses for labels all the time, and surely
computing offsets between them, when they're in the same section, can't
be that expensive.  Well, with relaxable frags whose sizes depend on the
offsets, it could in theory get expensive indeed, but would it in
practice?


I can see that mandatory [us]leb128 fields could be a problem, but are
there any fields that involve offsets into .debug_info, and that must be
represented as [us]leb128?  I don't think so.

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

Re: [PR84620] output symbolic entry_view as data2, not addr

[Jakub Jelinek]

On Thu, Mar 08, 2018 at 06:05:04AM -0300, Alexandre Oliva wrote:
> On Mar  6, 2018, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> > I'm afraid I haven't understood yet why we need labels instead of compiler
> > assigned numbers for the views,
> 
> The compiler can't determine points at which the insn address changes
> reliably (or at all).  It's impossible in the general case, considering
> that alignment requests might be fulfilled by an assembler without any
> address change, if the current address was just right, and only the
> assembler would know that.

An upper bound would be fine, count any potential spot where you introduce a
view and if the upper bound fits into 8-bit value, use data1, if into 16-bit
value, use data2, otherwise use data4.

	Jakub

Re: [PR84620] output symbolic entry_view as data2, not addr

[Alexandre Oliva]

On Mar  8, 2018, Jakub Jelinek <jakub@redhat.com> wrote:

> An upper bound would be fine, count any potential spot where you introduce a
> view and if the upper bound fits into 8-bit value, use data1, if into 16-bit
> value, use data2, otherwise use data4.

[IEPM] [PR debug/84620] use constant form for DW_AT_GNU_entry_view

When outputting entry views in symbolic mode, we used to use a lbl_id,
but that outputs the view as an addr, perhaps even in an indirect one,
which is all excessive and undesirable for a small assembler-computed
constant.

Introduce a new value class for symbolic views, so that we can output
the labels as constant data, using as narrow forms as possible, but
wide enough for any symbolic views output in the compilation.  We
don't know exactly where the assembler will reset views, but we count
the symbolic views since known reset points and use that as an upper
bound for view numbers.

Ideally, we'd use uleb128, but then the compiler would have to defer
.debug_info offset computation to the assembler.  I'm not going there
for now, so a symbolic uleb128 assembler constant in an attribute is
not something GCC can deal with ATM.

Regstrapped on i686- and x86_64-linux-gnu, with binutils 2.30.  Through
visual inspection, I've confirmed there are consistent uses of data1 and
data2 for DW_AT_GNU_entry_view in libgcc and libstdc++; wider const
forms were not needed.  Ok to install?

for  gcc/ChangeLog

	PR debug/84620
	* dwarf2out.h (dw_val_class): Add dw_val_class_symview.
	(dw_val_node): Add val_symbolic_view.
	* dwarf2out.c (dw_line_info_table): Add symviews_since_reset.
	(symview_upper_bound): New.
	(new_line_info_table): Initialize symviews_since_reset.
	(dwarf2out_source_line): Count symviews_since_reset and set
	symview_upper_bound.
	(dw_val_equal_p): Handle symview.
	(add_AT_symview): New.
	(print_dw_val): Handle symview.
	(attr_checksum, attr_checksum_ordered): Likewise.
	(same_dw_val_p, size_of_die): Likewise.
	(value_format, output_die): Likewise.
	(add_high_low_attributes): Use add_AT_symview for entry_view.
---
 gcc/dwarf2out.c |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 gcc/dwarf2out.h |    4 ++
 2 files changed, 94 insertions(+), 7 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 0348f4460e98..fbf87378ac79 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1434,6 +1434,8 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b)
       return a->v.val_die_ref.die == b->v.val_die_ref.die;
     case dw_val_class_fde_ref:
       return a->v.val_fde_index == b->v.val_fde_index;
+    case dw_val_class_symview:
+      return strcmp (a->v.val_symbolic_view, b->v.val_symbolic_view) == 0;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -2951,6 +2953,11 @@ struct GTY(()) dw_line_info_table {
      going to ask the assembler to assign.  */
   var_loc_view view;
 
+  /* This counts the number of symbolic views emitted in this table
+     since the latest view reset.  Its max value, over all tables,
+     sets symview_upper_bound.  */
+  var_loc_view symviews_since_reset;
+
 #define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1)
 #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0)
 #define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1)
@@ -2959,6 +2966,13 @@ struct GTY(()) dw_line_info_table {
   vec<dw_line_info_entry, va_gc> *entries;
 };
 
+/* This is an upper bound for view numbers that the assembler may
+   assign to symbolic views output in this translation.  It is used to
+   decide how big a field to use to represent view numbers in
+   symview-classed attributes.  */
+
+static var_loc_view symview_upper_bound;
+
 /* If we're keep track of location views and their reset points, and
    INSN is a reset point (i.e., it necessarily advances the PC), mark
    the next view in TABLE as reset.  */
@@ -3603,6 +3617,7 @@ static addr_table_entry *add_addr_table_entry (void *, enum ate_kind);
 static void remove_addr_table_entry (addr_table_entry *);
 static void add_AT_addr (dw_die_ref, enum dwarf_attribute, rtx, bool);
 static inline rtx AT_addr (dw_attr_node *);
+static void add_AT_symview (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lineptr (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_macptr (dw_die_ref, enum dwarf_attribute, const char *);
@@ -5117,6 +5132,21 @@ add_AT_vms_delta (dw_die_ref die, enum dwarf_attribute attr_kind,
   add_dwarf_attr (die, &attr);
 }
 
+/* Add a symbolic view identifier attribute value to a DIE.  */
+
+static inline void
+add_AT_symview (dw_die_ref die, enum dwarf_attribute attr_kind,
+               const char *view_label)
+{
+  dw_attr_node attr;
+
+  attr.dw_attr = attr_kind;
+  attr.dw_attr_val.val_class = dw_val_class_symview;
+  attr.dw_attr_val.val_entry = NULL;
+  attr.dw_attr_val.v.val_symbolic_view = xstrdup (view_label);
+  add_dwarf_attr (die, &attr);
+}
+
 /* Add a label identifier attribute value to a DIE.  */
 
 static inline void
@@ -6460,6 +6490,9 @@ print_dw_val (dw_val_node *val, bool recurse, FILE *outfile)
       fprintf (outfile, "delta: @slotcount(%s-%s)",
 	       val->v.val_vms_delta.lbl2, val->v.val_vms_delta.lbl1);
       break;
+    case dw_val_class_symview:
+      fprintf (outfile, "view: %s", val->v.val_symbolic_view);
+      break;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -6831,6 +6864,7 @@ attr_checksum (dw_attr_node *at, struct md5_ctx *ctx, int *mark)
 
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7127,6 +7161,7 @@ attr_checksum_ordered (enum dwarf_tag tag, dw_attr_node *at,
       break;
 
     case dw_val_class_fde_ref:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7627,6 +7662,9 @@ same_dw_val_p (const dw_val_node *v1, const dw_val_node *v2, int *mark)
     case dw_val_class_die_ref:
       return same_die_p (v1->v.val_die_ref.die, v2->v.val_die_ref.die, mark);
 
+    case dw_val_class_symview:
+      return strcmp (v1->v.val_symbolic_view, v2->v.val_symbolic_view) == 0;
+
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
     case dw_val_class_lbl_id:
@@ -9287,6 +9325,16 @@ size_of_die (dw_die_ref die)
 	      size += csize;
 	  }
 	  break;
+	case dw_val_class_symview:
+	  if (symview_upper_bound <= 0xff)
+	    size += 1;
+	  else if (symview_upper_bound <= 0xffff)
+	    size += 2;
+	  else if (symview_upper_bound <= 0xffffffff)
+	    size += 4;
+	  else
+	    size += 8;
+	  break;
 	case dw_val_class_const_implicit:
 	case dw_val_class_unsigned_const_implicit:
 	case dw_val_class_file_implicit:
@@ -9735,6 +9783,17 @@ value_format (dw_attr_node *a)
 	default:
 	  return DW_FORM_block1;
 	}
+    case dw_val_class_symview:
+      /* ??? We might use uleb128, but then we'd have to compute
+	 .debug_info offsets in the assembler.  */
+      if (symview_upper_bound <= 0xff)
+	return DW_FORM_data1;
+      else if (symview_upper_bound <= 0xffff)
+	return DW_FORM_data2;
+      else if (symview_upper_bound <= 0xffffffff)
+	return DW_FORM_data4;
+      else
+	return DW_FORM_data8;
     case dw_val_class_vec:
       switch (constant_size (a->dw_attr_val.v.val_vec.length
 			     * a->dw_attr_val.v.val_vec.elt_size))
@@ -10500,6 +10559,22 @@ output_die (dw_die_ref die)
 	  }
 	  break;
 
+	case dw_val_class_symview:
+	  {
+	    int vsize;
+	    if (symview_upper_bound <= 0xff)
+	      vsize = 1;
+	    else if (symview_upper_bound <= 0xffff)
+	      vsize = 2;
+	    else if (symview_upper_bound <= 0xffffffff)
+	      vsize = 4;
+	    else
+	      vsize = 8;
+	    dw2_asm_output_addr (vsize, a->dw_attr_val.v.val_symbolic_view,
+				 "%s", name);
+	  }
+	  break;
+
 	case dw_val_class_const_implicit:
 	  if (flag_debug_asm)
 	    fprintf (asm_out_file, "\t\t\t%s %s ("
@@ -23812,7 +23887,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
 		 indirecting them through a table might make things
 		 easier, but even that would be more wasteful,
 		 space-wise, than what we have now.  */
-	      add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);
+	      add_AT_symview (die, DW_AT_GNU_entry_view, label);
 	    }
 	}
 
@@ -27409,6 +27484,7 @@ new_line_info_table (void)
   table->line_num = 1;
   table->is_stmt = DWARF_LINE_DEFAULT_IS_STMT_START;
   FORCE_RESET_NEXT_VIEW (table->view);
+  table->symviews_since_reset = 0;
 
   return table;
 }
@@ -27606,11 +27682,16 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	  /* 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.  */
+	     compute it from the previous view, or equate it to a
+	     subsequent view (though it might not be there!), 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.  ??? Extend the assembler so that the
+	     compiler could emit e.g. ".locview .LVU#", to output a
+	     view without changing line number information.  We'd then
+	     have to count it in symviews_since_reset; when it's omitted,
+	     it doesn't count.  */
 	  if (!zero_view_p)
 	    zero_view_p = BITMAP_GGC_ALLOC ();
 	  bitmap_set_bit (zero_view_p, table->view);
@@ -27699,6 +27780,9 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	{
 	  if (!RESETTING_VIEW_P (table->view))
 	    {
+	      table->symviews_since_reset++;
+	      if (table->symviews_since_reset > symview_upper_bound)
+		symview_upper_bound = table->symviews_since_reset;
 	      /* When we're using the assembler to compute view
 		 numbers, we output symbolic labels after "view" in
 		 .loc directives, and the assembler will set them for
@@ -27717,6 +27801,7 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	    }
 	  else
 	    {
+	      table->symviews_since_reset = 0;
 	      if (FORCE_RESETTING_VIEW_P (table->view))
 		fputs (" view -0", asm_out_file);
 	      else
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index a1856a5185e2..a0ba414014d4 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -161,7 +161,8 @@ enum dw_val_class
   dw_val_class_const_implicit,
   dw_val_class_unsigned_const_implicit,
   dw_val_class_file_implicit,
-  dw_val_class_view_list
+  dw_val_class_view_list,
+  dw_val_class_symview
 };
 
 /* Describe a floating point constant value, or a vector constant value.  */
@@ -233,6 +234,7 @@ struct GTY(()) dw_val_node {
 	} GTY ((tag ("dw_val_class_vms_delta"))) val_vms_delta;
       dw_discr_value GTY ((tag ("dw_val_class_discr_value"))) val_discr_value;
       dw_discr_list_ref GTY ((tag ("dw_val_class_discr_list"))) val_discr_list;
+      char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view;
     }
   GTY ((desc ("%1.val_class"))) v;
 };


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

Re: [PR84620] output symbolic entry_view as data2, not addr

[Jakub Jelinek]

On Fri, Mar 09, 2018 at 05:49:34AM -0300, Alexandre Oliva wrote:
> @@ -27699,6 +27780,9 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
>  	{
>  	  if (!RESETTING_VIEW_P (table->view))
>  	    {
> +	      table->symviews_since_reset++;
> +	      if (table->symviews_since_reset > symview_upper_bound)
> +		symview_upper_bound = table->symviews_since_reset;

You could have used
	      symview_upper_bound
		= MAX (symview_upper_bound, table->symviews_since_reset);

Also, I'd say there should be
  symview_upper_bound = 0;
in dwarf2out_c_finalize for better behavior of GCC JIT.

Otherwise LGTM, but please give Jason a day to comment on.

	Jakub

Re: [PR84620] output symbolic entry_view as data2, not addr

[Jason Merrill]

On Fri, Mar 9, 2018 at 4:02 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Mar 09, 2018 at 05:49:34AM -0300, Alexandre Oliva wrote:
>> @@ -27699,6 +27780,9 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
>>       {
>>         if (!RESETTING_VIEW_P (table->view))
>>           {
>> +           table->symviews_since_reset++;
>> +           if (table->symviews_since_reset > symview_upper_bound)
>> +             symview_upper_bound = table->symviews_since_reset;
>
> You could have used
>               symview_upper_bound
>                 = MAX (symview_upper_bound, table->symviews_since_reset);
>
> Also, I'd say there should be
>   symview_upper_bound = 0;
> in dwarf2out_c_finalize for better behavior of GCC JIT.
>
> Otherwise LGTM, but please give Jason a day to comment on.

OK.

Jason

Re: [PR84620] output symbolic entry_view as data2, not addr

[Alexandre Oliva]

On Mar  9, 2018, Jakub Jelinek <jakub@redhat.com> wrote:

> Also, I'd say there should be
>   symview_upper_bound = 0;
> in dwarf2out_c_finalize for better behavior of GCC JIT.

Thanks.  I added code to clear zero_view_p too.

Here's what I'm checking in.


[IEPM] [PR debug/84620] use constant form for DW_AT_GNU_entry_view

When outputting entry views in symbolic mode, we used to use a lbl_id,
but that outputs the view as an addr, perhaps even in an indirect one,
which is all excessive and undesirable for a small assembler-computed
constant.

Introduce a new value class for symbolic views, so that we can output
the labels as constant data, using as narrow forms as possible, but
wide enough for any symbolic views output in the compilation.  We
don't know exactly where the assembler will reset views, but we count
the symbolic views since known reset points and use that as an upper
bound for view numbers.

Ideally, we'd use uleb128, but then the compiler would have to defer
.debug_info offset computation to the assembler.  I'm not going there
for now, so a symbolic uleb128 assembler constant in an attribute is
not something GCC can deal with ATM.

for  gcc/ChangeLog

	PR debug/84620
	* dwarf2out.h (dw_val_class): Add dw_val_class_symview.
	(dw_val_node): Add val_symbolic_view.
	* dwarf2out.c (dw_line_info_table): Add symviews_since_reset.
	(symview_upper_bound): New.
	(new_line_info_table): Initialize symviews_since_reset.
	(dwarf2out_source_line): Count symviews_since_reset and set
	symview_upper_bound.
	(dw_val_equal_p): Handle symview.
	(add_AT_symview): New.
	(print_dw_val): Handle symview.
	(attr_checksum, attr_checksum_ordered): Likewise.
	(same_dw_val_p, size_of_die): Likewise.
	(value_format, output_die): Likewise.
	(add_high_low_attributes): Use add_AT_symview for entry_view.
	(dwarf2out_finish): Reset symview_upper_bound, clear
	zero_view_p.
---
 gcc/dwarf2out.c |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 gcc/dwarf2out.h |    4 ++
 2 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index c39910f26cb3..4e6ee5e8f82e 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -1434,6 +1434,8 @@ dw_val_equal_p (dw_val_node *a, dw_val_node *b)
       return a->v.val_die_ref.die == b->v.val_die_ref.die;
     case dw_val_class_fde_ref:
       return a->v.val_fde_index == b->v.val_fde_index;
+    case dw_val_class_symview:
+      return strcmp (a->v.val_symbolic_view, b->v.val_symbolic_view) == 0;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -2951,6 +2953,11 @@ struct GTY(()) dw_line_info_table {
      going to ask the assembler to assign.  */
   var_loc_view view;
 
+  /* This counts the number of symbolic views emitted in this table
+     since the latest view reset.  Its max value, over all tables,
+     sets symview_upper_bound.  */
+  var_loc_view symviews_since_reset;
+
 #define FORCE_RESET_NEXT_VIEW(x) ((x) = (var_loc_view)-1)
 #define RESET_NEXT_VIEW(x) ((x) = (var_loc_view)0)
 #define FORCE_RESETTING_VIEW_P(x) ((x) == (var_loc_view)-1)
@@ -2959,6 +2966,13 @@ struct GTY(()) dw_line_info_table {
   vec<dw_line_info_entry, va_gc> *entries;
 };
 
+/* This is an upper bound for view numbers that the assembler may
+   assign to symbolic views output in this translation.  It is used to
+   decide how big a field to use to represent view numbers in
+   symview-classed attributes.  */
+
+static var_loc_view symview_upper_bound;
+
 /* If we're keep track of location views and their reset points, and
    INSN is a reset point (i.e., it necessarily advances the PC), mark
    the next view in TABLE as reset.  */
@@ -3603,6 +3617,7 @@ static addr_table_entry *add_addr_table_entry (void *, enum ate_kind);
 static void remove_addr_table_entry (addr_table_entry *);
 static void add_AT_addr (dw_die_ref, enum dwarf_attribute, rtx, bool);
 static inline rtx AT_addr (dw_attr_node *);
+static void add_AT_symview (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_lineptr (dw_die_ref, enum dwarf_attribute, const char *);
 static void add_AT_macptr (dw_die_ref, enum dwarf_attribute, const char *);
@@ -5117,6 +5132,21 @@ add_AT_vms_delta (dw_die_ref die, enum dwarf_attribute attr_kind,
   add_dwarf_attr (die, &attr);
 }
 
+/* Add a symbolic view identifier attribute value to a DIE.  */
+
+static inline void
+add_AT_symview (dw_die_ref die, enum dwarf_attribute attr_kind,
+               const char *view_label)
+{
+  dw_attr_node attr;
+
+  attr.dw_attr = attr_kind;
+  attr.dw_attr_val.val_class = dw_val_class_symview;
+  attr.dw_attr_val.val_entry = NULL;
+  attr.dw_attr_val.v.val_symbolic_view = xstrdup (view_label);
+  add_dwarf_attr (die, &attr);
+}
+
 /* Add a label identifier attribute value to a DIE.  */
 
 static inline void
@@ -6460,6 +6490,9 @@ print_dw_val (dw_val_node *val, bool recurse, FILE *outfile)
       fprintf (outfile, "delta: @slotcount(%s-%s)",
 	       val->v.val_vms_delta.lbl2, val->v.val_vms_delta.lbl1);
       break;
+    case dw_val_class_symview:
+      fprintf (outfile, "view: %s", val->v.val_symbolic_view);
+      break;
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -6831,6 +6864,7 @@ attr_checksum (dw_attr_node *at, struct md5_ctx *ctx, int *mark)
 
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7127,6 +7161,7 @@ attr_checksum_ordered (enum dwarf_tag tag, dw_attr_node *at,
       break;
 
     case dw_val_class_fde_ref:
+    case dw_val_class_symview:
     case dw_val_class_lbl_id:
     case dw_val_class_lineptr:
     case dw_val_class_macptr:
@@ -7627,6 +7662,9 @@ same_dw_val_p (const dw_val_node *v1, const dw_val_node *v2, int *mark)
     case dw_val_class_die_ref:
       return same_die_p (v1->v.val_die_ref.die, v2->v.val_die_ref.die, mark);
 
+    case dw_val_class_symview:
+      return strcmp (v1->v.val_symbolic_view, v2->v.val_symbolic_view) == 0;
+
     case dw_val_class_fde_ref:
     case dw_val_class_vms_delta:
     case dw_val_class_lbl_id:
@@ -9287,6 +9325,16 @@ size_of_die (dw_die_ref die)
 	      size += csize;
 	  }
 	  break;
+	case dw_val_class_symview:
+	  if (symview_upper_bound <= 0xff)
+	    size += 1;
+	  else if (symview_upper_bound <= 0xffff)
+	    size += 2;
+	  else if (symview_upper_bound <= 0xffffffff)
+	    size += 4;
+	  else
+	    size += 8;
+	  break;
 	case dw_val_class_const_implicit:
 	case dw_val_class_unsigned_const_implicit:
 	case dw_val_class_file_implicit:
@@ -9735,6 +9783,17 @@ value_format (dw_attr_node *a)
 	default:
 	  return DW_FORM_block1;
 	}
+    case dw_val_class_symview:
+      /* ??? We might use uleb128, but then we'd have to compute
+	 .debug_info offsets in the assembler.  */
+      if (symview_upper_bound <= 0xff)
+	return DW_FORM_data1;
+      else if (symview_upper_bound <= 0xffff)
+	return DW_FORM_data2;
+      else if (symview_upper_bound <= 0xffffffff)
+	return DW_FORM_data4;
+      else
+	return DW_FORM_data8;
     case dw_val_class_vec:
       switch (constant_size (a->dw_attr_val.v.val_vec.length
 			     * a->dw_attr_val.v.val_vec.elt_size))
@@ -10500,6 +10559,22 @@ output_die (dw_die_ref die)
 	  }
 	  break;
 
+	case dw_val_class_symview:
+	  {
+	    int vsize;
+	    if (symview_upper_bound <= 0xff)
+	      vsize = 1;
+	    else if (symview_upper_bound <= 0xffff)
+	      vsize = 2;
+	    else if (symview_upper_bound <= 0xffffffff)
+	      vsize = 4;
+	    else
+	      vsize = 8;
+	    dw2_asm_output_addr (vsize, a->dw_attr_val.v.val_symbolic_view,
+				 "%s", name);
+	  }
+	  break;
+
 	case dw_val_class_const_implicit:
 	  if (flag_debug_asm)
 	    fprintf (asm_out_file, "\t\t\t%s %s ("
@@ -23815,7 +23890,7 @@ add_high_low_attributes (tree stmt, dw_die_ref die)
 		 indirecting them through a table might make things
 		 easier, but even that would be more wasteful,
 		 space-wise, than what we have now.  */
-	      add_AT_lbl_id (die, DW_AT_GNU_entry_view, label);
+	      add_AT_symview (die, DW_AT_GNU_entry_view, label);
 	    }
 	}
 
@@ -27412,6 +27487,7 @@ new_line_info_table (void)
   table->line_num = 1;
   table->is_stmt = DWARF_LINE_DEFAULT_IS_STMT_START;
   FORCE_RESET_NEXT_VIEW (table->view);
+  table->symviews_since_reset = 0;
 
   return table;
 }
@@ -27609,11 +27685,16 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	  /* 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.  */
+	     compute it from the previous view, or equate it to a
+	     subsequent view (though it might not be there!), 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.  ??? Extend the assembler so that the
+	     compiler could emit e.g. ".locview .LVU#", to output a
+	     view without changing line number information.  We'd then
+	     have to count it in symviews_since_reset; when it's omitted,
+	     it doesn't count.  */
 	  if (!zero_view_p)
 	    zero_view_p = BITMAP_GGC_ALLOC ();
 	  bitmap_set_bit (zero_view_p, table->view);
@@ -27702,6 +27783,9 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	{
 	  if (!RESETTING_VIEW_P (table->view))
 	    {
+	      table->symviews_since_reset++;
+	      if (table->symviews_since_reset > symview_upper_bound)
+		symview_upper_bound = table->symviews_since_reset;
 	      /* When we're using the assembler to compute view
 		 numbers, we output symbolic labels after "view" in
 		 .loc directives, and the assembler will set them for
@@ -27720,6 +27804,7 @@ dwarf2out_source_line (unsigned int line, unsigned int column,
 	    }
 	  else
 	    {
+	      table->symviews_since_reset = 0;
 	      if (FORCE_RESETTING_VIEW_P (table->view))
 		fputs (" view -0", asm_out_file);
 	      else
@@ -31295,6 +31380,11 @@ dwarf2out_finish (const char *)
       debug_line_str_hash->traverse<enum dwarf_form,
 				    output_indirect_string> (form);
     }
+
+  /* ??? Move lvugid out of dwarf2out_source_line and reset it too?  */
+  symview_upper_bound = 0;
+  if (zero_view_p)
+    bitmap_clear (zero_view_p);
 }
 
 /* Returns a hash value for X (which really is a variable_value_struct).  */
diff --git a/gcc/dwarf2out.h b/gcc/dwarf2out.h
index a1856a5185e2..a0ba414014d4 100644
--- a/gcc/dwarf2out.h
+++ b/gcc/dwarf2out.h
@@ -161,7 +161,8 @@ enum dw_val_class
   dw_val_class_const_implicit,
   dw_val_class_unsigned_const_implicit,
   dw_val_class_file_implicit,
-  dw_val_class_view_list
+  dw_val_class_view_list,
+  dw_val_class_symview
 };
 
 /* Describe a floating point constant value, or a vector constant value.  */
@@ -233,6 +234,7 @@ struct GTY(()) dw_val_node {
 	} GTY ((tag ("dw_val_class_vms_delta"))) val_vms_delta;
       dw_discr_value GTY ((tag ("dw_val_class_discr_value"))) val_discr_value;
       dw_discr_list_ref GTY ((tag ("dw_val_class_discr_list"))) val_discr_list;
+      char * GTY ((tag ("dw_val_class_symview"))) val_symbolic_view;
     }
   GTY ((desc ("%1.val_class"))) v;
 };


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