Re: [ping] Fix PR debug/66728

[Mike Stump <mikestump@comcast.net>]

On Nov 4, 2015, at 12:50 PM, Richard Sandiford <richard.sandiford@arm.com> wrote:

> Mike Stump <mikestump@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?