On Tue, Jan 22, 2019 at 1:27 AM Uros Bizjak <ubiz...@gmail.com> wrote: > > On Tue, Jan 22, 2019 at 7:57 AM Hongtao Liu <crazy...@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....@intel.com> > > H.J. Lu <hongjiu...@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.