Message ID | 20220415042942.77387-1-hongyu.wang@intel.com |
---|---|
State | New |
Headers | show |
Series | i386: Correct target attribute for crc32 intrinsics | expand |
On Fri, Apr 15, 2022 at 6:30 AM Hongyu Wang <hongyu.wang@intel.com> wrote: > > Hi, > > Complile _mm_crc32_u8/16/32/64 intrinsics with -mcrc32 > would meet target specific option mismatch. Correct target pragma > to fix. > > Bootstrapped/regtest on x86_64-pc-linux-gnu{-m32,}. > > Ok for master and backport to GCC 11? > > gcc/ChangeLog: > > * config/i386/smmintrin.h: Correct target pragma from sse4.1 > and sse4.2 to crc32 for crc32 intrinsics. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/crc32-6.c: Adjust to call builtin. > * gcc.target/i386/crc32-7.c: New test. > --- > gcc/config/i386/smmintrin.h | 25 +++++------------- > gcc/testsuite/gcc.target/i386/crc32-6.c | 2 +- > gcc/testsuite/gcc.target/i386/crc32-7.c | 34 +++++++++++++++++++++++++ > 3 files changed, 42 insertions(+), 19 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/crc32-7.c > > diff --git a/gcc/config/i386/smmintrin.h b/gcc/config/i386/smmintrin.h > index b42b212300f..eb6a451c10a 100644 > --- a/gcc/config/i386/smmintrin.h > +++ b/gcc/config/i386/smmintrin.h > @@ -810,17 +810,11 @@ _mm_cmpgt_epi64 (__m128i __X, __m128i __Y) > > #include <popcntintrin.h> > > -#ifndef __SSE4_1__ > +#ifndef __CRC32__ > #pragma GCC push_options > -#pragma GCC target("sse4.1") > -#define __DISABLE_SSE4_1__ > -#endif /* __SSE4_1__ */ > - > -#ifndef __SSE4_2__ > -#pragma GCC push_options > -#pragma GCC target("sse4.2") > -#define __DISABLE_SSE4_2__ > -#endif /* __SSE4_1__ */ > +#pragma GCC target("crc32") > +#define __DISABLE_CRC32__ > +#endif /* __CRC32__ */ > > /* Accumulate CRC32 (polynomial 0x11EDC6F41) value. */ > extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) > @@ -849,14 +843,9 @@ _mm_crc32_u64 (unsigned long long __C, unsigned long long __V) > } > #endif > > -#ifdef __DISABLE_SSE4_2__ > -#undef __DISABLE_SSE4_2__ > +#ifdef __DISABLE_CRC32__ > +#undef __DISABLE_CRC32__ > #pragma GCC pop_options > -#endif /* __DISABLE_SSE4_2__ */ > - > -#ifdef __DISABLE_SSE4_1__ > -#undef __DISABLE_SSE4_1__ > -#pragma GCC pop_options > -#endif /* __DISABLE_SSE4_1__ */ > +#endif /* __DISABLE_CRC32__ */ > > #endif /* _SMMINTRIN_H_INCLUDED */ > diff --git a/gcc/testsuite/gcc.target/i386/crc32-6.c b/gcc/testsuite/gcc.target/i386/crc32-6.c > index 464e3444069..1f306534bb8 100644 > --- a/gcc/testsuite/gcc.target/i386/crc32-6.c > +++ b/gcc/testsuite/gcc.target/i386/crc32-6.c > @@ -7,7 +7,7 @@ > unsigned int > test_mm_crc32_u8 (unsigned int CRC, unsigned char V) > { > - return _mm_crc32_u8 (CRC, V); > + return __builtin_ia32_crc32qi (CRC, V); This test should not be changed, it correctly reports ISA mismatch. It even passes -mno-crc32. Uros. > } > > /* { dg-error "needs isa option -mcrc32" "" { target *-*-* } 0 } */ > diff --git a/gcc/testsuite/gcc.target/i386/crc32-7.c b/gcc/testsuite/gcc.target/i386/crc32-7.c > new file mode 100644 > index 00000000000..2e310e38b82 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/crc32-7.c > @@ -0,0 +1,34 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mcrc32" } */ > +/* { dg-final { scan-assembler "crc32b\[^\\n\]*eax" } } */ > +/* { dg-final { scan-assembler "crc32w\[^\\n\]*eax" } } */ > +/* { dg-final { scan-assembler "crc32l\[^\\n\]*eax" } } */ > +/* { dg-final { scan-assembler "crc32q\[^\\n\]*rax" { target { ! ia32 } } } } */ > + > +#include <immintrin.h> > + > +unsigned int > +test_mm_crc32_u8 (unsigned int CRC, unsigned char V) > +{ > + return _mm_crc32_u8 (CRC, V); > +} > + > +unsigned int > +test_mm_crc32_u16 (unsigned int CRC, unsigned short V) > +{ > + return _mm_crc32_u16 (CRC, V); > +} > + > +unsigned int > +test_mm_crc32_u32 (unsigned int CRC, unsigned int V) > +{ > + return _mm_crc32_u32 (CRC, V); > +} > + > +#ifdef __x86_64__ > +unsigned long long > +test_mm_crc32_u64 (unsigned long long CRC, unsigned long long V) > +{ > + return _mm_crc32_u64 (CRC, V); > +} > +#endif > -- > 2.18.1 >
> This test should not be changed, it correctly reports ISA mismatch. It > even passes -mno-crc32. The error message changes from "needs isa option -mcrc32" to "target specific option mismatch" with the #pragma change. I see many of our intrinsic would throw such error, it has been a long term issue for intrinsic diagnostic. So for this test either I change the dg-error message or the call to builtin, otherwise it would fail. Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> 于2022年4月15日周五 15:54写道: > > On Fri, Apr 15, 2022 at 6:30 AM Hongyu Wang <hongyu.wang@intel.com> wrote: > > > > Hi, > > > > Complile _mm_crc32_u8/16/32/64 intrinsics with -mcrc32 > > would meet target specific option mismatch. Correct target pragma > > to fix. > > > > Bootstrapped/regtest on x86_64-pc-linux-gnu{-m32,}. > > > > Ok for master and backport to GCC 11? > > > > gcc/ChangeLog: > > > > * config/i386/smmintrin.h: Correct target pragma from sse4.1 > > and sse4.2 to crc32 for crc32 intrinsics. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/i386/crc32-6.c: Adjust to call builtin. > > * gcc.target/i386/crc32-7.c: New test. > > --- > > gcc/config/i386/smmintrin.h | 25 +++++------------- > > gcc/testsuite/gcc.target/i386/crc32-6.c | 2 +- > > gcc/testsuite/gcc.target/i386/crc32-7.c | 34 +++++++++++++++++++++++++ > > 3 files changed, 42 insertions(+), 19 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/crc32-7.c > > > > diff --git a/gcc/config/i386/smmintrin.h b/gcc/config/i386/smmintrin.h > > index b42b212300f..eb6a451c10a 100644 > > --- a/gcc/config/i386/smmintrin.h > > +++ b/gcc/config/i386/smmintrin.h > > @@ -810,17 +810,11 @@ _mm_cmpgt_epi64 (__m128i __X, __m128i __Y) > > > > #include <popcntintrin.h> > > > > -#ifndef __SSE4_1__ > > +#ifndef __CRC32__ > > #pragma GCC push_options > > -#pragma GCC target("sse4.1") > > -#define __DISABLE_SSE4_1__ > > -#endif /* __SSE4_1__ */ > > - > > -#ifndef __SSE4_2__ > > -#pragma GCC push_options > > -#pragma GCC target("sse4.2") > > -#define __DISABLE_SSE4_2__ > > -#endif /* __SSE4_1__ */ > > +#pragma GCC target("crc32") > > +#define __DISABLE_CRC32__ > > +#endif /* __CRC32__ */ > > > > /* Accumulate CRC32 (polynomial 0x11EDC6F41) value. */ > > extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) > > @@ -849,14 +843,9 @@ _mm_crc32_u64 (unsigned long long __C, unsigned long long __V) > > } > > #endif > > > > -#ifdef __DISABLE_SSE4_2__ > > -#undef __DISABLE_SSE4_2__ > > +#ifdef __DISABLE_CRC32__ > > +#undef __DISABLE_CRC32__ > > #pragma GCC pop_options > > -#endif /* __DISABLE_SSE4_2__ */ > > - > > -#ifdef __DISABLE_SSE4_1__ > > -#undef __DISABLE_SSE4_1__ > > -#pragma GCC pop_options > > -#endif /* __DISABLE_SSE4_1__ */ > > +#endif /* __DISABLE_CRC32__ */ > > > > #endif /* _SMMINTRIN_H_INCLUDED */ > > diff --git a/gcc/testsuite/gcc.target/i386/crc32-6.c b/gcc/testsuite/gcc.target/i386/crc32-6.c > > index 464e3444069..1f306534bb8 100644 > > --- a/gcc/testsuite/gcc.target/i386/crc32-6.c > > +++ b/gcc/testsuite/gcc.target/i386/crc32-6.c > > @@ -7,7 +7,7 @@ > > unsigned int > > test_mm_crc32_u8 (unsigned int CRC, unsigned char V) > > { > > - return _mm_crc32_u8 (CRC, V); > > + return __builtin_ia32_crc32qi (CRC, V); > > This test should not be changed, it correctly reports ISA mismatch. It > even passes -mno-crc32. > > Uros. > > > } > > > > /* { dg-error "needs isa option -mcrc32" "" { target *-*-* } 0 } */ > > diff --git a/gcc/testsuite/gcc.target/i386/crc32-7.c b/gcc/testsuite/gcc.target/i386/crc32-7.c > > new file mode 100644 > > index 00000000000..2e310e38b82 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/crc32-7.c > > @@ -0,0 +1,34 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -mcrc32" } */ > > +/* { dg-final { scan-assembler "crc32b\[^\\n\]*eax" } } */ > > +/* { dg-final { scan-assembler "crc32w\[^\\n\]*eax" } } */ > > +/* { dg-final { scan-assembler "crc32l\[^\\n\]*eax" } } */ > > +/* { dg-final { scan-assembler "crc32q\[^\\n\]*rax" { target { ! ia32 } } } } */ > > + > > +#include <immintrin.h> > > + > > +unsigned int > > +test_mm_crc32_u8 (unsigned int CRC, unsigned char V) > > +{ > > + return _mm_crc32_u8 (CRC, V); > > +} > > + > > +unsigned int > > +test_mm_crc32_u16 (unsigned int CRC, unsigned short V) > > +{ > > + return _mm_crc32_u16 (CRC, V); > > +} > > + > > +unsigned int > > +test_mm_crc32_u32 (unsigned int CRC, unsigned int V) > > +{ > > + return _mm_crc32_u32 (CRC, V); > > +} > > + > > +#ifdef __x86_64__ > > +unsigned long long > > +test_mm_crc32_u64 (unsigned long long CRC, unsigned long long V) > > +{ > > + return _mm_crc32_u64 (CRC, V); > > +} > > +#endif > > -- > > 2.18.1 > >
On Fri, Apr 15, 2022 at 10:34 AM Hongyu Wang <wwwhhhyyy333@gmail.com> wrote: > > > This test should not be changed, it correctly reports ISA mismatch. It > > even passes -mno-crc32. > > The error message changes from "needs isa option -mcrc32" to "target > specific option mismatch" with the #pragma change. > I see many of our intrinsic would throw such error, it has been a long > term issue for intrinsic diagnostic. > > So for this test either I change the dg-error message or the call to > builtin, otherwise it would fail. Please update the dg-error message. OK with the above change. Thanks, Uros. > > Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> 于2022年4月15日周五 15:54写道: > > > > On Fri, Apr 15, 2022 at 6:30 AM Hongyu Wang <hongyu.wang@intel.com> wrote: > > > > > > Hi, > > > > > > Complile _mm_crc32_u8/16/32/64 intrinsics with -mcrc32 > > > would meet target specific option mismatch. Correct target pragma > > > to fix. > > > > > > Bootstrapped/regtest on x86_64-pc-linux-gnu{-m32,}. > > > > > > Ok for master and backport to GCC 11? > > > > > > gcc/ChangeLog: > > > > > > * config/i386/smmintrin.h: Correct target pragma from sse4.1 > > > and sse4.2 to crc32 for crc32 intrinsics. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * gcc.target/i386/crc32-6.c: Adjust to call builtin. > > > * gcc.target/i386/crc32-7.c: New test. > > > --- > > > gcc/config/i386/smmintrin.h | 25 +++++------------- > > > gcc/testsuite/gcc.target/i386/crc32-6.c | 2 +- > > > gcc/testsuite/gcc.target/i386/crc32-7.c | 34 +++++++++++++++++++++++++ > > > 3 files changed, 42 insertions(+), 19 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/crc32-7.c > > > > > > diff --git a/gcc/config/i386/smmintrin.h b/gcc/config/i386/smmintrin.h > > > index b42b212300f..eb6a451c10a 100644 > > > --- a/gcc/config/i386/smmintrin.h > > > +++ b/gcc/config/i386/smmintrin.h > > > @@ -810,17 +810,11 @@ _mm_cmpgt_epi64 (__m128i __X, __m128i __Y) > > > > > > #include <popcntintrin.h> > > > > > > -#ifndef __SSE4_1__ > > > +#ifndef __CRC32__ > > > #pragma GCC push_options > > > -#pragma GCC target("sse4.1") > > > -#define __DISABLE_SSE4_1__ > > > -#endif /* __SSE4_1__ */ > > > - > > > -#ifndef __SSE4_2__ > > > -#pragma GCC push_options > > > -#pragma GCC target("sse4.2") > > > -#define __DISABLE_SSE4_2__ > > > -#endif /* __SSE4_1__ */ > > > +#pragma GCC target("crc32") > > > +#define __DISABLE_CRC32__ > > > +#endif /* __CRC32__ */ > > > > > > /* Accumulate CRC32 (polynomial 0x11EDC6F41) value. */ > > > extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) > > > @@ -849,14 +843,9 @@ _mm_crc32_u64 (unsigned long long __C, unsigned long long __V) > > > } > > > #endif > > > > > > -#ifdef __DISABLE_SSE4_2__ > > > -#undef __DISABLE_SSE4_2__ > > > +#ifdef __DISABLE_CRC32__ > > > +#undef __DISABLE_CRC32__ > > > #pragma GCC pop_options > > > -#endif /* __DISABLE_SSE4_2__ */ > > > - > > > -#ifdef __DISABLE_SSE4_1__ > > > -#undef __DISABLE_SSE4_1__ > > > -#pragma GCC pop_options > > > -#endif /* __DISABLE_SSE4_1__ */ > > > +#endif /* __DISABLE_CRC32__ */ > > > > > > #endif /* _SMMINTRIN_H_INCLUDED */ > > > diff --git a/gcc/testsuite/gcc.target/i386/crc32-6.c b/gcc/testsuite/gcc.target/i386/crc32-6.c > > > index 464e3444069..1f306534bb8 100644 > > > --- a/gcc/testsuite/gcc.target/i386/crc32-6.c > > > +++ b/gcc/testsuite/gcc.target/i386/crc32-6.c > > > @@ -7,7 +7,7 @@ > > > unsigned int > > > test_mm_crc32_u8 (unsigned int CRC, unsigned char V) > > > { > > > - return _mm_crc32_u8 (CRC, V); > > > + return __builtin_ia32_crc32qi (CRC, V); > > > > This test should not be changed, it correctly reports ISA mismatch. It > > even passes -mno-crc32. > > > > Uros. > > > > > } > > > > > > /* { dg-error "needs isa option -mcrc32" "" { target *-*-* } 0 } */ > > > diff --git a/gcc/testsuite/gcc.target/i386/crc32-7.c b/gcc/testsuite/gcc.target/i386/crc32-7.c > > > new file mode 100644 > > > index 00000000000..2e310e38b82 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/i386/crc32-7.c > > > @@ -0,0 +1,34 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-O2 -mcrc32" } */ > > > +/* { dg-final { scan-assembler "crc32b\[^\\n\]*eax" } } */ > > > +/* { dg-final { scan-assembler "crc32w\[^\\n\]*eax" } } */ > > > +/* { dg-final { scan-assembler "crc32l\[^\\n\]*eax" } } */ > > > +/* { dg-final { scan-assembler "crc32q\[^\\n\]*rax" { target { ! ia32 } } } } */ > > > + > > > +#include <immintrin.h> > > > + > > > +unsigned int > > > +test_mm_crc32_u8 (unsigned int CRC, unsigned char V) > > > +{ > > > + return _mm_crc32_u8 (CRC, V); > > > +} > > > + > > > +unsigned int > > > +test_mm_crc32_u16 (unsigned int CRC, unsigned short V) > > > +{ > > > + return _mm_crc32_u16 (CRC, V); > > > +} > > > + > > > +unsigned int > > > +test_mm_crc32_u32 (unsigned int CRC, unsigned int V) > > > +{ > > > + return _mm_crc32_u32 (CRC, V); > > > +} > > > + > > > +#ifdef __x86_64__ > > > +unsigned long long > > > +test_mm_crc32_u64 (unsigned long long CRC, unsigned long long V) > > > +{ > > > + return _mm_crc32_u64 (CRC, V); > > > +} > > > +#endif > > > -- > > > 2.18.1 > > >
diff --git a/gcc/config/i386/smmintrin.h b/gcc/config/i386/smmintrin.h index b42b212300f..eb6a451c10a 100644 --- a/gcc/config/i386/smmintrin.h +++ b/gcc/config/i386/smmintrin.h @@ -810,17 +810,11 @@ _mm_cmpgt_epi64 (__m128i __X, __m128i __Y) #include <popcntintrin.h> -#ifndef __SSE4_1__ +#ifndef __CRC32__ #pragma GCC push_options -#pragma GCC target("sse4.1") -#define __DISABLE_SSE4_1__ -#endif /* __SSE4_1__ */ - -#ifndef __SSE4_2__ -#pragma GCC push_options -#pragma GCC target("sse4.2") -#define __DISABLE_SSE4_2__ -#endif /* __SSE4_1__ */ +#pragma GCC target("crc32") +#define __DISABLE_CRC32__ +#endif /* __CRC32__ */ /* Accumulate CRC32 (polynomial 0x11EDC6F41) value. */ extern __inline unsigned int __attribute__((__gnu_inline__, __always_inline__, __artificial__)) @@ -849,14 +843,9 @@ _mm_crc32_u64 (unsigned long long __C, unsigned long long __V) } #endif -#ifdef __DISABLE_SSE4_2__ -#undef __DISABLE_SSE4_2__ +#ifdef __DISABLE_CRC32__ +#undef __DISABLE_CRC32__ #pragma GCC pop_options -#endif /* __DISABLE_SSE4_2__ */ - -#ifdef __DISABLE_SSE4_1__ -#undef __DISABLE_SSE4_1__ -#pragma GCC pop_options -#endif /* __DISABLE_SSE4_1__ */ +#endif /* __DISABLE_CRC32__ */ #endif /* _SMMINTRIN_H_INCLUDED */ diff --git a/gcc/testsuite/gcc.target/i386/crc32-6.c b/gcc/testsuite/gcc.target/i386/crc32-6.c index 464e3444069..1f306534bb8 100644 --- a/gcc/testsuite/gcc.target/i386/crc32-6.c +++ b/gcc/testsuite/gcc.target/i386/crc32-6.c @@ -7,7 +7,7 @@ unsigned int test_mm_crc32_u8 (unsigned int CRC, unsigned char V) { - return _mm_crc32_u8 (CRC, V); + return __builtin_ia32_crc32qi (CRC, V); } /* { dg-error "needs isa option -mcrc32" "" { target *-*-* } 0 } */ diff --git a/gcc/testsuite/gcc.target/i386/crc32-7.c b/gcc/testsuite/gcc.target/i386/crc32-7.c new file mode 100644 index 00000000000..2e310e38b82 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/crc32-7.c @@ -0,0 +1,34 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mcrc32" } */ +/* { dg-final { scan-assembler "crc32b\[^\\n\]*eax" } } */ +/* { dg-final { scan-assembler "crc32w\[^\\n\]*eax" } } */ +/* { dg-final { scan-assembler "crc32l\[^\\n\]*eax" } } */ +/* { dg-final { scan-assembler "crc32q\[^\\n\]*rax" { target { ! ia32 } } } } */ + +#include <immintrin.h> + +unsigned int +test_mm_crc32_u8 (unsigned int CRC, unsigned char V) +{ + return _mm_crc32_u8 (CRC, V); +} + +unsigned int +test_mm_crc32_u16 (unsigned int CRC, unsigned short V) +{ + return _mm_crc32_u16 (CRC, V); +} + +unsigned int +test_mm_crc32_u32 (unsigned int CRC, unsigned int V) +{ + return _mm_crc32_u32 (CRC, V); +} + +#ifdef __x86_64__ +unsigned long long +test_mm_crc32_u64 (unsigned long long CRC, unsigned long long V) +{ + return _mm_crc32_u64 (CRC, V); +} +#endif