On Fri, May 13, 2022 at 01:20:30PM -0500, will schmidt wrote:
> On Fri, 2022-05-13 at 12:17 -0400, Michael Meissner wrote:
> > Optimize multiply/add of DImode extended to TImode, PR target/103109.
> >
> > On power9 and power10 systems, we have instructions that support doing
> > 64-bit integers converted to 128-bit integers and producing 128-bit
> > results. This patch adds support to generate these instructions.
> >
> > Previously GCC had define_expands to handle conversion of the 64-bit
> > extend to 128-bit and multiply. This patch changes these define_expands
> > to define_insn_and_split and then it provides combiner patterns to
> > generate thes multiply/add instructions.
> >
> > To support using this optimization on power9, this patch extends the sign
> > extend DImode to TImode to also run on power9 (added for PR
> > target/104698).
> >
> > This patch needs the previous patch to add unsigned DImode to TImode
> > conversion so that the combiner can combine the extend, multiply, and add
> > instructions.
> >
> > I have built this patch on little endian power10, little endian power9, and
> > big
> > endian power8 systems. There were no regressions when I ran it. Can I
> > install
> > this patch into the GCC 13 master branch?
> >
> > 2022-05-13 Michael Meissner <meiss...@linux.ibm.com>
> >
> > gcc/
> > PR target/103109
> > * config/rs6000/rs6000.md (su_int32): New code attribute.
> > (<u>mul<mode><dmode>3): Convert from define_expand to
> > define_insn_and_split.
> > (maddld<mode>4): Add generator function.
>
> -(define_insn "*maddld<mode>4"
> +(define_insn "maddld<mode>4"
>
> Is the removal of the "*" considering adding generator? (Thats
> terminology that I'm not immediately familiar with).
Yes. If you have a pattern:
(define_insn "foosi2"
[(set (match_operand:SI 0 "register_operand" "=r")
(foo:SI (match_operand:SI 1 "register_operand" "r")))]
""
"foo %0,%1")
It creates a 'gen_foosi2' function that has 2 arguments, and it makes the insn
listed.
It then has support for insn recognition and output.
If the pattern starts with a '*', there is no 'gen_foosi2' function created,
but the insn recognitiion and output are still done.
In practice, you typically use the '*' names for patterns that are used as the
targets of combination, or separate insns for different machines.
Here is the verbage from rtl.texi:
These names serve one of two purposes. The first is to indicate that the
instruction performs a certain standard job for the RTL-generation
pass of the compiler, such as a move, an addition, or a conditional
jump. The second is to help the target generate certain target-specific
operations, such as when implementing target-specific intrinsic functions.
It is better to prefix target-specific names with the name of the
target, to avoid any clash with current or future standard names.
The absence of a name is indicated by writing an empty string
where the name should go. Nameless instruction patterns are never
used for generating RTL code, but they may permit several simpler insns
to be combined later on.
For the purpose of debugging the compiler, you may also specify a
name beginning with the @samp{*} character. Such a name is used only
for identifying the instruction in RTL dumps; it is equivalent to having
a nameless pattern for all other purposes. Names beginning with the
@samp{*} character are not required to be unique.
> > diff --git a/gcc/testsuite/gcc.target/powerpc/pr103109.c
> > b/gcc/testsuite/gcc.target/powerpc/pr103109.c
> > new file mode 100644
> > index 00000000000..ae2cfb9eda7
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/powerpc/pr103109.c
> > @@ -0,0 +1,62 @@
> > +/* { dg-require-effective-target int128 } */
> > +/* { dg-require-effective-target power10_ok } */
> > +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */
> > +
> > +/* This test makes sure that GCC generates the maddhd, maddhdu, and maddld
> > + power9 instructions when doing some forms of 64-bit integers converted
> > to
> > + 128-bit integers and used with multiply/add operations. */
> > +
> > +__int128_t
> > +s_mult_add (long long a,
> > + long long b,
> > + long long c)
> > +{
> > + /* maddhd, maddld. */
> > + return ((__int128_t)a * (__int128_t)b) + (__int128_t)c;
> > +}
> > +
> > +/* Test 32-bit constants that are loaded into GPRs instead of doing the
> > + mulld/mulhd and then addic/addime or addc/addze. */
>
> addme ?
Yes, I meant addme. Good catch.
--
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meiss...@linux.ibm.com
