public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Pat Haugen <pthaugen@linux.ibm.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
	"Kewen.Lin" <linkw@linux.ibm.com>,
	David Edelsohn <dje.gcc@gmail.com>,
	Peter Bergner <bergner@linux.ibm.com>
Subject: Re: [PATCH, rs6000] Tweak modulo define_insns to eliminate register copy
Date: Mon, 27 Feb 2023 14:53:31 -0600	[thread overview]
Message-ID: <20230227205331.GC25951@gate.crashing.org> (raw)
In-Reply-To: <20578dd1-fba8-858a-a6e5-cdbb3ca0b6c1@linux.ibm.com>

Hi!

On Mon, Feb 27, 2023 at 02:12:23PM -0600, Pat Haugen wrote:
> On 2/27/23 11:08 AM, Segher Boessenkool wrote:
> >On Mon, Feb 27, 2023 at 09:11:37AM -0600, Pat Haugen wrote:
> >>The define_insns for the modulo operation currently force the target
> >>register
> >>to a distinct reg in preparation for a possible future peephole combining
> >>div/mod. But this can lead to cases of a needless copy being inserted. 
> >>Fixed
> >>with the following patch.
> >
> >Have you verified those peepholes still match?
> 
> Yes, I verified the peepholes still match and transform the sequence.

Please add the testcases for that then?  Or do we have tests for it
already :-)

> >Do those peepholes actually improve performance?  On new CPUs?  The code
> >here says
> >;; On machines with modulo support, do a combined div/mod the old fashioned
> >;; method, since the multiply/subtract is faster than doing the mod 
> >instruction
> >;; after a divide.
> >but that really should not be true: we can do the div and mod in
> >parallel (except in SMT4 perhaps, which we never schedule for anyway),
> >so that should always be strictly faster.
> >
> Since the modulo insns were introduced in Power9, we're just talking 
> Power9/Power10. On paper, I would agree that separate div/mod could be 
> slightly faster to get the mod result,

"Slightly".  It takes 12 cycles for the two in parallel (64-bit, p9),
but 17 cycles for the "cheaper" sequence (divd+mulld+subf, 12+5+2).  It
is all worse if the units are busy of course, or if there are other
problems.

> but if you throw in another 
> independent div or mod in the insn stream then doing the peephole should 
> be a clear win since that 3rd insn can execute in parallel with the 
> initial divide as opposed to waiting for the one of the first div/mod to 
> clear the exclusive stage of the pipe.

That is the SMT4 case, the one we do not optimise for.  SMT2 and ST can
do four in parallel.  This means you can start a div or mod every 2nd
cycle on average, so it is very unlikely you will ever be limited by
this on real code.


Segher

  reply	other threads:[~2023-02-27 20:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-27 15:11 Pat Haugen
2023-02-27 17:08 ` Segher Boessenkool
2023-02-27 20:12   ` Pat Haugen
2023-02-27 20:53     ` Segher Boessenkool [this message]
2023-02-27 22:03       ` Pat Haugen
2023-02-27 22:30         ` Segher Boessenkool

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=20230227205331.GC25951@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=linkw@linux.ibm.com \
    --cc=pthaugen@linux.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).