On Wed, Nov 4, 2015 at 12:57 PM, Mike Stump <[email protected]> wrote:
> On Nov 4, 2015, at 1:43 AM, Richard Biener <[email protected]> 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);
> }
>
>