On Wed, Nov 4, 2015 at 12:57 PM, Mike Stump <mikest...@comcast.net> wrote:
> On Nov 4, 2015, at 1:43 AM, Richard Biener <richard.guent...@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);
>  }
>
>

Reply via email to