Hi Will,
Thanks for the cleanup! Some comments are inlined.
on 2022/7/14 05:39, will schmidt wrote:
> [PATCH, rs6000] Additional cleanup of rs6000_builtin_mask
>
> Hi,
> Post the rs6000 builtins rewrite, some of the leftover builtin
> code is redundant and can be removed.
> This replaces the remaining usage of bu_mask in
> rs6000_target_modify_macros() with checks against the rs6000_cpu directly.
> Thusly the bu_mask variable can be removed. After that variable
> is eliminated there are no other uses of rs6000_builtin_mask_calculate(),
> so that function can also be safely removed.
>
The TargetVariable rs6000_builtin_mask in rs6000.opt is useless, it seems
it can be removed together?
> I have tested this on current systems (P8,P9,P10) without regressions.
>
> OK for trunk?
>
>
> Thanks,
> -Will
>
> gcc/
> * config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): Remove
> bu_mask references. (rs6000_define_or_undefine_macro): Replace
> bu_mask reference with a rs6000_cpu value check.
> (rs6000_cpu_cpp_builtins): Remove rs6000_builtin_mask_calculate()
> parameter from call to rs6000_target_modify_macros.
> * config/rs6000/rs6000-protos.h (rs6000_target_modify_macros,
> rs6000_target_modify_macros_ptr): Remove parameter from extern
> for the prototype.
> * config/rs6000/rs6000.cc (rs6000_target_modify_macros_ptr): Remove
> parameter from prototype, update calls to this function.
> (rs6000_print_builtin_options): Remove prototype, call and function.
> (rs6000_builtin_mask_calculate): Remove function.
> (rs6000_debug_reg_global): Remove call to rs6000_print_builtin_options.
> (rs6000_option_override_internal): Remove rs6000_builtin_mask var
> and builtin_mask debug output.
> (rs6000_pragma_target_parse): Update calls to
> rs6000_target_modify_ptr.
>
>
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index 0d13645040ff..4d051b906582 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -333,24 +333,20 @@ rs6000_define_or_undefine_macro (bool define_p, const
> char *name)
> else
> cpp_undef (parse_in, name);
> }
>
> /* Define or undefine macros based on the current target. If the user does
> - #pragma GCC target, we need to adjust the macros dynamically. Note, some
> of
> - the options needed for builtins have been moved to separate variables, so
> - have both the target flags and the builtin flags as arguments. */
> + #pragma GCC target, we need to adjust the macros dynamically. */
>
> void
> -rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags,
> - HOST_WIDE_INT bu_mask)
> +rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags)
> {
> if (TARGET_DEBUG_BUILTIN || TARGET_DEBUG_TARGET)
> fprintf (stderr,
> - "rs6000_target_modify_macros (%s, " HOST_WIDE_INT_PRINT_HEX
> - ", " HOST_WIDE_INT_PRINT_HEX ")\n",
> + "rs6000_target_modify_macros (%s, " HOST_WIDE_INT_PRINT_HEX ")\n",
> (define_p) ? "define" : "undef",
> - flags, bu_mask);
> + flags);
>
> /* Each of the flags mentioned below controls whether certain
> preprocessor macros will be automatically defined when
> preprocessing source files for compilation by this compiler.
> While most of these flags can be enabled or disabled
> @@ -593,14 +589,12 @@ rs6000_target_modify_macros (bool define_p,
> HOST_WIDE_INT flags,
> /* OPTION_MASK_FLOAT128_HARDWARE can be turned on if -mcpu=power9 is used
> or
> via the target attribute/pragma. */
> if ((flags & OPTION_MASK_FLOAT128_HW) != 0)
> rs6000_define_or_undefine_macro (define_p, "__FLOAT128_HARDWARE__");
>
> - /* options from the builtin masks. */
> - /* Note that OPTION_MASK_FPRND is enabled only if
> - (rs6000_cpu == PROCESSOR_CELL) (e.g. -mcpu=cell). */
> - if ((bu_mask & OPTION_MASK_FPRND) != 0)
> + /* Tell the user if we are targeting CELL. */
> + if (rs6000_cpu == PROCESSOR_CELL)
> rs6000_define_or_undefine_macro (define_p, "__PPU__");
>
> /* Tell the user if we support the MMA instructions. */
> if ((flags & OPTION_MASK_MMA) != 0)
> rs6000_define_or_undefine_macro (define_p, "__MMA__");
> @@ -614,12 +608,11 @@ rs6000_target_modify_macros (bool define_p,
> HOST_WIDE_INT flags,
>
> void
> rs6000_cpu_cpp_builtins (cpp_reader *pfile)
> {
> /* Define all of the common macros. */
> - rs6000_target_modify_macros (true, rs6000_isa_flags,
> - rs6000_builtin_mask_calculate ());
> + rs6000_target_modify_macros (true, rs6000_isa_flags);
>
> if (TARGET_FRE)
> builtin_define ("__RECIP__");
> if (TARGET_FRES)
> builtin_define ("__RECIPF__");
> diff --git a/gcc/config/rs6000/rs6000-protos.h
> b/gcc/config/rs6000/rs6000-protos.h
> index 3ea010236090..b3c16e7448d8 100644
> --- a/gcc/config/rs6000/rs6000-protos.h
> +++ b/gcc/config/rs6000/rs6000-protos.h
> @@ -318,13 +318,12 @@ extern void rs6000_pragma_longcall (struct cpp_reader
> *);
> extern void rs6000_cpu_cpp_builtins (struct cpp_reader *);
> #ifdef TREE_CODE
> extern bool rs6000_pragma_target_parse (tree, tree);
> #endif
> extern void rs6000_activate_target_options (tree new_tree);
> -extern void rs6000_target_modify_macros (bool, HOST_WIDE_INT, HOST_WIDE_INT);
> -extern void (*rs6000_target_modify_macros_ptr) (bool, HOST_WIDE_INT,
> - HOST_WIDE_INT);
> +extern void rs6000_target_modify_macros (bool, HOST_WIDE_INT);
> +extern void (*rs6000_target_modify_macros_ptr) (bool, HOST_WIDE_INT);
>
> /* Declare functions in rs6000-d.cc */
> extern void rs6000_d_target_versions (void);
> extern void rs6000_d_register_target_info (void);
>
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 3ff16b8ae04d..5fda18d82795 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -276,11 +276,11 @@ const char *tcb_verification_symbol =
> "__parse_hwcap_and_convert_at_platform";
> bool cpu_builtin_p = false;
>
> /* Pointer to function (in rs6000-c.cc) that can define or undefine target
> macros that have changed. Languages that don't support the preprocessor
> don't link in rs6000-c.cc, so we can't call it directly. */
> -void (*rs6000_target_modify_macros_ptr) (bool, HOST_WIDE_INT, HOST_WIDE_INT);
> +void (*rs6000_target_modify_macros_ptr) (bool, HOST_WIDE_INT);
>
> /* Simplfy register classes into simpler classifications. We assume
> GPR_REG_TYPE - FPR_REG_TYPE are ordered so that we can use a simple range
> check for standard register classes (gpr/floating/altivec/vsx) and
> floating/vector classes (float/altivec/vsx). */
> @@ -1169,12 +1169,10 @@ enum reg_class (*rs6000_preferred_reload_class_ptr)
> (rtx, enum reg_class)
>
> const int INSN_NOT_AVAILABLE = -1;
>
> static void rs6000_print_isa_options (FILE *, int, const char *,
> HOST_WIDE_INT);
> -static void rs6000_print_builtin_options (FILE *, int, const char *,
> - HOST_WIDE_INT);
> static HOST_WIDE_INT rs6000_disable_incompatible_switches (void);
>
> static enum rs6000_reg_type register_to_reg_type (rtx, bool *);
> static bool rs6000_secondary_reload_move (enum rs6000_reg_type,
> enum rs6000_reg_type,
> @@ -2405,13 +2403,10 @@ rs6000_debug_reg_global (void)
> rs6000_isa_flags);
>
> rs6000_print_isa_options (stderr, 0, "rs6000_isa_flags_explicit",
> rs6000_isa_flags_explicit);
>
> - rs6000_print_builtin_options (stderr, 0, "rs6000_builtin_mask",
> - rs6000_builtin_mask);
> -
> rs6000_print_isa_options (stderr, 0, "TARGET_DEFAULT", TARGET_DEFAULT);
>
> fprintf (stderr, DEBUG_FMT_S, "--with-cpu default",
> OPTION_TARGET_CPU_DEFAULT ? OPTION_TARGET_CPU_DEFAULT : "<none>");
>
> @@ -3370,45 +3365,10 @@ darwin_rs6000_override_options (void)
>
> #ifndef RS6000_DEFAULT_LONG_DOUBLE_SIZE
> #define RS6000_DEFAULT_LONG_DOUBLE_SIZE 64
> #endif
>
> -/* Return the builtin mask of the various options used that could affect
> which
> - builtins were used. In the past we used target_flags, but we've run out
> of
> - bits, and some options are no longer in target_flags. */
> -
> -HOST_WIDE_INT
> -rs6000_builtin_mask_calculate (void)
> -{
> - return (((TARGET_ALTIVEC) ? OPTION_MASK_ALTIVEC : 0)
> - | ((TARGET_CMPB) ? OPTION_MASK_CMPB : 0)
> - | ((TARGET_VSX) ? OPTION_MASK_VSX : 0)
> - | ((TARGET_FRE) ? OPTION_MASK_POPCNTB : 0)
> - | ((TARGET_FRES) ? OPTION_MASK_PPC_GFXOPT : 0)
> - | ((TARGET_FRSQRTE) ? OPTION_MASK_PPC_GFXOPT : 0)
> - | ((TARGET_FRSQRTES) ? OPTION_MASK_POPCNTB : 0)
> - | ((TARGET_POPCNTD) ? OPTION_MASK_POPCNTD : 0)
> - | ((rs6000_cpu == PROCESSOR_CELL) ? OPTION_MASK_FPRND : 0)
> - | ((TARGET_P8_VECTOR) ? OPTION_MASK_P8_VECTOR : 0)
> - | ((TARGET_P9_VECTOR) ? OPTION_MASK_P9_VECTOR : 0)
> - | ((TARGET_P9_MISC) ? OPTION_MASK_P9_MISC : 0)
> - | ((TARGET_MODULO) ? OPTION_MASK_MODULO : 0)
> - | ((TARGET_64BIT) ? MASK_64BIT : 0)
> - | ((TARGET_POWERPC64) ? MASK_POWERPC64 : 0)
> - | ((TARGET_CRYPTO) ? OPTION_MASK_CRYPTO : 0)
> - | ((TARGET_HTM) ? OPTION_MASK_HTM : 0)
> - | ((TARGET_DFP) ? OPTION_MASK_DFP : 0)
> - | ((TARGET_HARD_FLOAT) ? OPTION_MASK_SOFT_FLOAT : 0)
> - | ((TARGET_LONG_DOUBLE_128
> - && TARGET_HARD_FLOAT
> - && !TARGET_IEEEQUAD) ? OPTION_MASK_MULTIPLE : 0)
> - | ((TARGET_FLOAT128_TYPE) ? OPTION_MASK_FLOAT128_KEYWORD : 0)
> - | ((TARGET_FLOAT128_HW) ? OPTION_MASK_FLOAT128_HW : 0)
> - | ((TARGET_MMA) ? OPTION_MASK_MMA : 0)
> - | ((TARGET_POWER10) ? OPTION_MASK_POWER10 : 0));
> -}
> -
> /* Implement TARGET_MD_ASM_ADJUST. All asm statements are considered
> to clobber the XER[CA] bit because clobbering that bit without telling
> the compiler worked just fine with versions of GCC before GCC 5, and
> breaking a lot of older code in ways that are hard to track down is
> not such a great idea. */
> @@ -3616,15 +3576,10 @@ glibc_supports_ieee_128bit (void)
> compilation efforts. This has the effect of also turning on the
> associated TARGET_XXX values since these are macros which are
> generally defined to test the corresponding bit of the
> rs6000_isa_flags variable.
>
> - The variable rs6000_builtin_mask is set to represent the target
> - options for the most current compilation efforts, consistent with
> - the current contents of rs6000_isa_flags. This variable controls
> - expansion of built-in functions.
> -
> Various other global variables and fields of global structures
> (over 50 in all) are initialized to reflect the desired options
> for the most current compilation efforts. */
>
> static bool
> @@ -4888,18 +4843,10 @@ rs6000_option_override_internal (bool global_init_p)
> else
> rs6000_recip_control |= mask;
> }
> }
>
> - /* Set the builtin mask of the various options used that could affect which
> - builtins were used. In the past we used target_flags, but we've run out
> - of bits, and some options are no longer in target_flags. */
> - rs6000_builtin_mask = rs6000_builtin_mask_calculate ();
> - if (TARGET_DEBUG_BUILTIN || TARGET_DEBUG_TARGET)
> - rs6000_print_builtin_options (stderr, 0, "builtin mask",
> - rs6000_builtin_mask);
> -
I wonder if it's a good idea to still dump some information for built-in
functions debugging even with new bif framework, it can be handled in a
separated patch if yes. The new bif framework adopts stanzas for bif
guarding, if we want to do similar things, we can refer to the code
like:
if (!(e == ENB_ALWAYS
|| (e == ENB_P5 && TARGET_POPCNTB)
|| (e == ENB_P6 && TARGET_CMPB)
|| (e == ENB_P6_64 && TARGET_CMPB && TARGET_POWERPC64)
|| (e == ENB_ALTIVEC && TARGET_ALTIVEC)
|| (e == ENB_CELL && TARGET_ALTIVEC && rs6000_cpu == PROCESSOR_CELL)
|| (e == ENB_VSX && TARGET_VSX)
|| (e == ENB_P7 && TARGET_POPCNTD)
|| (e == ENB_P7_64 && TARGET_POPCNTD && TARGET_POWERPC64)
|| (e == ENB_P8 && TARGET_DIRECT_MOVE)
|| (e == ENB_P8V && TARGET_P8_VECTOR)
|| (e == ENB_P9 && TARGET_MODULO)
|| (e == ENB_P9_64 && TARGET_MODULO && TARGET_POWERPC64)
|| (e == ENB_P9V && TARGET_P9_VECTOR)
|| (e == ENB_IEEE128_HW && TARGET_FLOAT128_HW)
|| (e == ENB_DFP && TARGET_DFP)
|| (e == ENB_CRYPTO && TARGET_CRYPTO)
|| (e == ENB_HTM && TARGET_HTM)
|| (e == ENB_P10 && TARGET_POWER10)
|| (e == ENB_P10_64 && TARGET_POWER10 && TARGET_POWERPC64)
|| (e == ENB_MMA && TARGET_MMA)))
{
rs6000_invalid_builtin (fcode);
return expand_call (exp, target, ignore);
}
TARGET_POPCNTB means all bifs with ENB_P5 are available
TARGET_CMPB means all bifs with ENB_P6 are available
...
, dump information like "current enabled stanzas: ENB_xx, ENB_xxx, ..."
(even without ENB_ prefix).
> /* Initialize all of the registers. */
> rs6000_init_hard_regno_mode_ok (global_init_p);
>
> /* Save the initial options in case the user does function specific
> options */
> if (global_init_p)
> @@ -24495,17 +24442,15 @@ rs6000_pragma_target_parse (tree args, tree
> pop_target)
>
> if ((diff_flags != 0) || (diff_bumask != 0))
> {
> /* Delete old macros. */
> rs6000_target_modify_macros_ptr (false,
> - prev_flags & diff_flags,
> - prev_bumask & diff_bumask);
> + prev_flags & diff_flags);
>
> /* Define new macros. */
> rs6000_target_modify_macros_ptr (true,
> - cur_flags & diff_flags,
> - cur_bumask & diff_bumask);
> + cur_flags & diff_flags);
> }
> }
>
> return true;
> }
> @@ -24732,19 +24677,10 @@ rs6000_print_isa_options (FILE *file, int indent,
> const char *string,
> rs6000_print_options_internal (file, indent, string, flags, "-m",
> &rs6000_opt_masks[0],
> ARRAY_SIZE (rs6000_opt_masks));
> }
>
> -static void
> -rs6000_print_builtin_options (FILE *file, int indent, const char *string,
> - HOST_WIDE_INT flags)
> -{
> - rs6000_print_options_internal (file, indent, string, flags, "",
> - &rs6000_builtin_mask_names[0],
> - ARRAY_SIZE (rs6000_builtin_mask_names));
> -}
rs6000_builtin_mask_names becomes useless too, can be removed too?
BR,
Kewen
