This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix recent i386 regressions (was Re: [PATCH] i386: Also disable AVX512IFMA/AVX5124FMAPS/AVX5124VNNIW)


On Mon, Oct 15, 2018 at 04:22:04PM +0200, Richard Biener wrote:
> On Sun, Oct 14, 2018 at 9:29 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Sat, Oct 13, 2018 at 11:54 PM H.J. Lu <hjl.tools@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?

2018-10-15  Jakub Jelinek  <jakub@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.

--- 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]