public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Mike Stump <mikestump@comcast.net>
Cc: Richard Sandiford <richard.sandiford@arm.com>,
	Bernd Schmidt <bschmidt@redhat.com>,
		Ulrich Weigand <uweigand@de.ibm.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [ping] Fix PR debug/66728
Date: Wed, 04 Nov 2015 12:15:00 -0000	[thread overview]
Message-ID: <CAFiYyc2DwthVCAk2TPcTnJOWyCjiuQFGXHKFw08MTNpzGPa9ug@mail.gmail.com> (raw)
In-Reply-To: <7ADD4B03-BF3E-48BA-8028-AC5EB50E773C@comcast.net>

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);
>  }
>
>

  reply	other threads:[~2015-11-04 12:15 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-21 14:24 Richard Sandiford
2015-08-21 16:20 ` Jeff Law
2015-10-28 12:04 ` [ping] " Ulrich Weigand
2015-10-28 13:14   ` Richard Sandiford
2015-10-28 14:25     ` Bernd Schmidt
2015-10-28 14:58       ` Ulrich Weigand
2015-11-02 15:30       ` Richard Sandiford
2015-11-02 16:29         ` Richard Sandiford
2015-11-02 20:34           ` [ping] " Mike Stump
2015-11-02 20:55             ` Richard Sandiford
2015-11-02 23:29               ` Mike Stump
2015-11-03  8:46                 ` Richard Sandiford
2015-11-03 21:59                   ` Mike Stump
2015-11-04  9:43                     ` Richard Biener
2015-11-04 11:58                       ` Mike Stump
2015-11-04 12:15                         ` Richard Biener [this message]
2015-11-04 19:36                           ` Mike Stump
2015-11-04 20:51                             ` Richard Sandiford
2015-11-04 23:45                               ` Mike Stump
2015-11-05 12:32                                 ` Richard Biener
2015-11-06  1:35                                   ` Mike Stump
2015-11-06 13:06                                     ` Richard Biener
2015-11-09 18:47                                       ` Mike Stump

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFiYyc2DwthVCAk2TPcTnJOWyCjiuQFGXHKFw08MTNpzGPa9ug@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=bschmidt@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=mikestump@comcast.net \
    --cc=richard.sandiford@arm.com \
    --cc=uweigand@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).