On Mon, Oct 15, 2018 at 4:50 PM Jakub Jelinek <ja...@redhat.com> wrote: > > On Mon, Oct 15, 2018 at 04:22:04PM +0200, Richard Biener wrote: > > On Sun, Oct 14, 2018 at 9:29 PM Uros Bizjak <ubiz...@gmail.com> wrote: > > > > > > On Sat, Oct 13, 2018 at 11:54 PM H.J. Lu <hjl.to...@gmail.com> wrote: > > > > > > > > Also disable AVX512IFMA, AVX5124FMAPS and AVX5124VNNIW when disabling > > > > AVX512F. > > > > > > > > gcc/ > > > > > > > > PR target/87572 > > > > * common/config/i386/i386-common.c > > > > (OPTION_MASK_ISA_AVX512F_UNSET): > > > > Add OPTION_MASK_ISA_AVX512IFMA_UNSET, > > > > OPTION_MASK_ISA_AVX5124FMAPS_UNSET and > > > > OPTION_MASK_ISA_AVX5124VNNIW_UNSET. > > > > > > > > gcc/testsuite/ > > > > > > > > PR target/87572 > > > > * gcc.target/i386/pr87572.c: New test. > > > > > > LGTM. > > > > This caused gazillion of testsuite FAILs like > > > > FAIL: gcc.target/i386/isa-11.c (test for excess errors) > > Excess errors: > > /tmp/ccyurT91.s:8: Error: invalid instruction suffix for `push' > > /tmp/ccyurT91.s:14: Error: invalid instruction suffix for `pop' > > > > where we now emit pushl in 64bit mode. > > That change was incorrect, avx5124fmaps and avx5124vnniw flags are > isa_flags2, rather than isa_flags, and are handled already properly: > #define OPTION_MASK_ISA2_AVX512F_UNSET \ > (OPTION_MASK_ISA_AVX5124FMAPS_UNSET | OPTION_MASK_ISA_AVX5124VNNIW_UNSET) > > So I think we need at least following patch, ok for trunk? > > Plus, I wonder if we shouldn't make it harder to run into these issues, by > changing > Target Report Mask(ISA_AVX5124FMAPS) Var(ix86_isa_flags2) Save > etc. to > Target Report Mask(ISA2_AVX5124FMAPS) Var(ix86_isa_flags2) Save > so that we'll have OPTION_MASK_ISA2_AVX5124FMAPS macros instead of > OPTION_MASK_ISA_AVX5124FMAPS and adjust all i386-common.c etc. uses from ISA > to ISA2 for the ix86_isa_flags2 options. Perhaps we could have > #define TARGET_ISA_AVX5124FMAPS TARGET_ISA2_AVX5124FMAPS > compatibility macro, because unlike the OPTION_MASK_* and TARGET_*_P macros > where you need to specify the right flags the TARGET_* macros already have > that in implicitly. Uros, thoughts on this?
I was looking for a mail, where we discussed x86_isa_flags2 as a temporary solution, with the expectation that some other extensible mechanism gets invented to handle ISA flags. Now we are in c++, and I guess there should be more elegant way to deal with the issue. > > 2018-10-15 Jakub Jelinek <ja...@redhat.com> > > PR target/87572 > * common/config/i386/i386-common.c (OPTION_MASK_ISA_AVX512F_UNSET): > Remove OPTION_MASK_ISA_AVX5124FMAPS_UNSET and > OPTION_MASK_ISA_AVX5124VNNIW_UNSET. Yes, please go ahead with this patch. Thanks, Uros. > --- gcc/common/config/i386/i386-common.c.jj 2018-10-15 16:27:59.214107805 > +0200 > +++ gcc/common/config/i386/i386-common.c 2018-10-15 16:30:30.750564097 > +0200 > @@ -195,8 +195,6 @@ along with GCC; see the file COPYING3. > | OPTION_MASK_ISA_AVX512PF_UNSET | OPTION_MASK_ISA_AVX512ER_UNSET \ > | OPTION_MASK_ISA_AVX512DQ_UNSET | OPTION_MASK_ISA_AVX512BW_UNSET \ > | OPTION_MASK_ISA_AVX512VL_UNSET | OPTION_MASK_ISA_AVX512IFMA_UNSET \ > - | OPTION_MASK_ISA_AVX5124FMAPS_UNSET \ > - | OPTION_MASK_ISA_AVX5124VNNIW_UNSET \ > | OPTION_MASK_ISA_AVX512VBMI2_UNSET \ > | OPTION_MASK_ISA_AVX512VNNI_UNSET \ > | OPTION_MASK_ISA_AVX512VPOPCNTDQ_UNSET \ > > > Jakub
