[Bug c++/98461] New: Suboptimal codegen for negating a movemask

[Bug c++/98461] New: Suboptimal codegen for negating a movemask

[denis.yaroshevskij at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

            Bug ID: 98461
           Summary: Suboptimal codegen for negating a movemask
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: denis.yaroshevskij at gmail dot com
  Target Milestone: ---

https://godbolt.org/z/vK85Ma

GCC fails to see that `not` can be generated instead of vector instructions.
This comes up in generic simd code.

[Bug c++/98461] Suboptimal codegen for negating a movemask

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
            Version|unknown                     |10.2.0
             Target|                            |i386,x86-64
                 CC|                            |crazylht at gmail dot com,
                   |                            |hjl.tools at gmail dot com
   Last reconfirmed|                            |2020-12-28

[Bug c++/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #1 from Hongtao.liu <crazylht at gmail dot com> ---
in rtl phase, gcc successfully simplify 
(and:V4DI (not:V4DI (reg:V4DI 91)) 
          (constm1_rtx)
to

(not:V4DI (reg:V4DI 91)).

But combine failed to match 

(set (reg:SI 88)
    (unspec:SI [
            (subreg:V32QI (not:V4DI (reg:V4DI 91)) 0)
        ] UNSPEC_MOVMSK))

So add combine splitter?

[Bug c++/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #2 from Hongtao.liu <crazylht at gmail dot com> ---
> So add combine splitter?

I'm testing this patch

1 file changed, 15 insertions(+), 1 deletion(-)
gcc/config/i386/sse.md | 16 +++++++++++++++-

modified   gcc/config/i386/sse.md
@@ -640,7 +640,8 @@ (define_mode_attr ssedoublemode

 (define_mode_attr ssebytemode
   [(V8DI "V64QI") (V4DI "V32QI") (V2DI "V16QI")
-   (V16SI "V64QI") (V8SI "V32QI") (V4SI "V16QI")])
+   (V16SI "V64QI") (V8SI "V32QI") (V4SI "V16QI")
+   (V32HI "V64QI") (V16HI "V32QI") (V8HI "V16QI")])

 ;; All 128bit vector integer modes
 (define_mode_iterator VI_128 [V16QI V8HI V4SI V2DI])
@@ -16065,6 +16066,19 @@ (define_insn "<sse2_avx2>_pmovmskb"
    (set_attr "prefix" "maybe_vex")
    (set_attr "mode" "SI")])

+(define_split
+  [(set (match_operand:SI 0 "register_operand")
+       (unspec:SI
+         [(subreg:<ssebytemode>
+            (not:VI248_AVX2
+              (match_operand:VI248_AVX2 1 "register_operand")) 0)]
+         UNSPEC_MOVMSK))]
+  "TARGET_SSE2"
+  [(set (match_dup 0)
+       (unspec:SI [(match_dup 2)] UNSPEC_MOVMSK))
+   (set (match_dup 0) (not:SI (match_dup 0)))]
+  "operands[2] = lowpart_subreg (<ssebytemode>mode, operands[1],
<MODE>mode);")
+
 (define_insn "*<sse2_avx2>_pmovmskb_zext"
   [(set (match_operand:DI 0 "register_operand" "=r")
        (zero_extend:DI

[Bug c++/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #3 from Hongtao.liu <crazylht at gmail dot com> ---
Hi  Denis Yaroshevskiy:
Could you mark this bug as blocks PR98375 
(This metabug is used to track all the patches which have been written during 
Stage 3 of GCC 11 but do not qualify for that stage, and are waiting for Stage 
1 of GCC 12 to be applied.)

[Bug c++/98461] Suboptimal codegen for negating a movemask

[denis.yaroshevskij at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #4 from Denis Yaroshevskiy <denis.yaroshevskij at gmail dot com> ---
(In reply to Hongtao.liu from comment #3)
> Could you mark this bug as blocks PR98375 

Done?

> ... waiting for Stage 1 of GCC 12 to be applied.)

So. the next gcc should have a fix? Fantastic! Will it also work for 128 bit
registers?

[Bug target/98461] Suboptimal codegen for negating a movemask

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49854
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49854&action=edit
gcc11-pr98461.patch

Untested fix.

[Bug target/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Jakub Jelinek from comment #5)
> Created attachment 49854 [details]
> gcc11-pr98461.patch
> 
> Untested fix.

+  if (GET_MODE_NUNITS (<MODE>mode) == 32)

Yes, and i missed this part.

[Bug target/98461] Suboptimal codegen for negating a movemask

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:8f7941ca37001773a36add8119791725aeb823ba

commit r11-6367-g8f7941ca37001773a36add8119791725aeb823ba
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Dec 30 11:21:24 2020 +0100

    i386: Optimize pmovmskb on inverted vector to inversion of pmovmskb result
[PR98461]

    The following patch adds combine splitters to optimize:
    -       vpcmpeqd        %ymm1, %ymm1, %ymm1
    -       vpandn  %ymm1, %ymm0, %ymm0
            vpmovmskb       %ymm0, %eax
    +       notl    %eax
    etc. (for vectors with less than 32 elements with xorl instead of notl).

    2020-12-30  Jakub Jelinek  <jakub@redhat.com>

            PR target/98461
            * config/i386/sse.md (<sse2_avx2>_pmovmskb): Add splitters
            for pmovmskb of NOT vector.

            * gcc.target/i386/sse2-pr98461.c: New test.
            * gcc.target/i386/avx2-pr98461.c: New test.

[Bug target/98461] Suboptimal codegen for negating a movemask

[denis.yaroshevskij at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #8 from Denis Yaroshevskiy <denis.yaroshevskij at gmail dot com> ---
Thank you for the fast fix.

I can already see that the code is in trunk and works for both 256 and 128 bit
registers.

256: https://godbolt.org/z/5sT48f
128: https://godbolt.org/z/Exo3d9

I am a bit confused as to why 128 bit codegen differs in two cases but both
seem to be reasonable at glance.

[Bug target/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #9 from Hongtao.liu <crazylht at gmail dot com> ---
> 128: https://godbolt.org/z/Exo3d9

zero_extend should be redudant, add another combine splitter for the bellow?

Failed to match this instruction:
(set (reg:SI 91 [ _6 ])
    (zero_extend:SI (subreg:HI (unspec:SI [
                    (subreg:V16QI (reg:V2DI 92) 0)
                ] UNSPEC_MOVMSK) 0)))

[Bug target/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #10 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Hongtao.liu from comment #9)
> > 128: https://godbolt.org/z/Exo3d9
> 
> zero_extend should be redudant, add another combine splitter for the bellow?

Pass_combine failed to match combine_splitter with i0, i1 as NULL, so add
define_insn_and_split.

[Bug target/98461] Suboptimal codegen for negating a movemask

[denis.yaroshevskij at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #11 from Denis Yaroshevskiy <denis.yaroshevskij at gmail dot com> ---
Previous 128bit contained a bug.
Here is an updated one: https://godbolt.org/z/6vdxro

There is still a bit of a difference between 2 versions, not sure if important.

[Bug target/98461] Suboptimal codegen for negating a movemask

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:bea984814c6fcd056dc80c99805925eb19a591b9

commit r11-6462-gbea984814c6fcd056dc80c99805925eb19a591b9
Author: liuhongt <hongtao.liu@intel.com>
Date:   Mon Jan 4 11:24:30 2021 +0800

    i386: Optimize pmovskb on zero_extend of subreg HI of pmovskb result
[PR98461]

    The following patch adds define_insn_and_split to optimize

           vpmovmskb       %xmm0, %eax
    -       movzwl  %ax, %eax
            notl    %eax

    and combine splitter to optimize

            pmovmskb        %xmm0, %eax
    -       notl    %eax
    -       movzwl  %ax, %eax
    +       xorl    $65535, %eax

    gcc/ChangeLog
            PR target/98461
            * config/i386/sse.md (*sse2_pmovskb_zexthisi): New
            define_insn_and_split for zero_extend of subreg HI of pmovskb
            result.
            (*sse2_pmovskb_zexthisi): Add new combine splitters for
            zero_extend of not of subreg HI of pmovskb result.

    gcc/testsuite/ChangeLog
            * gcc.target/i386/sse2-pr98461-2.c: New test.

[Bug target/98461] Suboptimal codegen for negating a movemask

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #13 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Denis Yaroshevskiy from comment #11)
> Previous 128bit contained a bug.
> Here is an updated one: https://godbolt.org/z/6vdxro
> 
> There is still a bit of a difference between 2 versions, not sure if
> important.

Should be fixed in the upper commit.

[Bug target/98461] Suboptimal codegen for negating a movemask

[denis.yaroshevskij at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #14 from Denis Yaroshevskiy <denis.yaroshevskij at gmail dot com> ---
(In reply to Hongtao.liu from comment #13)
> (In reply to Denis Yaroshevskiy from comment #11)
> > Previous 128bit contained a bug.
> > Here is an updated one: https://godbolt.org/z/6vdxro
> > 
> > There is still a bit of a difference between 2 versions, not sure if
> > important.
> 
> Should be fixed in the upper commit.

Fixed now

[Bug target/98461] Suboptimal codegen for negating a movemask

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

--- Comment #15 from H.J. Lu <hjl.tools at gmail dot com> ---
Is this fixed now?

[Bug target/98461] Suboptimal codegen for negating a movemask

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I hope so.

[Bug target/98461] Suboptimal codegen for negating a movemask

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98461

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0