[4/5] Altera Nios II: dwarf generation fix

On 2013/4/23 01:35 AM, Cary Coutant wrote:
>> A : host_integerp (value, TYPE_UNSIGNED (TREE_TYPE (value)))
>> B : host_integerp (value, 0)
>>
>>                 AB      AB      AB      AB
>> type_size,hwi | 00      01      10      11
>> --------------+-------------------------------
>> 32,32         | X       X       int     int
>> 64,32         | X       X       int     int
>> 32,64         | X       X       -       int
>> 64,64         | X       X       int     int
> 
> In the third column (AB == 10), we're emitting a single int today even
> though we know it's not technically correct: GDB will display the
> unsigned value as a negative number. That's marginally better than
> emitting nothing at all when the value is larger than an hwi, but I
> was arguing that as long as we're adding the ability to emit the
> constant as a double, why not also use a double for an unsigned that
> doesn't fit in a signed hwi? Yes, it'll waste some space, but the
> value will be correctly displayed as a result.

I'm not sure of other cases, but here it is only to mark the values of
an enumerator type, so as long as the values are consistent, the correct
behavior is to print out the right enum string (I know that's a bit not
ideal, but just to point that out)

> Upon further reflection, however...
> 
> This comment is wrong:
> 
>     /* DWARF2 does not provide a way of indicating whether or
>        not enumeration constants are signed or unsigned.  GDB
>        always assumes the values are signed, so we output all
>        values as if they were signed.  That means that
>        enumeration constants with very large unsigned values
>        will appear to have negative values in the debugger.  */
> 
> DWARF does in fact provide a way of indicating whether a constant is
> signed or unsigned: DW_FORM_sdata and DW_FORM_udata. These forms were
> in DWARF-2, and the following comment was added to the DWARF-3 spec:
> 
> "If one of the DW_FORM_data<n> forms is used to represent a signed or
> unsigned integer, it can be hard for a consumer to discover the
> context necessary to determine which interpretation is intended.
> Producers are therefore strongly encouraged to use DW_FORM_sdata or
> DW_FORM_udata for signed and unsigned integers respectively, rather
> than DW_FORM_data<n>."
> 
> We should really be emitting unsigned constants using add_AT_unsigned:
> 
>   if (TYPE_UNSIGNED (TREE_TYPE (value)))
>     {
>       if (host_integerp (value, 1))
>         add_AT_unsigned (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value));
>       else
>         add_AT_unsigned_double (enum_die, DW_AT_const_value,
>                                 TREE_INT_CST_HIGH (value),
>                                 TREE_INT_CST_LOW (value));
>     }
>   else
>     {
>       if (host_integerp (value, 0))
>         add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value));
>       else
>         add_AT_double (enum_die, DW_AT_const_value,
>                        TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value));
>     }
> 
> add_AT_unsigned_double would be new, and would need a new
> dw_val_class_unsigned_const_double enum.

That seems like the completely correct solution; correct unsigned/signed
x int/double tags for all situations. 0000...1xxx... can then be
correctly read as an unsigned int, rather than an excessively wide
double or (incorrectly) signed int.

You said OK for Julian's patch in the last mail, so I'll take that as
approved (for an interim solution). If you don't mind, I'll add a TODO
to the comments (attached patch).

Thanks,
Chung-Lin

> You said OK for Julian's patch in the last mail, so I'll take that as
> approved (for an interim solution). If you don't mind, I'll add a TODO
> to the comments (attached patch).

Looks good, thanks!

-cary