| Message ID | 1397511012.5633.53.camel@bordewijk.wildebeest.org |
|---|---|
| State | New |
| Headers | show |
> + /* If HOST_WIDE_INT is big enough then represent the bound as > + a constant value. Note that we need to make sure the type > + is signed or unsigned. We cannot just add an unsigned > + constant if the value itself is positive. Some DWARF > + consumers will lookup the bounds type and then sign extend > + any unsigned values found for signed types. This is only > + for DW_AT_lower_bound, normally unsigned values > + (DW_FORM_data[1248]) are assumed to not need > + sign-extension. */ This comment confuses me. By "we need to make sure the type is signed or unsigned" (what else can it be?), I think you mean "we need to choose a form based on whether the type is signed or unsigned." And by "This is only for DW_AT_lower_bound, ...", I think you mean "This is needed only for DW_AT_{lower,upper}_bound, since for most other attributes, consumers will treat DW_FORM_data[1248] as unsigned values, regardless of the underlying type." Otherwise, the patch looks OK to me. -cary
On Tue, 2014-04-15 at 14:24 -0700, Cary Coutant wrote: > > + /* If HOST_WIDE_INT is big enough then represent the bound as > > + a constant value. Note that we need to make sure the type > > + is signed or unsigned. We cannot just add an unsigned > > + constant if the value itself is positive. Some DWARF > > + consumers will lookup the bounds type and then sign extend > > + any unsigned values found for signed types. This is only > > + for DW_AT_lower_bound, normally unsigned values > > + (DW_FORM_data[1248]) are assumed to not need > > + sign-extension. */ > > This comment confuses me. Sorry, obviously not my intention. But I see what I was trying to say and how I said it didn't make things very clear. Apologies. > By "we need to make sure the type is signed > or unsigned" (what else can it be?), I think you mean "we need to > choose a form based on whether the type is signed or unsigned." Yes, right. I was confusing matters in my comment because I was thinking of non-constants (reference or exprlocs) that are handled elsewhere later on in the code. > And by "This is only for DW_AT_lower_bound, ...", I think you mean "This is > needed only for DW_AT_{lower,upper}_bound, since for most other > attributes, consumers will treat DW_FORM_data[1248] as unsigned > values, regardless of the underlying type." Yes, right again. > Otherwise, the patch looks OK to me. Thanks I pushed it with the comment changed to how you expressed things. It now reads: /* If HOST_WIDE_INT is big enough then represent the bound as a constant value. We need to choose a form based on whether the type is signed or unsigned. We cannot just call add_AT_unsigned if the value itself is positive (add_AT_unsigned might add the unsigned value encoded as DW_FORM_data[1248]). Some DWARF consumers will lookup the bounds type and then sign extend any unsigned values found for signed types. This is needed only for DW_AT_{lower,upper}_bound, since for most other attributes, consumers will treat DW_FORM_data[1248] as unsigned values, regardless of the underlying type. */ Thanks, Mark
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index f2c127c..b1706ce 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2014-03-20 Mark Wielaard <mjw@redhat.com> + + * dwarf2out.c (add_bound_info): If HOST_WIDE_INT is big enough, + then represent the bound as normal constant value. + 2014-04-14 Paolo Carlini <paolo.carlini@oracle.com> * tree.h (TYPE_IDENTIFIER): Declare. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 721f761..7eef56c 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -16203,21 +16203,29 @@ add_bound_info (dw_die_ref subrange_die, enum dwarf_attribute bound_attr, tree b && tree_to_shwi (bound) == dflt) ; - /* Otherwise represent the bound as an unsigned value with the - precision of its type. The precision and signedness of the - type will be necessary to re-interpret it unambiguously. */ - else if (prec < HOST_BITS_PER_WIDE_INT) + /* If HOST_WIDE_INT is big enough then represent the bound as + a constant value. Note that we need to make sure the type + is signed or unsigned. We cannot just add an unsigned + constant if the value itself is positive. Some DWARF + consumers will lookup the bounds type and then sign extend + any unsigned values found for signed types. This is only + for DW_AT_lower_bound, normally unsigned values + (DW_FORM_data[1248]) are assumed to not need + sign-extension. */ + else if (prec <= HOST_BITS_PER_WIDE_INT + || TREE_INT_CST_HIGH (bound) == 0) { - unsigned HOST_WIDE_INT mask - = ((unsigned HOST_WIDE_INT) 1 << prec) - 1; - add_AT_unsigned (subrange_die, bound_attr, - TREE_INT_CST_LOW (bound) & mask); + if (TYPE_UNSIGNED (TREE_TYPE (bound))) + add_AT_unsigned (subrange_die, bound_attr, + TREE_INT_CST_LOW (bound)); + else + add_AT_int (subrange_die, bound_attr, TREE_INT_CST_LOW (bound)); } - else if (prec == HOST_BITS_PER_WIDE_INT - || TREE_INT_CST_HIGH (bound) == 0) - add_AT_unsigned (subrange_die, bound_attr, - TREE_INT_CST_LOW (bound)); else + /* Otherwise represent the bound as an unsigned value with + the precision of its type. The precision and signedness + of the type will be necessary to re-interpret it + unambiguously. */ add_AT_double (subrange_die, bound_attr, TREE_INT_CST_HIGH (bound), TREE_INT_CST_LOW (bound)); }