[Bug target/101142] New: [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[Bug target/101142] New: [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[crazylht at gmail dot com]

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

            Bug ID: 101142
           Summary: [11/12 regression] Regression due to  supporting
                    bitwise operators on AVX512 masks.
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: crazylht at gmail dot com
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu
            Target: x86_64-*-* i?86-*-*

Created attachment 51040
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51040&action=edit
test.cpp

options to reproduce regression: g++ byteswap.cpp test.cpp -march=native -O3
-DDTYPE32

The regression is due to r11-2796-g388cb292a94f98a276548cd6ce01285cf36d17df
which supports bitwise operator for avx512 masks. In byteswap.cpp there're a
lot of bitwise operations, the register pressure is very high, so LRA allocate
some bitwise operations to mask registers to avoid spills, the problem is mask
bitwise instructions has 1/4 throught of those gpr versions which causes the
regression.

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[crazylht at gmail dot com]

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

--- Comment #1 from Hongtao.liu <crazylht at gmail dot com> ---
Created attachment 51041
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51041&action=edit
byteswap.cpp

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[crazylht at gmail dot com]

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

--- Comment #2 from Hongtao.liu <crazylht at gmail dot com> ---
I'm working on a patch which disparages slightly the mask register alternative
for bitwise operations(using "?k" in alternatives). It can prevent mask bitwise
instruction generation when the input is not allocate as mask registers.

Also when allocano cost of GENERAL_REGS is same as MASK_REGS, allocate
MASK_REGS first since it has already been disparaged. This is for testcase like
below where the input is allocated as mask registers, then mask bitwise
instructions should be used here.

#include<immintrin.h>
volatile __mmask8 foo;
void
foo_orb (__m512i a, __m512i b, __m512i c, __m512i d)
{
  __mmask8 m1 = _mm512_cmp_epi64_mask (a, b, 2);
  __mmask8 m2 = _mm512_cmp_epi64_mask (c, d, 4);
  foo = m1 | m2;
}

        vpcmpq  $2, %zmm1, %zmm0, %k0
        vpcmpq  $4, %zmm3, %zmm2, %k1
        korb    %k1, %k0, %k2
        kmovb   %k2, foo(%rip)
        ret
foo:

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.2
           Keywords|                            |missed-optimization
   Last reconfirmed|                            |2021-06-21
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The exact command line to hit this issue is:
 -O3 -march=skylake-avx512

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[crazylht at gmail dot com]

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

--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Andrew Pinski from comment #3)
> The exact command line to hit this issue is:
>  -O3 -march=skylake-avx512

Yes, thanks for the clarification.

g++ byteswap.cpp test.cpp -march=skylake-avx512 -O3 -DDTYPE32

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:08c85f609a73fb36fdcbd9f327a5a645c20ac816

commit r12-1669-g08c85f609a73fb36fdcbd9f327a5a645c20ac816
Author: liuhongt <hongtao.liu@intel.com>
Date:   Tue Jun 15 16:25:16 2021 +0800

    Disparage slightly the mask register alternative for bitwise operations.

    The avx512 supports bitwise operations with mask registers, but the
    throughput of those instructions is much lower than that of the
    corresponding gpr version, so we would additionally disparages
    slightly the mask register alternative for bitwise operations in the
    LRA.

    Also when allocano cost of GENERAL_REGS is same as MASK_REGS, allocate
    MASK_REGS first since it has already been disparaged.

    gcc/ChangeLog:

            PR target/101142
            * config/i386/i386.md: (*anddi_1): Disparage slightly the mask
            register alternative.
            (*and<mode>_1): Ditto.
            (*andqi_1): Ditto.
            (*andn<mode>_1): Ditto.
            (*<code><mode>_1): Ditto.
            (*<code>qi_1): Ditto.
            (*one_cmpl<mode>2_1): Ditto.
            (*one_cmplsi2_1_zext): Ditto.
            (*one_cmplqi2_1): Ditto.
            * config/i386/i386.c (x86_order_regs_for_local_alloc): Change
            the order of mask registers to be before general registers.

    gcc/testsuite/ChangeLog:

            PR target/101142
            * gcc.target/i386/spill_to_mask-1.c: Adjust testcase.
            * gcc.target/i386/spill_to_mask-2.c: Adjust testcase.
            * gcc.target/i386/spill_to_mask-3.c: Adjust testcase.
            * gcc.target/i386/spill_to_mask-4.c: Adjust testcase.

[Bug target/101142] [11/12 regression] Regression due to supporting bitwise operators on AVX512 masks.

[crazylht at gmail dot com]

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

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
Not sure backport to GCC11, this patch only modifies the cost model of RA on
the back end, no functional changes.

[Bug target/101142] [11 regression] Regression due to supporting bitwise operators on AVX512 masks.

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r11-8737-gc34da273aa1f3f2f5457c43dd815fd0ee8c3b627
Author: liuhongt <hongtao.liu@intel.com>
Date:   Tue Jun 15 16:25:16 2021 +0800

    Disparage slightly the mask register alternative for bitwise operations.

    The avx512 supports bitwise operations with mask registers, but the
    throughput of those instructions is much lower than that of the
    corresponding gpr version, so we would additionally disparages
    slightly the mask register alternative for bitwise operations in the
    LRA.

    Also when allocano cost of GENERAL_REGS is same as MASK_REGS, allocate
    MASK_REGS first since it has already been disparaged.

    gcc/ChangeLog:

            PR target/101142
            * config/i386/i386.md: (*anddi_1): Disparage slightly the mask
            register alternative.
            (*and<mode>_1): Ditto.
            (*andqi_1): Ditto.
            (*andn<mode>_1): Ditto.
            (*<code><mode>_1): Ditto.
            (*<code>qi_1): Ditto.
            (*one_cmpl<mode>2_1): Ditto.
            (*one_cmplsi2_1_zext): Ditto.
            (*one_cmplqi2_1): Ditto.
            * config/i386/i386.c (x86_order_regs_for_local_alloc): Change
            the order of mask registers to be before general registers.

    gcc/testsuite/ChangeLog:

            PR target/101142
            * gcc.target/i386/spill_to_mask-1.c: Adjust testcase.
            * gcc.target/i386/spill_to_mask-2.c: Adjust testcase.
            * gcc.target/i386/spill_to_mask-3.c: Adjust testcase.
            * gcc.target/i386/spill_to_mask-4.c: Adjust testcase.

[Bug target/101142] [11 regression] Regression due to supporting bitwise operators on AVX512 masks.

[crazylht at gmail dot com]

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

--- Comment #8 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Hongtao.liu from comment #6)
> Not sure backport to GCC11, this patch only modifies the cost model of RA on
> the back end, no functional changes.

Backport to GCC11, along with r12-1800

[Bug target/101142] [11 regression] Regression due to supporting bitwise operators on AVX512 masks.

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed then.