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?
