public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Uros Bizjak <ubizjak@gmail.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH, i386]: Do not limit the cost of moves to/from XMM register to minimum 8.
Date: Fri, 30 Aug 2019 08:29:00 -0000	[thread overview]
Message-ID: <CAFULd4aoX-JqbkFECYSMHgCEx2zL=WkDFGR0ZrE4a5sywYW3Zw@mail.gmail.com> (raw)
In-Reply-To: <CAFiYyc1HOHw0RTXKP32OpsgNqAHXyZoHOp7PWFV8Zc_LYpLb4Q@mail.gmail.com>

On Fri, Aug 30, 2019 at 9:22 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Thu, Aug 29, 2019 at 9:54 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Wed, Aug 28, 2019 at 5:12 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> > >
> > > Attached patch improves costing for STV shifts and corrects reject
> > > condition for out of range shift count operands.
> > >
> > > 2019-08-28  Uroš Bizjak  <ubizjak@gmail.com>
> > >
> > >     * config/i386/i386-features.c
> > >     (general_scalar_chain::compute_convert_gain):
> > >     Correct cost for double-word shifts.
> > >     (general_scalar_to_vector_candidate_p): Reject count operands
> > >     greater or equal to mode bitsize.
> > >
> > > Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> > >
> > > Committed to mainline SVN.
> >
> > Ouch... I mixed up patches and actually committed the patch that
> > removes maximum from cost of sse<->int moves.
> >
> > I can leave the patch for a day, so we can see the effects of the cost
> > change, and if the patch creates problems, I'll revert it.
>
> Regresses gromacs and namd quite a bit on Haswell, also perl in SPEC 2000.
> I guess we should try understand why rather than reverting immediately
> (I'd leave it in at least another few days to get more testers pick up the
> rev.).

The correct patch is actually [1] (I have updated the subject):

2019-08-28  Uroš Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.c (ix86_register_move_cost): Do not
    limit the cost of moves to/from XMM register to minimum 8.

There is no technical reason to limit the costs to minimum 8 anymore,
and several targets (e.g skylake) also claim that moves between SSE
and general regs are as cheap as moves between general regs. However,
these values were never benchmarked properly due to the mentioned
limitation and apparently cause some unwanted secondary effects
(lowering the clock).

I agree with your proposal to leave the change in the tree some more
time. At the end, we could raise the costs for all targets to 8 to
effectively revert to the previous state.

[1] https://gcc.gnu.org/ml/gcc-patches/2019-08/msg02009.html

Uros.

  reply	other threads:[~2019-08-30  7:42 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 16:12 [PATCH, i386]: Improve STV conversion of shifts Uros Bizjak
2019-08-29  8:49 ` Uros Bizjak
2019-08-29 10:11   ` [PATCH, i386]: Fix secondary_reload_needed (was: Re: [PATCH, i386]: Improve STV conversion of shifts) Uros Bizjak
2019-08-29 10:17     ` Uros Bizjak
2019-08-30  7:40   ` [PATCH, i386]: Improve STV conversion of shifts Richard Biener
2019-08-30  8:29     ` Uros Bizjak [this message]
2019-08-31  3:01       ` [PATCH, i386]: Do not limit the cost of moves to/from XMM register to minimum 8 Alan Modra
2019-08-31 16:12         ` Richard Biener
2019-09-01 19:49           ` Uros Bizjak
2019-09-02  7:16             ` Alan Modra
2019-09-02  8:13             ` Hongtao Liu
2019-09-02  8:41               ` Uros Bizjak
2019-09-03  7:57                 ` Hongtao Liu
2019-09-03 11:24                   ` Richard Biener
2019-09-03 11:33                     ` Richard Biener
2019-09-03 16:50                       ` Uros Bizjak
2019-09-04  1:42                         ` Hongtao Liu
2019-09-05  5:47                           ` Hongtao Liu
2019-09-05  8:54                             ` Uros Bizjak
2019-09-06 19:32                               ` [PATCH, i386]: Fix PR 91654: Runtime SPEC regression on Haswell Uros Bizjak
2019-09-02 10:23               ` [PATCH, i386]: Do not limit the cost of moves to/from XMM register to minimum 8 Richard Biener
2019-09-03  1:37                 ` Hongtao Liu
2019-08-29 20:01 Uros Bizjak
2019-08-30  7:12 ` Hongtao Liu
2019-08-30  7:16   ` Hongtao Liu
2019-08-30  7:22   ` Uros Bizjak
2019-08-30  7:30     ` Hongtao Liu

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='CAFULd4aoX-JqbkFECYSMHgCEx2zL=WkDFGR0ZrE4a5sywYW3Zw@mail.gmail.com' \
    --to=ubizjak@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.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).