From: Richard Sandiford <richard.sandiford@arm.com>
To: Ulrich Weigand <uweigand@de.ibm.com>
Cc: <gcc-patches@gcc.gnu.org>
Subject: Re: [ping] Re: Fix PR debug/66728
Date: Wed, 28 Oct 2015 13:14:00 -0000 [thread overview]
Message-ID: <87ziz3ta4t.fsf@e105548-lin.cambridge.arm.com> (raw)
In-Reply-To: <20151028115839.1E7C85C3D@oc7340732750.ibm.com> (Ulrich Weigand's message of "Wed, 28 Oct 2015 12:58:39 +0100")
Ulrich Weigand <uweigand@de.ibm.com> writes:
> Hi Richard,
>
> 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.
Thanks,
Richard
>> This is yet another bug caused by rtx having modeless scalar integer
>> constants. We need to use context to find the actual mode of a
>> CONST_INT or CONST_WIDE_INT.
>>
>> Getting a mode is especially awkward here. Decls have two modes
>> associated with them: TYPE_MODE (TREE_TYPE (decl)) and DECL_MODE (decl).
>> Promotion via things like promote_decl_mode can lead to the rtl having
>> a mode that is different from both of them. Sometimes structure decls
>> have BLKmode but are assigned an integer-mode rtl (e.g. when passing
>> 3-byte structures by value to functions).
>>
>> I think in this case we're just going to have to assume that none of
>> these fancy mode changes happen for something that's big enough to
>> need a CONST_WIDE_INT.
>>
>> loc_descriptor refuses to use CONST_INT for BLKmode decls (which aren't
>> actually integers at the source level). That seems like the right
>> behaviour, so this patch does that for add_const_value_attribute too.
>> It asserts that the mode is otherwise sensible for both CONST_INT
>> and CONST_WIDE_INT. Asserting for CONST_INT isn't strictly necessary
>> but means that the assumption will get much more coverage than asserting
>> only for CONST_WIDE_INT does.
>>
>> Tested on x86_64-linux-gnu and aarch64-linux-gnu. Also tested against
>> the gdb testsuite. OK to install?
>>
>> 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.
>>
>> gcc/testsuite/
>> PR debug/66728
>> * gcc.dg/debug/dwarf2/pr66728.c: New test.
>>
>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
>> index d9d3063..383d705 100644
>> --- a/gcc/dwarf2out.c
>> +++ b/gcc/dwarf2out.c
>> @@ -3259,7 +3259,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 *);
>> @@ -13795,10 +13795,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);
>> @@ -15576,25 +15575,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
>> @@ -15671,7 +15678,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))
>> @@ -16171,7 +16178,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
>> @@ -16192,7 +16199,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
>> @@ -16395,7 +16402,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} } } */
>>
next prev parent reply other threads:[~2015-10-28 13:07 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-21 14:24 Richard Sandiford
2015-08-21 16:20 ` Jeff Law
2015-10-28 12:04 ` [ping] " Ulrich Weigand
2015-10-28 13:14 ` Richard Sandiford [this message]
2015-10-28 14:25 ` Bernd Schmidt
2015-10-28 14:58 ` Ulrich Weigand
2015-11-02 15:30 ` Richard Sandiford
2015-11-02 16:29 ` Richard Sandiford
2015-11-02 20:34 ` [ping] " Mike Stump
2015-11-02 20:55 ` Richard Sandiford
2015-11-02 23:29 ` Mike Stump
2015-11-03 8:46 ` Richard Sandiford
2015-11-03 21:59 ` Mike Stump
2015-11-04 9:43 ` Richard Biener
2015-11-04 11:58 ` Mike Stump
2015-11-04 12:15 ` Richard Biener
2015-11-04 19:36 ` Mike Stump
2015-11-04 20:51 ` Richard Sandiford
2015-11-04 23:45 ` Mike Stump
2015-11-05 12:32 ` Richard Biener
2015-11-06 1:35 ` Mike Stump
2015-11-06 13:06 ` Richard Biener
2015-11-09 18:47 ` Mike Stump
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ziz3ta4t.fsf@e105548-lin.cambridge.arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=uweigand@de.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).