On Thu, Feb 14, 2019 at 3:21 PM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Fri, Feb 15, 2019 at 12:14 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> > > > > > > > > gcc/
> > > > > > > > >
> > > > > > > > > PR target/89021
> > > > > > > > > * config/i386/i386-builtin.def: Enable MMX intrinsics
> > > > > > > > > with
> > > > > > > > > SSE/SSE2/SSSE3.
> > > > > > > > > * config/i386/i386.c (ix86_option_override_internal):
> > > > > > > > > Don't
> > > > > > > > > enable MMX ISA with TARGET_MMX_WITH_SSE by default.
> > > > > > > > > (ix86_init_mmx_sse_builtins): Enable MMX intrinsics
> > > > > > > > > with
> > > > > > > > > SSE/SSE2/SSSE3.
> > > > > > > > > (ix86_expand_builtin): Allow SSE/SSE2/SSSE3 to
> > > > > > > > > emulate MMX
> > > > > > > > > intrinsics with TARGET_MMX_WITH_SSE.
> > > > > > > > > * config/i386/mmintrin.h: Don't require MMX in 64-bit
> > > > > > > > > mode.
> > > > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> > > > > > > > > index a9abbe8706b..1d417e08734 100644
> > > > > > > > > --- a/gcc/config/i386/i386.c
> > > > > > > > > +++ b/gcc/config/i386/i386.c
> > > > > > > > > @@ -4165,12 +4165,15 @@ ix86_option_override_internal (bool
> > > > > > > > > main_args_p,
> > > > > > > > > opts->x_target_flags
> > > > > > > > > |= TARGET_SUBTARGET64_DEFAULT &
> > > > > > > > > ~opts_set->x_target_flags;
> > > > > > > > >
> > > > > > > > > - /* Enable by default the SSE and MMX builtins. Do
> > > > > > > > > allow the user to
> > > > > > > > > - explicitly disable any of these. In particular,
> > > > > > > > > disabling SSE and
> > > > > > > > > - MMX for kernel code is extremely useful. */
> > > > > > > > > + /* Enable the SSE and MMX builtins by default. Don't
> > > > > > > > > enable MMX
> > > > > > > > > + ISA with TARGET_MMX_WITH_SSE by default. Do allow
> > > > > > > > > the user to
> > > > > > > > > + explicitly disable any of these. In particular,
> > > > > > > > > disabling SSE
> > > > > > > > > + and MMX for kernel code is extremely useful. */
> > > > > > > > > if (!ix86_arch_specified)
> > > > > > > > > opts->x_ix86_isa_flags
> > > > > > > > > - |= ((OPTION_MASK_ISA_SSE2 | OPTION_MASK_ISA_SSE |
> > > > > > > > > OPTION_MASK_ISA_MMX
> > > > > > > > > + |= ((OPTION_MASK_ISA_SSE2 | OPTION_MASK_ISA_SSE
> > > > > > > > > + | (TARGET_MMX_WITH_SSE_P (opts->x_ix86_isa_flags)
> > > > > > > > > + ? 0 : OPTION_MASK_ISA_MMX)
> > > > > > > > > | TARGET_SUBTARGET64_ISA_DEFAULT)
> > > > > > > > > & ~opts->x_ix86_isa_flags_explicit);
> > > > > > > >
> > > > > > > > Please split the above into two clauses, the first that sets
> > > > > > > > SSE and
> > > > > > > > MMX by default, and the second to or with
> > > > > > > >
> > > > > > > > opts->x_ix86_isa_flags
> > > > > > > > |= TARGET_SUBTARGET64_ISA_DEFAULT &
> > > > > > > > ~opts->x_ix86_isa_flags_explicit
> > > > > > > >
> > > > > > >
> > > > > > > Like this?
> > > > > >
> > > > > > Yes, but also split the comment.
> > > > >
> > > > > I will go with
> > > > >
> > > > > /* Enable by default the SSE and MMX builtins. Do allow the
> > > > > user to
> > > > > explicitly disable any of these. In particular, disabling
> > > > > SSE and
> > > > > MMX for kernel code is extremely useful. */
> > > > > if (!ix86_arch_specified)
> > > > > {
> > > > > /* Don't enable MMX ISA with TARGET_MMX_WITH_SSE. */
> > > > > opts->x_ix86_isa_flags
> > > > > |= ((OPTION_MASK_ISA_SSE2 | OPTION_MASK_ISA_SSE
> > > > > | (TARGET_MMX_WITH_SSE_P (opts->x_ix86_isa_flags)
> > > > > ? 0 : OPTION_MASK_ISA_MMX))
> > > > > & ~opts->x_ix86_isa_flags_explicit);
> > > > > opts->x_ix86_isa_flags
> > > > > |= (TARGET_SUBTARGET64_ISA_DEFAULT
> > > > > & ~opts->x_ix86_isa_flags_explicit);
> > > > > }
> > > >
> > > > I'll commit the following patch that finally defines
> > > > TARGET_SUBTARGET64_ISA_DEFAULT. You could then simply clear the MMX
> > > > bit from x_i86_isa_flags, like:
> > > >
> > > > if (!ix86_arch_specified)
> > > > opts->x_ix86_isa_flags
> > > > |= TARGET_SUBTARGET64_ISA_DEFAULT &
> > > > ~opts->x_ix86_isa_flags_explicit;
> > > >
> > > > /* Don't enable MMX ISA with TARGET_MMX_WITH_SSE. */
> > > > if (TARGET_MMX_WITH_SSE_P (opts->x_ix86_isa_flags))
> > > > opts->x_ix86_isa_flags &= ~OPTION_MASK_ISA_MMX;
> > >
> > > I think it should be:
> > >
> > > /* Don't enable MMX ISA with TARGET_MMX_WITH_SSE. */
> > > if (!(opts->x_ix86_isa_flags & OPTION_MASK_ISA_MMX)
> > I meant opts->x_ix86_isa_flags_explicit.
> > > && TARGET_MMX_WITH_SSE_P (opts->x_ix86_isa_flags))
> > > opts->x_ix86_isa_flags &= ~OPTION_MASK_ISA_MMX;
>
> Well ... I didn't test this part. OTOH, maybe this part is not needed,
> MMX disabling can go *after*
>
> /* Turn on MMX builtins for -msse. */
> if (TARGET_SSE_P (opts->x_ix86_isa_flags))
> opts->x_ix86_isa_flags
> |= OPTION_MASK_ISA_MMX & ~opts->x_ix86_isa_flags_explicit;
>
It works only if TARGET_SUBTARGET64_ISA_DEFAULT doesn't
include OPTION_MASK_ISA_MMX.
--
H.J.
