| Message ID | 20240703072453.548243-1-hongtao.liu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [committed] Move runtime check into a separate function and guard it with target ("no-avx") | expand |
On Wed, Jul 3, 2024 at 9:25 AM liuhongt <hongtao.liu@intel.com> wrote: > > The patch can avoid SIGILL on non-AVX512 machine due to kmovd is > generated in dynamic check. > > Committed as an obvious fix. Hmm, now all avx512 tests SIGILL when testing with -m32: Dump of assembler code for function __get_cpuid_count: => 0x08049500 <+0>: kmovd %eax,%k2 0x08049504 <+4>: kmovd %edx,%k1 0x08049508 <+8>: pushf 0x08049509 <+9>: pushf 0x0804950a <+10>: pop %eax 0x0804950b <+11>: mov %eax,%edx looks like __get_cpuid_count is no longer inlined but AVX512 is in effect for it. Maybe use #pragma GCC target around the includes instead? > gcc/testsuite/ChangeLog: > > PR target/115748 > * gcc.target/i386/avx512-check.h: Move runtime check into a > separate function and guard it with target ("no-avx"). > --- > gcc/testsuite/gcc.target/i386/avx512-check.h | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h b/gcc/testsuite/gcc.target/i386/avx512-check.h > index 0ad9064f637..71858a33dac 100644 > --- a/gcc/testsuite/gcc.target/i386/avx512-check.h > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h > @@ -34,8 +34,9 @@ check_osxsave (void) > return (ecx & bit_OSXSAVE) != 0; > } > > +__attribute__((noipa,target("no-avx"))) > int > -main () > +avx512_runtime_support_p () > { > unsigned int eax, ebx, ecx, edx; > > @@ -100,6 +101,17 @@ main () > && (edx & bit_AVX512VP2INTERSECT) > #endif > && avx512f_os_support ()) > + { > + return 1; > + } > + > + return 0; > +} > + > +int > +main () > +{ > + if (avx512_runtime_support_p ()) > { > DO_TEST (); > #ifdef DEBUG > -- > 2.31.1 >
On Wed, Jul 3, 2024, 9:37 PM Richard Biener <richard.guenther@gmail.com> wrote: > On Wed, Jul 3, 2024 at 9:25 AM liuhongt <hongtao.liu@intel.com> wrote: > > > > The patch can avoid SIGILL on non-AVX512 machine due to kmovd is > > generated in dynamic check. > > > > Committed as an obvious fix. > > Hmm, now all avx512 tests SIGILL when testing with -m32: > > Dump of assembler code for function __get_cpuid_count: > => 0x08049500 <+0>: kmovd %eax,%k2 > 0x08049504 <+4>: kmovd %edx,%k1 > 0x08049508 <+8>: pushf > 0x08049509 <+9>: pushf > 0x0804950a <+10>: pop %eax > 0x0804950b <+11>: mov %eax,%edx > > looks like __get_cpuid_count is no longer inlined but AVX512 is in > effect for it. > > Maybe use #pragma GCC target around the includes instead? > Can the built-in cpu supports be used? > > gcc/testsuite/ChangeLog: > > > > PR target/115748 > > * gcc.target/i386/avx512-check.h: Move runtime check into a > > separate function and guard it with target ("no-avx"). > > --- > > gcc/testsuite/gcc.target/i386/avx512-check.h | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h > b/gcc/testsuite/gcc.target/i386/avx512-check.h > > index 0ad9064f637..71858a33dac 100644 > > --- a/gcc/testsuite/gcc.target/i386/avx512-check.h > > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h > > @@ -34,8 +34,9 @@ check_osxsave (void) > > return (ecx & bit_OSXSAVE) != 0; > > } > > > > +__attribute__((noipa,target("no-avx"))) > > int > > -main () > > +avx512_runtime_support_p () > > { > > unsigned int eax, ebx, ecx, edx; > > > > @@ -100,6 +101,17 @@ main () > > && (edx & bit_AVX512VP2INTERSECT) > > #endif > > && avx512f_os_support ()) > > + { > > + return 1; > > + } > > + > > + return 0; > > +} > > + > > +int > > +main () > > +{ > > + if (avx512_runtime_support_p ()) > > { > > DO_TEST (); > > #ifdef DEBUG > > -- > > 2.31.1 > > >
On Thu, Jul 4, 2024 at 6:17 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > On Wed, Jul 3, 2024, 9:37 PM Richard Biener <richard.guenther@gmail.com> wrote: >> >> On Wed, Jul 3, 2024 at 9:25 AM liuhongt <hongtao.liu@intel.com> wrote: >> > >> > The patch can avoid SIGILL on non-AVX512 machine due to kmovd is >> > generated in dynamic check. >> > >> > Committed as an obvious fix. >> >> Hmm, now all avx512 tests SIGILL when testing with -m32: oops, I should have a test on non-avx512 machine. >> >> Dump of assembler code for function __get_cpuid_count: >> => 0x08049500 <+0>: kmovd %eax,%k2 >> 0x08049504 <+4>: kmovd %edx,%k1 >> 0x08049508 <+8>: pushf >> 0x08049509 <+9>: pushf >> 0x0804950a <+10>: pop %eax >> 0x0804950b <+11>: mov %eax,%edx >> >> looks like __get_cpuid_count is no longer inlined but AVX512 is in >> effect for it. >> >> Maybe use #pragma GCC target around the includes instead? > > > Can the built-in cpu supports be used? But we still need avx512f_os_support which is in avx512f-os-support.h. I'll try to push GCC target ("no-avx") to #include "cpuid.h" and "avx512f-os-support.h" > >> >> > gcc/testsuite/ChangeLog: >> > >> > PR target/115748 >> > * gcc.target/i386/avx512-check.h: Move runtime check into a >> > separate function and guard it with target ("no-avx"). >> > --- >> > gcc/testsuite/gcc.target/i386/avx512-check.h | 14 +++++++++++++- >> > 1 file changed, 13 insertions(+), 1 deletion(-) >> > >> > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h b/gcc/testsuite/gcc.target/i386/avx512-check.h >> > index 0ad9064f637..71858a33dac 100644 >> > --- a/gcc/testsuite/gcc.target/i386/avx512-check.h >> > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h >> > @@ -34,8 +34,9 @@ check_osxsave (void) >> > return (ecx & bit_OSXSAVE) != 0; >> > } >> > >> > +__attribute__((noipa,target("no-avx"))) >> > int >> > -main () >> > +avx512_runtime_support_p () >> > { >> > unsigned int eax, ebx, ecx, edx; >> > >> > @@ -100,6 +101,17 @@ main () >> > && (edx & bit_AVX512VP2INTERSECT) >> > #endif >> > && avx512f_os_support ()) >> > + { >> > + return 1; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > +int >> > +main () >> > +{ >> > + if (avx512_runtime_support_p ()) >> > { >> > DO_TEST (); >> > #ifdef DEBUG >> > -- >> > 2.31.1 >> >
On Thu, Jul 4, 2024, 9:12 AM Hongtao Liu <crazylht@gmail.com> wrote: > On Thu, Jul 4, 2024 at 6:17 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > > > On Wed, Jul 3, 2024, 9:37 PM Richard Biener <richard.guenther@gmail.com> > wrote: > >> > >> On Wed, Jul 3, 2024 at 9:25 AM liuhongt <hongtao.liu@intel.com> wrote: > >> > > >> > The patch can avoid SIGILL on non-AVX512 machine due to kmovd is > >> > generated in dynamic check. > >> > > >> > Committed as an obvious fix. > >> > >> Hmm, now all avx512 tests SIGILL when testing with -m32: > oops, I should have a test on non-avx512 machine. > >> > >> Dump of assembler code for function __get_cpuid_count: > >> => 0x08049500 <+0>: kmovd %eax,%k2 > >> 0x08049504 <+4>: kmovd %edx,%k1 > >> 0x08049508 <+8>: pushf > >> 0x08049509 <+9>: pushf > >> 0x0804950a <+10>: pop %eax > >> 0x0804950b <+11>: mov %eax,%edx > >> > >> looks like __get_cpuid_count is no longer inlined but AVX512 is in > >> effect for it. > >> > >> Maybe use #pragma GCC target around the includes instead? > > > > > > Can the built-in cpu supports be used? > But we still need avx512f_os_support which is in avx512f-os-support.h. > Doesn't the built-in CPU supports also check the OS support? I'll try to push GCC target ("no-avx") to #include "cpuid.h" and > "avx512f-os-support.h" > > > >> > >> > gcc/testsuite/ChangeLog: > >> > > >> > PR target/115748 > >> > * gcc.target/i386/avx512-check.h: Move runtime check into a > >> > separate function and guard it with target ("no-avx"). > >> > --- > >> > gcc/testsuite/gcc.target/i386/avx512-check.h | 14 +++++++++++++- > >> > 1 file changed, 13 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h > b/gcc/testsuite/gcc.target/i386/avx512-check.h > >> > index 0ad9064f637..71858a33dac 100644 > >> > --- a/gcc/testsuite/gcc.target/i386/avx512-check.h > >> > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h > >> > @@ -34,8 +34,9 @@ check_osxsave (void) > >> > return (ecx & bit_OSXSAVE) != 0; > >> > } > >> > > >> > +__attribute__((noipa,target("no-avx"))) > >> > int > >> > -main () > >> > +avx512_runtime_support_p () > >> > { > >> > unsigned int eax, ebx, ecx, edx; > >> > > >> > @@ -100,6 +101,17 @@ main () > >> > && (edx & bit_AVX512VP2INTERSECT) > >> > #endif > >> > && avx512f_os_support ()) > >> > + { > >> > + return 1; > >> > + } > >> > + > >> > + return 0; > >> > +} > >> > + > >> > +int > >> > +main () > >> > +{ > >> > + if (avx512_runtime_support_p ()) > >> > { > >> > DO_TEST (); > >> > #ifdef DEBUG > >> > -- > >> > 2.31.1 > >> > > > > > -- > BR, > Hongtao >
On Thu, Jul 4, 2024 at 9:41 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > > On Thu, Jul 4, 2024, 9:12 AM Hongtao Liu <crazylht@gmail.com> wrote: >> >> On Thu, Jul 4, 2024 at 6:17 AM H.J. Lu <hjl.tools@gmail.com> wrote: >> > >> > >> > On Wed, Jul 3, 2024, 9:37 PM Richard Biener <richard.guenther@gmail.com> wrote: >> >> >> >> On Wed, Jul 3, 2024 at 9:25 AM liuhongt <hongtao.liu@intel.com> wrote: >> >> > >> >> > The patch can avoid SIGILL on non-AVX512 machine due to kmovd is >> >> > generated in dynamic check. >> >> > >> >> > Committed as an obvious fix. >> >> >> >> Hmm, now all avx512 tests SIGILL when testing with -m32: >> oops, I should have a test on non-avx512 machine. >> >> >> >> Dump of assembler code for function __get_cpuid_count: >> >> => 0x08049500 <+0>: kmovd %eax,%k2 >> >> 0x08049504 <+4>: kmovd %edx,%k1 >> >> 0x08049508 <+8>: pushf >> >> 0x08049509 <+9>: pushf >> >> 0x0804950a <+10>: pop %eax >> >> 0x0804950b <+11>: mov %eax,%edx >> >> >> >> looks like __get_cpuid_count is no longer inlined but AVX512 is in >> >> effect for it. >> >> >> >> Maybe use #pragma GCC target around the includes instead? >> > >> > >> > Can the built-in cpu supports be used? >> But we still need avx512f_os_support which is in avx512f-os-support.h. > > > Doesn't the built-in CPU supports also check the OS support? I checked the code, get_available_features also checks the OS support, so yes built-in CPU supports should work. > >> I'll try to push GCC target ("no-avx") to #include "cpuid.h" and >> "avx512f-os-support.h" >> > >> >> >> >> > gcc/testsuite/ChangeLog: >> >> > >> >> > PR target/115748 >> >> > * gcc.target/i386/avx512-check.h: Move runtime check into a >> >> > separate function and guard it with target ("no-avx"). >> >> > --- >> >> > gcc/testsuite/gcc.target/i386/avx512-check.h | 14 +++++++++++++- >> >> > 1 file changed, 13 insertions(+), 1 deletion(-) >> >> > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h b/gcc/testsuite/gcc.target/i386/avx512-check.h >> >> > index 0ad9064f637..71858a33dac 100644 >> >> > --- a/gcc/testsuite/gcc.target/i386/avx512-check.h >> >> > +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h >> >> > @@ -34,8 +34,9 @@ check_osxsave (void) >> >> > return (ecx & bit_OSXSAVE) != 0; >> >> > } >> >> > >> >> > +__attribute__((noipa,target("no-avx"))) >> >> > int >> >> > -main () >> >> > +avx512_runtime_support_p () >> >> > { >> >> > unsigned int eax, ebx, ecx, edx; >> >> > >> >> > @@ -100,6 +101,17 @@ main () >> >> > && (edx & bit_AVX512VP2INTERSECT) >> >> > #endif >> >> > && avx512f_os_support ()) >> >> > + { >> >> > + return 1; >> >> > + } >> >> > + >> >> > + return 0; >> >> > +} >> >> > + >> >> > +int >> >> > +main () >> >> > +{ >> >> > + if (avx512_runtime_support_p ()) >> >> > { >> >> > DO_TEST (); >> >> > #ifdef DEBUG >> >> > -- >> >> > 2.31.1 >> >> > >> >> >> >> -- >> BR, >> Hongtao -- BR, Hongtao
diff --git a/gcc/testsuite/gcc.target/i386/avx512-check.h b/gcc/testsuite/gcc.target/i386/avx512-check.h index 0ad9064f637..71858a33dac 100644 --- a/gcc/testsuite/gcc.target/i386/avx512-check.h +++ b/gcc/testsuite/gcc.target/i386/avx512-check.h @@ -34,8 +34,9 @@ check_osxsave (void) return (ecx & bit_OSXSAVE) != 0; } +__attribute__((noipa,target("no-avx"))) int -main () +avx512_runtime_support_p () { unsigned int eax, ebx, ecx, edx; @@ -100,6 +101,17 @@ main () && (edx & bit_AVX512VP2INTERSECT) #endif && avx512f_os_support ()) + { + return 1; + } + + return 0; +} + +int +main () +{ + if (avx512_runtime_support_p ()) { DO_TEST (); #ifdef DEBUG
The patch can avoid SIGILL on non-AVX512 machine due to kmovd is generated in dynamic check. Committed as an obvious fix. gcc/testsuite/ChangeLog: PR target/115748 * gcc.target/i386/avx512-check.h: Move runtime check into a separate function and guard it with target ("no-avx"). --- gcc/testsuite/gcc.target/i386/avx512-check.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)