From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25816 invoked by alias); 6 Mar 2018 06:13:29 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 25447 invoked by uid 89); 6 Mar 2018 06:13:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 06 Mar 2018 06:13:27 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8D5D3C049D49 for ; Tue, 6 Mar 2018 06:13:25 +0000 (UTC) Received: from freie.home (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2C4F0600C8; Tue, 6 Mar 2018 06:13:22 +0000 (UTC) Received: from livre (livre.home [172.31.160.2]) by freie.home (8.15.2/8.15.2) with ESMTP id w266DBjc157312; Tue, 6 Mar 2018 03:13:13 -0300 From: Alexandre Oliva To: jakub@redhat.com Cc: gcc-patches@gcc.gnu.org, mjw@redhat.com Subject: Re: [PR84620] output symbolic entry_view as data2, not addr References: Date: Tue, 06 Mar 2018 06:13:00 -0000 In-Reply-To: (Alexandre Oliva's message of "Fri, 02 Mar 2018 04:57:43 -0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2018-03/txt/msg00248.txt.bz2 On Mar 2, 2018, Alexandre Oliva 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