From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49855 invoked by alias); 4 Nov 2015 09:43:56 -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 49737 invoked by uid 89); 4 Nov 2015 09:43:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-yk0-f181.google.com Received: from mail-yk0-f181.google.com (HELO mail-yk0-f181.google.com) (209.85.160.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 04 Nov 2015 09:43:47 +0000 Received: by ykba4 with SMTP id a4so62271635ykb.3 for ; Wed, 04 Nov 2015 01:43:45 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.129.76.14 with SMTP id z14mr437808ywa.263.1446630225296; Wed, 04 Nov 2015 01:43:45 -0800 (PST) Received: by 10.37.117.136 with HTTP; Wed, 4 Nov 2015 01:43:45 -0800 (PST) In-Reply-To: <29C8CB44-A0EC-4A9A-A4D3-3ABB6D66DA5B@comcast.net> References: <20151028115839.1E7C85C3D@oc7340732750.ibm.com> <87ziz3ta4t.fsf@e105548-lin.cambridge.arm.com> <5630D8AE.1090608@redhat.com> <87h9l4v2pi.fsf@e105548-lin.cambridge.arm.com> <87611kuzz4.fsf@e105548-lin.cambridge.arm.com> <871tc8unnx.fsf@e105548-lin.cambridge.arm.com> <87wptztqpx.fsf@e105548-lin.cambridge.arm.com> <29C8CB44-A0EC-4A9A-A4D3-3ABB6D66DA5B@comcast.net> Date: Wed, 04 Nov 2015 09:43:00 -0000 Message-ID: Subject: Re: [ping] Fix PR debug/66728 From: Richard Biener To: Mike Stump Cc: Richard Sandiford , Bernd Schmidt , Ulrich Weigand , GCC Patches Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-11/txt/msg00307.txt.bz2 On Tue, Nov 3, 2015 at 10:58 PM, Mike Stump wrote: > On Nov 3, 2015, at 12:46 AM, Richard Sandiford wrote: >> This isn't just an argument about the DWARF standard though. It's an >> argument about GCC internals. Presumably these hypothetical BLKmode >> types would need to support addition, > > I don=E2=80=99t recall seeing that as a requirement in the dwarf standard= , nor in the language standard of the hypothetical language I posited. Fur= ther, I don=E2=80=99t recall seeing a prohibition on fetching from memory, = combining three byte to form a 24 bit value in a 64 bit register on x86_64,= nor the prohibition on doing a 64-bit add on that architecture. Do you ha= ve a quote to support your position? Further, I don=E2=80=99t even see sup= port that add must be supported in the language front end I posited. Since= I control it, you can=E2=80=99t can=E2=80=99t require much of anything in = it. > >> but plus:BLK is not well formed, > > BLKmode is well formed in gcc. I can=E2=80=99t image what this even mean= s. > >> and wouldn't distinguish between your 3-byte and 152-bit cases. > > A 3 byte value in dwarf has a byte length of 3, and a 152 bit value has a= byte length of 19. I fail to see why in dwarf they cannot be distinguishe= d. A cleaver front end can track the values in that front end, and regions= of memory that is uses to back objects, usually the front end does track o= bjects, and when asked to, will generate dwarf for them. I believe that fr= ont ends are able to track object byte sized for objects. I believe that t= he C++ front end does this for example, and will generate dwarf. > >> I don=E2=80=99t think const_int and const_wide_int are logically differe= nt. > > They aren=E2=80=99t. Indeed, the code for const_wide_int was copied from= const_int and/or const_double and only enhanced to have an arbitrary size. > >> There's the >> historical decision that const_int doesn't have a stored mode, but I >> don't think that was because we wanted to support const_ints that are >> conceptually BLKmode. > > So, given that what must be, given the semantics of dwarf, and the totali= ty of semantics available to a front end, BLKmode values are perfectly well= defined. I don=E2=80=99t know why you would think otherwise. Further, ar= guing that we must break them, because=E2=80=A6 is silly, without the beca= use elaborated. A constant value that is known at compile time to be an ar= bitrary value whose type is a signed n byte int can be represented in const= _int when n fits, and can be expressed as a const_wide_int when n fits and = can be expressed in dwarf. Further, const_int is not unreasonable to repre= sent constants, further, there is no prohibition on a front end having cons= tants, nor on the size of those constants being n bytes in size, at least f= or values of n where n can be supported directly by const_wide_int. If ther= e were one, you could be able to quote it. > >> I think from an rtl perspective the only sensible way for frontends to >> cope with integers whose size doesn't match an rtl mode is to promote >> to the next widest mode, > > No, again, to support your position, you=E2=80=99d have to quote the part= of gcc that disclaims support. You have not. You can think there is a pr= ohibition on 3 byte ints, if you want. I can=E2=80=99t fix that. What I c= an object to, is adding a line to the document that says that gcc cannot be= used to write a front end that supports 3 byte ints, or 19 byte ints. > >> which is what the stor-layout.c code I quoted >> does. Obviously if your 3 byte type is actually 3 bytes in memory rather >> than 4, and no 3-byte mode is available, you can't just load and store >> the value using a normal rtl move. You have to use bitfield extraction >> and insertion instead. > > So? Another technique that a front end could use is to fetch a HImode, a= nd a QImode value, shift and or them together, but no matter. > >> I picked this PR up because it was wide-int-related, even though >> (as is probably all too obvious from this thread) I'm not familiar >> with the dwarf2out.c code. It's actually your commit that I'm trying >> to fix here (r201707). Would you mind taking the PR over and handling >> it the way you think it should be handled? > > So, I did up the patch: > > Index: rtl.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- rtl.h (revision 229720) > +++ rtl.h (working copy) > @@ -2086,6 +2086,15 @@ > inline unsigned int > wi::int_traits ::get_precision (const rtx_mode_t &x) > { > + /* VOIDmode CONST_WIDE_INT pairings are used for debug information > + when the type of the underlying object has not been tracked and > + is unimportant so we form a precision from the value of the > + constant. This is sufficient for dwarf generation as dwarf can > + track the type separately from the value. */ > + if (x.second =3D=3D VOIDmode > + && CONST_WIDE_INT_P (x.first)) > + return CONST_WIDE_INT_NUNITS (x.first) * HOST_BITS_PER_WIDE_INT; > + > return GET_MODE_PRECISION (x.second); > } I think you should limit the effect of this patch to the dwarf2out use as the above doesn't make sense to me. As Richard said, a (plus:BLK ...) isn't well-formed. Ideally we'd have an assert that you don't create a rtx_mode_t with VOIDmode or BLKmode. Handling the CONST_WIDE_INT in dwarf2out.c the same as we did before (with CONST_DOUBLE) looks sensible as far of fixing a regression (I assume the diff to the dwarf results in essentially the same DWARF as what was present before wide-int). Richard. > Index: wide-int.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- wide-int.h (revision 229720) > +++ wide-int.h (working copy) > @@ -156,6 +156,10 @@ > rtx r =3D ... > wide_int x =3D std::make_pair (r, mode); > > + If the mode is VOIDmode, then the precision of the wide_int will be > + the number of bits used to represent the rtl's value. This is used f= or > + untyped constants for dwarf debug information. > + > Similarly, a wide_int can only be constructed from a host value if > the target precision is given explicitly, such as in: > > The intent is to exactly match what gcc did before, to have wide-int hand= ling match what const_int does, and what const_double did in the past, mere= ly extended to support arbitrarily large values. I find, in this case, wha= t was done in the past to be valid and desirable. I don=E2=80=99t find any= reason to deviate from what was done before. > > The test case from the PR changes like this: > > --- t-bad.s 2015-11-03 16:19:13.786248224 -0500 > +++ t-fixed.s 2015-11-03 16:02:48.414290258 -0500 > @@ -23,7 +23,7 @@ test: > .Letext0: > .section .debug_info,"",@progbits > .Ldebug_info0: > - .long 0x63 # Length of Compilation Unit Info > + .long 0x74 # Length of Compilation Unit Info > .value 0x4 # DWARF version number > .long .Ldebug_abbrev0 # Offset Into Abbrev. Section > .byte 0x8 # Pointer Size (in bytes) > @@ -41,25 +41,28 @@ test: > .byte 0x1 # DW_AT_decl_file (t.c) > .byte 0x1 # DW_AT_decl_line > # DW_AT_prototyped > - .long 0x5a # DW_AT_type > + .long 0x6b # DW_AT_type > .quad .LFB0 # DW_AT_low_pc > .quad .LFE0-.LFB0 # DW_AT_high_pc > .uleb128 0x1 # DW_AT_frame_base > .byte 0x9c # DW_OP_call_frame_cfa > # DW_AT_GNU_all_call_sites > - .long 0x5a # DW_AT_sibling > + .long 0x6b # DW_AT_sibling > .uleb128 0x3 # (DIE (0x4e) DW_TAG_variable) > .long .LASF3 # DW_AT_name: "two127" > .byte 0x1 # DW_AT_decl_file (t.c) > .byte 0x3 # DW_AT_decl_line > - .long 0x61 # DW_AT_type > + .long 0x72 # DW_AT_type > + .byte 0x10 > + .quad 0 # DW_AT_const_value > + .quad 0x8000000000000000 # (null) > .byte 0 # end of children of DIE 0x2d > - .uleb128 0x4 # (DIE (0x5a) DW_TAG_base_type) > + .uleb128 0x4 # (DIE (0x6b) DW_TAG_base_type) > .byte 0x10 # DW_AT_byte_size > .byte 0x7 # DW_AT_encoding > .long .LASF4 # DW_AT_name: "__int128 unsigned" > - .uleb128 0x5 # (DIE (0x61) DW_TAG_const_type) > - .long 0x5a # DW_AT_type > + .uleb128 0x5 # (DIE (0x72) DW_TAG_const_type) > + .long 0x6b # DW_AT_type > .byte 0 # end of children of DIE 0xb > .section .debug_abbrev,"",@progbits > > I=E2=80=99m not a dwarf expert, but, I see the addition of two quads for = a const value, and that value seems to be 1 << 127 to my untrained eye. If= I had to theorizes, I=E2=80=99d say this is what the debug information was= before. [ checking ] I checked gcc-4.8 on x86_64, and the debug informati= on is now virtually the same: > > *** t-48.s 2015-11-03 16:24:23.286235021 -0500 > --- t-fixed.s 2015-11-03 16:02:48.414290258 -0500 > *************** test: > *** 28,35 **** > .long .Ldebug_abbrev0 # Offset Into Abbrev. Section > .byte 0x8 # Pointer Size (in bytes) > .uleb128 0x1 # (DIE (0xb) DW_TAG_compile_unit) > ! .long .LASF0 # DW_AT_producer: "GNU C 4.8.4 -mtune=3Dgeneric -= march=3Dx86-64 -g -O -fstack-protector" > ! .byte 0x1 # DW_AT_language > .ascii "t.c\0" # DW_AT_name > .long .LASF1 # DW_AT_comp_dir: "/home/mrs/net/gcc-linuxO0/gcc" > .quad .Ltext0 # DW_AT_low_pc > --- 28,35 ---- > .long .Ldebug_abbrev0 # Offset Into Abbrev. Section > .byte 0x8 # Pointer Size (in bytes) > .uleb128 0x1 # (DIE (0xb) DW_TAG_compile_unit) > ! .long .LASF0 # DW_AT_producer: "GNU C11 6.0.0 20151103 (experi= mental) [trunk revision 229720] -O -g" > ! .byte 0xc # DW_AT_language > .ascii "t.c\0" # DW_AT_name > .long .LASF1 # DW_AT_comp_dir: "/home/mrs/net/gcc-linuxO0/gcc" > .quad .Ltext0 # DW_AT_low_pc > *************** test: > *** 55,61 **** > .long 0x72 # DW_AT_type > .byte 0x10 > .quad 0 # DW_AT_const_value > ! .quad 0x8000000000000000 > .byte 0 # end of children of DIE 0x2d > .uleb128 0x4 # (DIE (0x6b) DW_TAG_base_type) > .byte 0x10 # DW_AT_byte_size > --- 55,61 ---- > .long 0x72 # DW_AT_type > .byte 0x10 > .quad 0 # DW_AT_const_value > ! .quad 0x8000000000000000 # (null) > .byte 0 # end of children of DIE 0x2d > .uleb128 0x4 # (DIE (0x6b) DW_TAG_base_type) > .byte 0x10 # DW_AT_byte_size > *************** test: > *** 162,168 **** > .Ldebug_line0: > .section .debug_str,"MS",@progbits,1 > .LASF0: > ! .string "GNU C 4.8.4 -mtune=3Dgeneric -march=3Dx86-64 -g -O -fsta= ck-protector" > .LASF1: > .string "/home/mrs/net/gcc-linuxO0/gcc" > .LASF3: > --- 162,168 ---- > .Ldebug_line0: > .section .debug_str,"MS",@progbits,1 > .LASF0: > ! .string "GNU C11 6.0.0 20151103 (experimental) [trunk revision 22= 9720] -O -g" > .LASF1: > .string "/home/mrs/net/gcc-linuxO0/gcc" > .LASF3: > *************** test: > *** 171,175 **** > .string "test" > .LASF4: > .string "__int128 unsigned" > ! .ident "GCC: (Ubuntu 4.8.4-2ubuntu1~14.04) 4.8.4" > .section .note.GNU-stack,"",@progbits > --- 171,175 ---- > .string "test" > .LASF4: > .string "__int128 unsigned" > ! .ident "GCC: (GNU) 6.0.0 20151103 (experimental) [trunk revision= 229720]" > .section .note.GNU-stack,"",@progbits > > The language seems weird, but, I bet it is inconsequential and an artifac= t of how I ran the test case. The # (null) is unfortunate, I don=E2=80=99t= think this is better than what gcc-4.8 did, but, I don=E2=80=99t think tha= t was caused by my patch. > > So, this would be my proposal to fix the issue. I'd invite anyone that t= hinks the dwarf information was wrong in gcc-4.8 or wrong post the patch, t= o let us know why. If there are reasons why my position is wrong, I=E2=80= =99d like to hear about it, otherwise I=E2=80=99ll plan on checking this in= with my wide-int hat on. Since this is a serious regression in debug qual= ity, this has to go the the release branches that contain wide-int.