On Thu, Jan 13, 2022 at 2:53 AM Jiang, Haochen <haochen.ji...@intel.com> wrote:
>
> Hi Uros,
>
> Has fixed that format issue with this new patch. Ok for trunk?

The patch was already approved in my previous message, so no need to
re-approve it. I'm sure you are able to move one brace to a new
position without another review. ;)

Uros.

> Thx,
> Haochen
>
> -----Original Message-----
> From: Uros Bizjak <ubiz...@gmail.com>
> Sent: Thursday, January 13, 2022 3:22 AM
> To: Jiang, Haochen <haochen.ji...@intel.com>
> Cc: gcc-patches@gcc.gnu.org; Liu, Hongtao <hongtao....@intel.com>
> Subject: Re: [PATCH] [i386] Optimize a ^ ((a ^ b) & mask) to (~mask & a) | (b 
> & mask).
>
> On Wed, Jan 12, 2022 at 9:11 AM Haochen Jiang <haochen.ji...@intel.com> wrote:
> >
> > Hi all,
> >
> > This patch targets PR94790, which change the instruction selection under 
> > the following circumstance.
> >
> > Regtested on x86_64-pc-linux-gnu. Ok for trunk?
>
> Please also test with -m32, e.g.:
>
> make -j 12 -k check RUNTESTFLAGS="--target_board=unix\{,-m32\}"
>
> OK (with an it below), if new testcases do not FAIL with -m32.
>
> Thanks,
> Uros.
>
> >
> > BRs,
> > Haochen
> >
> > From the perspective of the pipeline, `andn + and + ior` version take
> > 2 cycles(AND and ANDN doesn't have dependence), but xor + and + xor
> > will take 3 cycles.
> >
> > -       xorl    %edi, %esi
> >         andl    %edx, %esi
> > -       movl    %esi, %eax
> > -       xorl    %edi, %eax
> > +       andn    %edi, %edx, %eax
> > +       orl     %esi, %eax
> >
> > gcc/ChangeLog:
> >
> >         PR taeget/94790
> >         * config/i386/i386.md (*xor2andn): New define_insn_and_split.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         PR taeget/94790
> >         * gcc.target/i386/pr94790-1.c: New test.
> >         * gcc.target/i386/pr94790-2.c: Ditto.
> > ---
> >  gcc/config/i386/i386.md                   | 39 +++++++++++++++++++++++
> >  gcc/testsuite/gcc.target/i386/pr94790-1.c | 14 ++++++++
> > gcc/testsuite/gcc.target/i386/pr94790-2.c |  9 ++++++
> >  3 files changed, 62 insertions(+)
> >  create mode 100755 gcc/testsuite/gcc.target/i386/pr94790-1.c
> >  create mode 100755 gcc/testsuite/gcc.target/i386/pr94790-2.c
> >
> > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index
> > 9b424a3935b..38efc6d5837 100644
> > --- a/gcc/config/i386/i386.md
> > +++ b/gcc/config/i386/i386.md
> > @@ -10452,6 +10452,45 @@
> >     (set_attr "znver1_decode" "double")
> >     (set_attr "mode" "DI")])
> >
> > +;; PR target/94790: Optimize a ^ ((a ^ b) & mask) to (~mask & a) | (b
> > +& mask) (define_insn_and_split "*xor2andn"
> > +  [(set (match_operand:SWI248 0 "nonimmediate_operand")
> > +       (xor:SWI248
> > +         (and:SWI248
> > +           (xor:SWI248
> > +             (match_operand:SWI248 1 "nonimmediate_operand")
> > +             (match_operand:SWI248 2 "nonimmediate_operand"))
> > +           (match_operand:SWI248 3 "nonimmediate_operand"))
> > +         (match_dup 1)))
> > +    (clobber (reg:CC FLAGS_REG))]
> > +  "(TARGET_BMI || TARGET_AVX512BW)
> > +   && ix86_pre_reload_split ()"
> > +  "#"
> > +  "&& 1"
> > +  [(parallel [(set (match_dup 4)
> > +               (and:SWI248
> > +                 (not:SWI248
> > +                   (match_dup 3))
> > +                 (match_dup 1)))
> > +             (clobber (reg:CC FLAGS_REG))])
> > +   (parallel [(set (match_dup 5)
> > +               (and:SWI248
> > +                 (match_dup 2)
> > +                 (match_dup 3)))
> > +             (clobber (reg:CC FLAGS_REG))])
> > +   (parallel [(set (match_dup 0)
> > +               (ior:SWI248
> > +                 (match_dup 4)
> > +                 (match_dup 5)))
> > +             (clobber (reg:CC FLAGS_REG))])]
> > +  {
> > +    operands[1] = force_reg (<MODE>mode, operands[1]);
> > +    operands[3] = force_reg (<MODE>mode, operands[3]);
> > +    operands[4] = gen_reg_rtx (<MODE>mode);
> > +    operands[5] = gen_reg_rtx (<MODE>mode);
> > +  }
> > +)
>
> Please put brace just after the curved brace, see numerous examples in .md 
> files.
>
> > +
> >  ;; See comment for addsi_1_zext why we do use nonimmediate_operand
> > (define_insn "*<code>si_1_zext"
> >    [(set (match_operand:DI 0 "register_operand" "=r") diff --git
> > a/gcc/testsuite/gcc.target/i386/pr94790-1.c
> > b/gcc/testsuite/gcc.target/i386/pr94790-1.c
> > new file mode 100755
> > index 00000000000..6ebbec15cfd
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr94790-1.c
> > @@ -0,0 +1,14 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -mbmi" } */
> > +/* { dg-final { scan-assembler-times "andn\[ \\t\]" 2 } } */
> > +/* { dg-final { scan-assembler-not "xorl\[ \\t\]" } } */
> > +
> > +unsigned r1(unsigned a, unsigned b, unsigned mask) {
> > +  return a ^ ((a ^ b) & mask);
> > +}
> > +
> > +unsigned r2(unsigned a, unsigned b, unsigned mask) {
> > +  return (~mask & a) | (b & mask);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/i386/pr94790-2.c
> > b/gcc/testsuite/gcc.target/i386/pr94790-2.c
> > new file mode 100755
> > index 00000000000..d7b0eec5bef
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr94790-2.c
> > @@ -0,0 +1,9 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -mbmi" } */
> > +/* { dg-final { scan-assembler-not "andn\[ \\t\]" } } */
> > +/* { dg-final { scan-assembler-times "xorl\[ \\t\]" 2 } } */
> > +
> > +unsigned r1(unsigned a, unsigned b, unsigned mask) {
> > +  return a ^ ((a ^ b) & mask) + (a ^ b); }
> > --
> > 2.18.1
> >

Reply via email to