[PATCH]i386: Add BDESC2 for ix86_isa_flags2 and refine all related.

[H.J. Lu]

On Tue, Jan 22, 2019 at 1:27 AM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Tue, Jan 22, 2019 at 7:57 AM Hongtao Liu <crazylht@gmail.com> wrote:
>
> > This is adjusted patch: Modify BDESC to accept two flags and merge
> > ARGS2/SPEICAL_ARGS2 into ARGS/SPECIAL_ARGS.
> > Bootstrap test is ok, no regression on x86 backend testcases.
> >
> > Changelog:
> > ---
> > gcc/
> >
> > 2019-01-21  Hongtao Liu <hongtao.liu@intel.com>
> >     H.J. Lu <hongjiu.lu@intel.com>
> >
> > PR target/88909
> > * config/i386/i386-builtin.def:
> > Refine all builtins and merge ARGS2 and SPECIAL_ARGS2
> > into ARGS and SPECIAL_ARGS.
> > * config/i386/i386.c (BDESC):
> > Refine to accept mask2.
> > (BDESC_FIRST): Likewise.
> > (builtin_description): Add new member mask2.
> > (define_builtin): Modified to handle both
> > ix86_isa_flags and ix86_isa_flags2.
> > (define_builtin_const): Likewise.
> > (define_builtin_pure): Likewise.
> > (bdesc_tm[]): Likewise.
> > (ix86_init_mmx_sse_builtins): Likewise.
> > (define_builtin2): Delete.
> > (define_builtin_const2): Likewise.
>
> "Refine" is not the correct word, better say e.g.:
> * config/i386/i386-builtin.def: Add mask2 initialization to all
> builtins.  Merge ...
> * config/i386/i386.c (BDESC): Add mask2 to the definition.
> ...
> (define_builtin): Handle both ix86_isa_flags and ix86_isa_flags2.

Fixed.

> So, you should write ChangeLog as if you are giving orders to the
> codebase what to do.  Please rewrite the ChangeLog in this way (maybe
> ask HJ for a review).
>
> The patch is OK for mainline with some small adjustments below. No
> need to repost.
>
> Thanks,
> Uros.
>
> +  /* A may be 64bit only regardless of ISAs.  */
>    if (!(mask & OPTION_MASK_ISA_64BIT) || TARGET_64BIT)
>      {
>        ix86_builtins_isa[(int) code].isa = mask;
> +      ix86_builtins_isa[(int) code].isa2 = mask2;
>
> Please fix the above comment.

Fixed.

> -      if (mask == 0
> -      || (mask & ix86_isa_flags) != 0
> +      if (((mask2 == 0 || (mask2 & ix86_isa_flags2) != 0)
> +       && (mask == 0 || (mask & ix86_isa_flags) != 0))
>        || (lang_hooks.builtin_function
>            == lang_hooks.builtin_function_ext_scope))
>
> Please fix the indenting.

Fixed.

>    def_builtin_const (OPTION_MASK_ISA_SSE | OPTION_MASK_ISA_3DNOW_A
>               /* As it uses V4HImode, we have to require -mmmx too.  */
>               | OPTION_MASK_ISA_MMX,
> -             "__builtin_ia32_vec_ext_v4hi",
> +             0, "__builtin_ia32_vec_ext_v4hi",
>               HI_FTYPE_V4HI_INT, IX86_BUILTIN_VEC_EXT_V4HI);
>
> Please move "0" up one line, just after OPTION_MASK_ISA_MMX, similar
> to __builtin_ia32_maskmovq change.
>
>    def_builtin_const (OPTION_MASK_ISA_SSE | OPTION_MASK_ISA_3DNOW_A
>               /* As it uses V4HImode, we have to require -mmmx too.  */
>               | OPTION_MASK_ISA_MMX,
> -             "__builtin_ia32_vec_set_v4hi",
> +             0, "__builtin_ia32_vec_set_v4hi",
>               V4HI_FTYPE_V4HI_HI_INT, IX86_BUILTIN_VEC_SET_V4HI);
>
> Same here.
>
>    def_builtin (OPTION_MASK_ISA_RDSEED | OPTION_MASK_ISA_64BIT,
> -           "__builtin_ia32_rdseed_di_step",
> +           0, "__builtin_ia32_rdseed_di_step",
>             INT_FTYPE_PULONGLONG, IX86_BUILTIN_RDSEED64_STEP);
>
> And here.
>
>    def_builtin (OPTION_MASK_ISA_64BIT,
> -           "__builtin_ia32_addcarryx_u64",
> +           0, "__builtin_ia32_addcarryx_u64",
>             UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PULONGLONG,
>             IX86_BUILTIN_ADDCARRYX64);
>
> And here.
>
>    def_builtin (OPTION_MASK_ISA_64BIT,
> -           "__builtin_ia32_sbb_u64",
> +           0, "__builtin_ia32_sbb_u64",
>             UCHAR_FTYPE_UCHAR_ULONGLONG_ULONGLONG_PULONGLONG,
>             IX86_BUILTIN_SBB64);
>
> And here.

All fixed.

The updated patch is almost 1MB which is too large for the GCC mailing
list.  Here
is the link for patch we are checking in:

https://gcc.gnu.org/git/?p=gcc.git;a=patch;h=c6524483f65047c26668706f8577deb85a8f026e

Thanks.

-- 
H.J.