| Message ID | 7ADD4B03-BF3E-48BA-8028-AC5EB50E773C@comcast.net |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 4, 2015 at 12:57 PM, Mike Stump <mikestump@comcast.net> wrote: > On Nov 4, 2015, at 1:43 AM, Richard Biener <richard.guenther@gmail.com> wrote: >> I think you should limit the effect of this patch to the dwarf2out use >> as the above doesn't make sense to me. > > Since dwarf is so special, and since other clients already do something sort of like this anyway, it isn’t unreasonable to make the client be responsible for picking a sensible mode, and asserting if they fail to. This also transfers the cost of the special case code out to the one client that needs it, and avoids that cost for all the other clients. > > The new patch is below for your consideration. > > Ok? > >> Ideally we'd have an assert that you don't create a rtx_mode_t with >> VOIDmode or BLKmode. > > Added. > >> Handling the CONST_WIDE_INT in dwarf2out.c the same as we did before >> (with CONST_DOUBLE) >> looks sensible as far of fixing a regression (I assume the diff to the >> dwarf results in essentially the >> same DWARF as what was present before wide-int). > > Yes, the dwarf is the same. > > Index: dwarf2out.c > =================================================================== > --- dwarf2out.c (revision 229720) > +++ dwarf2out.c (working copy) > @@ -15593,8 +15593,15 @@ > return true; > > case CONST_WIDE_INT: > - add_AT_wide (die, DW_AT_const_value, > - std::make_pair (rtl, GET_MODE (rtl))); > + { > + machine_mode mode = GET_MODE (rtl); > + if (mode == VOIDmode) > + mode = mode_for_size (CONST_WIDE_INT_NUNITS (rtl) > + * HOST_BITS_PER_WIDE_INT, > + MODE_INT, 0); > + add_AT_wide (die, DW_AT_const_value, > + std::make_pair (rtl, mode)); I wonder if we'll manage to to get mode_for_size return BLKmode in case of an original mode that was not of a size multiple of HOST_BITS_PER_WIDE_INT (and that's host dependent even...). We probably should use smallest_mode_for_size on a precision derived from the value (ignore leading ones and zeros or so, exact details need to be figured out). Eventually hide this detail in a smallest_mode_for_const_wide_int () helper. > + } > return true; > > case CONST_DOUBLE: > Index: rtl.h > =================================================================== > --- rtl.h (revision 229720) > +++ rtl.h (working copy) > @@ -2086,6 +2086,7 @@ > inline unsigned int > wi::int_traits <rtx_mode_t>::get_precision (const rtx_mode_t &x) > { > + gcc_assert (x.second != BLKmode && x.second != VOIDmode); Please use gcc_checking_assert here. Richard. > return GET_MODE_PRECISION (x.second); > } > >
Index: dwarf2out.c =================================================================== --- dwarf2out.c (revision 229720) +++ dwarf2out.c (working copy) @@ -15593,8 +15593,15 @@ return true; case CONST_WIDE_INT: - add_AT_wide (die, DW_AT_const_value, - std::make_pair (rtl, GET_MODE (rtl))); + { + machine_mode mode = GET_MODE (rtl); + if (mode == VOIDmode) + mode = mode_for_size (CONST_WIDE_INT_NUNITS (rtl) + * HOST_BITS_PER_WIDE_INT, + MODE_INT, 0); + add_AT_wide (die, DW_AT_const_value, + std::make_pair (rtl, mode)); + } return true; case CONST_DOUBLE: Index: rtl.h =================================================================== --- rtl.h (revision 229720) +++ rtl.h (working copy) @@ -2086,6 +2086,7 @@ inline unsigned int wi::int_traits <rtx_mode_t>::get_precision (const rtx_mode_t &x) { + gcc_assert (x.second != BLKmode && x.second != VOIDmode); return GET_MODE_PRECISION (x.second); }