From patchwork Tue Apr 23 10:15:30 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chung-Lin Tang X-Patchwork-Id: 238852 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id D7DC42C015A for ; Tue, 23 Apr 2013 20:15:53 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=An5RGSn3AEY1RQFLb Y3S9zZBIGUmF4dn4NhEQ3o3x/o0Cmf+r9VXcbCukqzxtttj1zlPn5mgj9DhjLBKq xAzXPyeVs+8WhVGfPSpTaNfpJWCG5WVolL0RwpYJAVLUGLocPNUkqAlcauXO2Rg3 CY4Ab5pQGVIoLJkqTtQrfrToWE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=Jj8d/Qr2lEYq5R7k19E8xht rW2M=; b=SEjpawD+EIEt0p5gZcpg1s0oWzE0z+FB7rxoKTaD8/MSTluC/O+aJwX 6GiIaEwcMq6+Xh2VqkLGCWtE4+yw50Czd/2f/t6l4Cvoz2I+u44L2fcHdcr//rxH KHv3I1TerdXY2WrKGVqXXPFjNqdexLn5HOPmj3OJf3K+l0xX/KPI= Received: (qmail 9309 invoked by alias); 23 Apr 2013 10:15:47 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 9299 invoked by uid 89); 23 Apr 2013 10:15:46 -0000 X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 23 Apr 2013 10:15:46 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1UUaGF-0004vg-Sz from ChungLin_Tang@mentor.com ; Tue, 23 Apr 2013 03:15:43 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 23 Apr 2013 03:15:43 -0700 Received: from [0.0.0.0] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.2.247.3; Tue, 23 Apr 2013 03:15:22 -0700 Message-ID: <51765F42.3070808@codesourcery.com> Date: Tue, 23 Apr 2013 18:15:30 +0800 From: Chung-Lin Tang User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/20130328 Thunderbird/17.0.5 MIME-Version: 1.0 To: Cary Coutant CC: Julian Brown , gcc-patches , Sandra Loosemore Subject: Re: [PATCH 4/5] Altera Nios II: dwarf generation fix References: <516FF9F4.10501@codesourcery.com> <51753CE6.4070008@codesourcery.com> <20130422171148.17045830@octopus> In-Reply-To: X-Virus-Found: No On 2013/4/23 01:35 AM, Cary Coutant wrote: >> A : host_integerp (value, TYPE_UNSIGNED (TREE_TYPE (value))) >> B : host_integerp (value, 0) >> >> AB AB AB AB >> type_size,hwi | 00 01 10 11 >> --------------+------------------------------- >> 32,32 | X X int int >> 64,32 | X X int int >> 32,64 | X X - int >> 64,64 | X X int int > > In the third column (AB == 10), we're emitting a single int today even > though we know it's not technically correct: GDB will display the > unsigned value as a negative number. That's marginally better than > emitting nothing at all when the value is larger than an hwi, but I > was arguing that as long as we're adding the ability to emit the > constant as a double, why not also use a double for an unsigned that > doesn't fit in a signed hwi? Yes, it'll waste some space, but the > value will be correctly displayed as a result. I'm not sure of other cases, but here it is only to mark the values of an enumerator type, so as long as the values are consistent, the correct behavior is to print out the right enum string (I know that's a bit not ideal, but just to point that out) > Upon further reflection, however... > > This comment is wrong: > > /* DWARF2 does not provide a way of indicating whether or > not enumeration constants are signed or unsigned. GDB > always assumes the values are signed, so we output all > values as if they were signed. That means that > enumeration constants with very large unsigned values > will appear to have negative values in the debugger. */ > > DWARF does in fact provide a way of indicating whether a constant is > signed or unsigned: DW_FORM_sdata and DW_FORM_udata. These forms were > in DWARF-2, and the following comment was added to the DWARF-3 spec: > > "If one of the DW_FORM_data forms is used to represent a signed or > unsigned integer, it can be hard for a consumer to discover the > context necessary to determine which interpretation is intended. > Producers are therefore strongly encouraged to use DW_FORM_sdata or > DW_FORM_udata for signed and unsigned integers respectively, rather > than DW_FORM_data." > > We should really be emitting unsigned constants using add_AT_unsigned: > > if (TYPE_UNSIGNED (TREE_TYPE (value))) > { > if (host_integerp (value, 1)) > add_AT_unsigned (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value)); > else > add_AT_unsigned_double (enum_die, DW_AT_const_value, > TREE_INT_CST_HIGH (value), > TREE_INT_CST_LOW (value)); > } > else > { > if (host_integerp (value, 0)) > add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value)); > else > add_AT_double (enum_die, DW_AT_const_value, > TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value)); > } > > add_AT_unsigned_double would be new, and would need a new > dw_val_class_unsigned_const_double enum. That seems like the completely correct solution; correct unsigned/signed x int/double tags for all situations. 0000...1xxx... can then be correctly read as an unsigned int, rather than an excessively wide double or (incorrectly) signed int. You said OK for Julian's patch in the last mail, so I'll take that as approved (for an interim solution). If you don't mind, I'll add a TODO to the comments (attached patch). Thanks, Chung-Lin Index: dwarf2out.c =================================================================== --- dwarf2out.c (revision 198177) +++ dwarf2out.c (working copy) @@ -17027,15 +17027,27 @@ gen_enumeration_type_die (tree type, dw_die_ref co if (TREE_CODE (value) == CONST_DECL) value = DECL_INITIAL (value); - if (host_integerp (value, TYPE_UNSIGNED (TREE_TYPE (value)))) + if (host_integerp (value, TYPE_UNSIGNED (TREE_TYPE (value))) + && (simple_type_size_in_bits (TREE_TYPE (value)) + <= HOST_BITS_PER_WIDE_INT || host_integerp (value, 0))) /* DWARF2 does not provide a way of indicating whether or not enumeration constants are signed or unsigned. GDB always assumes the values are signed, so we output all values as if they were signed. That means that enumeration constants with very large unsigned values - will appear to have negative values in the debugger. */ - add_AT_int (enum_die, DW_AT_const_value, - tree_low_cst (value, tree_int_cst_sgn (value) > 0)); + will appear to have negative values in the debugger. + + TODO: the above comment is wrong, DWARF2 does provide + DW_FORM_sdata/DW_FORM_udata to represent signed/unsigned data. + This should be re-worked to use correct signed/unsigned + int/double tags for all cases, instead of always treating as + signed. */ + add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value)); + else + /* Enumeration constants may be wider than HOST_WIDE_INT. Handle + that here. */ + add_AT_double (enum_die, DW_AT_const_value, + TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value)); } add_gnat_descriptive_type_attribute (type_die, type, context_die);