From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89301 invoked by alias); 2 Nov 2015 15:30:26 -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 89279 invoked by uid 89); 2 Nov 2015 15:30:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Nov 2015 15:30:23 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-17-HNaaQSvZSlCS_-cAESqtnA-1; Mon, 02 Nov 2015 15:30:18 +0000 Received: from localhost ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 2 Nov 2015 15:30:17 +0000 From: Richard Sandiford To: Bernd Schmidt Mail-Followup-To: Bernd Schmidt ,Ulrich Weigand , , richard.sandiford@arm.com Cc: Ulrich Weigand , Subject: Re: [ping] Re: Fix PR debug/66728 References: <20151028115839.1E7C85C3D@oc7340732750.ibm.com> <87ziz3ta4t.fsf@e105548-lin.cambridge.arm.com> <5630D8AE.1090608@redhat.com> Date: Mon, 02 Nov 2015 15:30:00 -0000 In-Reply-To: <5630D8AE.1090608@redhat.com> (Bernd Schmidt's message of "Wed, 28 Oct 2015 15:16:14 +0100") Message-ID: <87h9l4v2pi.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-MC-Unique: HNaaQSvZSlCS_-cAESqtnA-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-11/txt/msg00084.txt.bz2 Bernd Schmidt writes: > On 10/28/2015 02:06 PM, Richard Sandiford wrote: >> Ulrich Weigand writes: >>> seems this still hasn't gone upstream ... Any news? >> >> Ah, sorry, I should have been pinging it. I think it's still waiting >> for review. > > Hmm, unfortunately I have a hard time making sense of this patch. Some=20 > extra explanation would be appreciated. CONST_WIDE_INTs are like CONST_INTs in that their mode is always VOIDmode. The loc_descriptor code: case CONST_WIDE_INT: if (mode =3D=3D VOIDmode) mode =3D GET_MODE (rtl); was simply bogus, but a harmless no-op. Much more serious was the add_const_value_attribute code: case CONST_WIDE_INT: add_AT_wide (die, DW_AT_const_value, std::make_pair (rtl, GET_MODE (rtl))); return true; which, because GET_MODE (rtl) returns VOIDmode, would always try to create a zero-precision integer. The problem is that, unlike loc_descriptor, add_const_value_attribute had no separate mode parameter. The patch therefore adds one and tries its best to find the appropriate value, given the TYPE_MODE/DECL_MODE split. I used DECL_MODE in add_location_or_const_value_attribute for consistency with the {int_,}loc_descriptor calls in dw_loc_list_1 and loc_list_from_tree. I used TYPE_MODE in tree_add_const_value_attribute because it doesn't deal exclusively with decls. But like I say, the two should be the same in practice, since I don't think anything would promote to a value that's big enough to need a CONST_WIDE_INT. >>>> static bool >>>> -add_const_value_attribute (dw_die_ref die, rtx rtl) >>>> +add_const_value_attribute (dw_die_ref die, machine_mode mode, rtx rtl) > > Need to document the extra parameter, especially in light of the=20 > TYPE_MODE/DECL_MODE/rtx mode confusion you pointed out. I've reattached it below FWIW. >>>> { >>>> switch (GET_CODE (rtl)) >>>> { >>>> case CONST_INT: >>>> - { >>>> - HOST_WIDE_INT val =3D3D INTVAL (rtl); >>>> + if (mode !=3D3D BLKmode) >>>> + { >>>> + gcc_checking_assert (SCALAR_INT_MODE_P (mode)); >>>> + HOST_WIDE_INT val =3D3D INTVAL (rtl); >>>> =3D20 >>>> - if (val < 0) >>>> - add_AT_int (die, DW_AT_const_value, val); >>>> - else >>>> - add_AT_unsigned (die, DW_AT_const_value, (unsigned HOST_WIDE_INT) = val); >>>> - } >>>> - return true; >>>> + if (val < 0) >>>> + add_AT_int (die, DW_AT_const_value, val); >>>> + else >>>> + add_AT_unsigned (die, DW_AT_const_value, >>>> + (unsigned HOST_WIDE_INT) val); >>>> + return true; >>>> + } > > This is all a bit html mangled, but this and the other change in=20 > loc_descriptor seem to have the effect of doing nothing when called with= =20 > BLKmode. What is the desired effect of this on the debug information,=20 > and how is it achieved? Well, I'm not familiar with this code, so I'm not really the best person to say. But for CONST_INT loc_descriptor currently just refuses to generate debug information for the value, which is achieved by returning null from that function. The patch extends this to CONST_WIDE_INT. The patch also takes the same approach in add_const_value_attribute, where the desired effect of not generating debug info for the value is achieved by returning false. This fixes the bug because previously we tried to generate debug info and return true, but did so using a zero-bit value. Thanks, Richard gcc/ PR debug/66728 * dwarf2out.c (loc_descriptor): Remove redundant GET_MODE of CONST_WIDE_INTs. Handle BLKmode for CONST_WIDE_INT too. (add_const_value_attribute): Add a mode parameter. Check it for CONST_INT and CONST_WIDE_INT. Use it to build wide_int values. (add_location_or_const_value_attribute): Update call. (tree_add_const_value_attribute): Likewise. =20=20=20=20 gcc/testsuite/ PR debug/66728 * gcc.dg/debug/dwarf2/pr66728.c: New test. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 9ce3f09..cd84af6 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -3252,7 +3252,7 @@ static HOST_WIDE_INT field_byte_offset (const_tree); static void add_AT_location_description (dw_die_ref, enum dwarf_attribute, dw_loc_list_ref); static void add_data_member_location_attribute (dw_die_ref, tree); -static bool add_const_value_attribute (dw_die_ref, rtx); +static bool add_const_value_attribute (dw_die_ref, machine_mode, rtx); static void insert_int (HOST_WIDE_INT, unsigned, unsigned char *); static void insert_wide_int (const wide_int &, unsigned char *, int); static void insert_float (const_rtx, unsigned char *); @@ -13799,10 +13799,9 @@ loc_descriptor (rtx rtl, machine_mode mode, break; =20 case CONST_WIDE_INT: - if (mode =3D=3D VOIDmode) - mode =3D GET_MODE (rtl); - - if (mode !=3D VOIDmode && (dwarf_version >=3D 4 || !dwarf_strict)) + if (mode !=3D VOIDmode + && mode !=3D BLKmode + && (dwarf_version >=3D 4 || !dwarf_strict)) { loc_result =3D new_loc_descr (DW_OP_implicit_value, GET_MODE_SIZE (mode), 0); @@ -15577,25 +15576,33 @@ insert_float (const_rtx rtl, unsigned char *array) constants do not necessarily get memory "homes". */ =20 static bool -add_const_value_attribute (dw_die_ref die, rtx rtl) +add_const_value_attribute (dw_die_ref die, machine_mode mode, rtx rtl) { switch (GET_CODE (rtl)) { case CONST_INT: - { - HOST_WIDE_INT val =3D INTVAL (rtl); + if (mode !=3D BLKmode) + { + gcc_checking_assert (SCALAR_INT_MODE_P (mode)); + HOST_WIDE_INT val =3D INTVAL (rtl); =20 - if (val < 0) - add_AT_int (die, DW_AT_const_value, val); - else - add_AT_unsigned (die, DW_AT_const_value, (unsigned HOST_WIDE_INT) val); - } - return true; + if (val < 0) + add_AT_int (die, DW_AT_const_value, val); + else + add_AT_unsigned (die, DW_AT_const_value, + (unsigned HOST_WIDE_INT) val); + return true; + } + return false; =20 case CONST_WIDE_INT: - add_AT_wide (die, DW_AT_const_value, - std::make_pair (rtl, GET_MODE (rtl))); - return true; + if (mode !=3D BLKmode) + { + gcc_checking_assert (SCALAR_INT_MODE_P (mode)); + add_AT_wide (die, DW_AT_const_value, std::make_pair (rtl, mode)); + return true; + } + return false; =20 case CONST_DOUBLE: /* Note that a CONST_DOUBLE rtx could represent either an integer or= a @@ -15672,7 +15679,7 @@ add_const_value_attribute (dw_die_ref die, rtx rtl) =20 case CONST: if (CONSTANT_P (XEXP (rtl, 0))) - return add_const_value_attribute (die, XEXP (rtl, 0)); + return add_const_value_attribute (die, mode, XEXP (rtl, 0)); /* FALLTHROUGH */ case SYMBOL_REF: if (!const_ok_for_output (rtl)) @@ -16175,7 +16182,7 @@ add_location_or_const_value_attribute (dw_die_ref d= ie, tree decl, bool cache_p) =20 rtl =3D rtl_for_decl_location (decl); if (rtl && (CONSTANT_P (rtl) || GET_CODE (rtl) =3D=3D CONST_STRING) - && add_const_value_attribute (die, rtl)) + && add_const_value_attribute (die, DECL_MODE (decl), rtl)) return true; =20 /* See if we have single element location list that is equivalent to @@ -16196,7 +16203,7 @@ add_location_or_const_value_attribute (dw_die_ref d= ie, tree decl, bool cache_p) if (GET_CODE (rtl) =3D=3D EXPR_LIST) rtl =3D XEXP (rtl, 0); if ((CONSTANT_P (rtl) || GET_CODE (rtl) =3D=3D CONST_STRING) - && add_const_value_attribute (die, rtl)) + && add_const_value_attribute (die, DECL_MODE (decl), rtl)) return true; } /* If this decl is from BLOCK_NONLOCALIZED_VARS, we might need its @@ -16399,7 +16406,7 @@ tree_add_const_value_attribute (dw_die_ref die, tre= e t) =20 rtl =3D rtl_for_decl_init (init, type); if (rtl) - return add_const_value_attribute (die, rtl); + return add_const_value_attribute (die, TYPE_MODE (type), rtl); /* If the host and target are sane, try harder. */ else if (CHAR_BIT =3D=3D 8 && BITS_PER_UNIT =3D=3D 8 && initializer_constant_valid_p (init, type)) diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c b/gcc/testsuite/gc= c.dg/debug/dwarf2/pr66728.c new file mode 100644 index 0000000..ba41e97 --- /dev/null +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c @@ -0,0 +1,14 @@ +/* { dg-do compile { target { x86_64-*-* && lp64 } } } */ +/* { dg-options "-O -gdwarf -dA" } */ + +__uint128_t +test (void) +{ + static const __uint128_t foo =3D ((((__uint128_t) 0x22334455) << 96) + | 0x99aabb); + + return foo; +} + +/* { dg-final { scan-assembler {\.quad\t0x99aabb\t# DW_AT_const_value} } }= */ +/* { dg-final { scan-assembler {\.quad\t0x2233445500000000\t} } } */