On Thu, Nov 5, 2015 at 12:45 AM, Mike Stump <mikest...@comcast.net> wrote:
>
> On Nov 4, 2015, at 12:50 PM, Richard Sandiford <richard.sandif...@arm.com>
> wrote:
>
>> Mike Stump <mikest...@comcast.net> writes:
>>> Index: dwarf2out.c
>>> ===================================================================
>>> --- dwarf2out.c (revision 229720)
>>> +++ dwarf2out.c (working copy)
>>> @@ -15593,8 +15593,13 @@
>>> return true;
>>>
>>> case CONST_WIDE_INT:
>>> - add_AT_wide (die, DW_AT_const_value,
>>> - std::make_pair (rtl, GET_MODE (rtl)));
>>> + {
>>> + wide_int w1 = std::make_pair (rtl, MAX_MODE_INT);
>>> + int prec = MIN (wi::min_precision (w1, UNSIGNED),
>>> + (unsigned int)CONST_WIDE_INT_NUNITS (rtl) *
>>> HOST_BITS_PER_WIDE_INT);
>>> + wide_int w = wide_int::from (w1, prec, UNSIGNED);
>>> + add_AT_wide (die, DW_AT_const_value, w);
>>> + }
>>> return true;
>>>
>>> case CONST_DOUBLE:
>>
>> Setting the precision based on CONST_WIDE_INT_NUNITS means that
>> we might end up with two different precisions for two values of
>> the same variable. E.g. for a 192-bit type, 1<<64 would be given
>> a precision of 128 (because it needs two HWIs to store) but 1<<128
>> would be given a precision of 192 (because it needs three HWIs to store).
>> We could then abort when comparing them for equality, since equality
>> needs both integers to have the same precision. E.g. from same_dw_val_p:
>>
>> case dw_val_class_wide_int:
>> return *v1->v.val_wide == *v2->v.val_wide;
>
> Yeah, seems like we should have a v1.prec == v2.prec && on that. The bad
> news, there are four of them that are like this. The good news, 3 of them
> are location operands, and I don’t think they can hit for a very long time.
> I think this is an oversight from the double_int version of the code where we
> just check the 128 bits for equality. We can see if Richard wants to weigh
> in. I think I’d just pre-approve the change, though, I think a helper to
> perform mixed equality testing would be the way to go as there are 4 of them,
> and I pretty sure they should all use the mixed version. Though, maybe the
> location list versions are never mixed. If they aren’t, then there is only 1
> client, so, I’d just do the precision test inline. Anyone able to comment on
> the location list aspect of this?
No idea on location lists but maybe this means we should just use the
maximum supported integer mode for CONST_WIDE_INTs?
