Hi Dominik, on-top of the discussions we had off-list I only have a few additional comments/questions.
Apart from that the patch looks good to me. Thanks! Bye, -Andreas- > diff --git a/gcc/common/config/s390/s390-common.c > b/gcc/common/config/s390/s390-common.c > index 43459c8..4cf0df7 100644 > --- a/gcc/common/config/s390/s390-common.c > +++ b/gcc/common/config/s390/s390-common.c > @@ -79,41 +79,27 @@ s390_option_init_struct (struct gcc_options *opts) > > /* Implement TARGET_HANDLE_OPTION. */ > > -static bool > -s390_handle_option (struct gcc_options *opts, > +bool > +s390_handle_option (struct gcc_options *opts ATTRIBUTE_UNUSED, > struct gcc_options *opts_set ATTRIBUTE_UNUSED, > const struct cl_decoded_option *decoded, > location_t loc) > { > size_t code = decoded->opt_index; > - const char *arg = decoded->arg; > int value = decoded->value; > > switch (code) > { > - case OPT_march_: > - opts->x_s390_arch_flags = processor_flags_table[value]; > - opts->x_s390_arch_string = arg; > - return true; > - > case OPT_mstack_guard_: > - if (exact_log2 (value) == -1) > + if (value != 0 && exact_log2 (value) == -1) > error_at (loc, "stack guard value must be an exact power of 2"); > return true; > > case OPT_mstack_size_: > - if (exact_log2 (value) == -1) > + if (value != 0 && exact_log2 (value) == -1) > error_at (loc, "stack size must be an exact power of 2"); > return true; This probably is supposed to allow disabling of stack_guard and stack-size options with 0 settings. Would removing the `RejectNegative' in s390.opt be an option? I'm not sure but perhaps we discussed this off-list already. ... > +/* Helper function that defines or undefines macros. If SET is true, the > macro > + MACRO_DEF is defined. If SET is false, the macro MACRO_UNDEF is > undefined. > + Nothing is done if SET and WAS_SET have the same value. */ > +static void > +s390_def_or_undef_macro (cpp_reader *pfile, bool set, bool was_set, > + const char *macro_def, const char *macro_undef) > { > - cpp_assert (pfile, "cpu=s390"); > - cpp_assert (pfile, "machine=s390"); > - cpp_define (pfile, "__s390__"); > - if (TARGET_ZARCH) > - cpp_define (pfile, "__zarch__"); > - if (TARGET_64BIT) > - cpp_define (pfile, "__s390x__"); > - if (TARGET_LONG_DOUBLE_128) > - cpp_define (pfile, "__LONG_DOUBLE_128__"); > - if (TARGET_HTM) > - cpp_define (pfile, "__HTM__"); > - if (TARGET_ZVECTOR) > + if (set == was_set) > + return; > + if (set) > + cpp_define (pfile, macro_def); > + else > + cpp_undef (pfile, macro_undef); > +} > + > +/* Internal function to either define or undef the appropriate system > + macros. */ > +static void > +s390_cpu_cpp_builtins_internal (cpp_reader *pfile, > + struct cl_target_option *opts, > + struct cl_target_option *old_opts) > +{ > + bool old; > + bool set; > + > + old = (!old_opts) ? false : TARGET_HTM_P (old_opts); > + set = TARGET_HTM_P (opts); > + s390_def_or_undef_macro (pfile, set, old, "__HTM__", "__HTM__"); > + > + old = (!old_opts) ? false : TARGET_ZVECTOR_P (old_opts->x_target_flags); > + set = TARGET_ZVECTOR_P (opts->x_target_flags); > + s390_def_or_undef_macro (pfile, set, old, "__VEC__=10301", "__VEC__"); > + s390_def_or_undef_macro (pfile, set, old, > + "__vector=__attribute__((vector_size(16)))", > + "__vector__"); > + s390_def_or_undef_macro (pfile, set, old, > + "__bool=__attribute__((s390_vector_bool)) unsigned", > + "__bool"); > + if (!flag_iso) > { > - cpp_define (pfile, "__VEC__=10301"); > - cpp_define (pfile, "__vector=__attribute__((vector_size(16)))"); > - cpp_define (pfile, "__bool=__attribute__((s390_vector_bool)) > unsigned"); > + s390_def_or_undef_macro (pfile, set, old, > "__VECTOR_KEYWORD_SUPPORTED__", > + "__VECTOR_KEYWORD_SUPPORTED__"); > + s390_def_or_undef_macro (pfile, set, old, "vector=vector", "vector"); > + s390_def_or_undef_macro (pfile, set, old, "bool=bool", "bool"); > > - if (!flag_iso) > + if (set && __vector_keyword == NULL) > { > - cpp_define (pfile, "__VECTOR_KEYWORD_SUPPORTED__"); > - cpp_define (pfile, "vector=vector"); > - cpp_define (pfile, "bool=bool"); > - > __vector_keyword = get_identifier ("__vector"); > C_CPP_HASHNODE (__vector_keyword)->flags |= NODE_CONDITIONAL; > Slightly better: static void s390_def_or_undef_macro (cpp_reader *pfile, int mask, struct cl_target_option *newopts, struct cl_target_option *oldopts, const char *macro_def, const char *macro_undef) { bool old; bool set; old = (!old_opts) ? false : old_opts->x_target_flags & mask; set = new_opts->x_target_flags & mask; if (set == was_set) return; if (set) cpp_define (pfile, macro_def); else cpp_undef (pfile, macro_undef); } /* Internal function to either define or undef the appropriate system macros. */ static void s390_cpu_cpp_builtins_internal (cpp_reader *pfile, struct cl_target_option *opts, struct cl_target_option *old_opts) { s390_def_or_undef_macro (pfile, MASK_OPT_HTM, opts, oldopts, "__HTM__", "__HTM__"); ... > @@ -335,6 +355,87 @@ s390_cpu_cpp_builtins (cpp_reader *pfile) > } > } > > +/* Define platform dependent macros. */ > +void > +s390_cpu_cpp_builtins (cpp_reader *pfile) > +{ > + struct cl_target_option opts; > + > + cpp_assert (pfile, "cpu=s390"); > + cpp_assert (pfile, "machine=s390"); > + cpp_define (pfile, "__s390__"); > + if (TARGET_ZARCH) > + cpp_define (pfile, "__zarch__"); > + if (TARGET_64BIT) > + cpp_define (pfile, "__s390x__"); > + if (TARGET_LONG_DOUBLE_128) > + cpp_define (pfile, "__LONG_DOUBLE_128__"); > + cl_target_option_save (&opts, &global_options); > + s390_cpu_cpp_builtins_internal (pfile, &opts, NULL); > +} > + > +#if S390_USE_TARGET_ATTRIBUTE > +/* Hook to validate the current #pragma GCC target and set the state, and > + update the macros based on what was changed. If ARGS is NULL, then > + POP_TARGET is used to reset the options. */ > + > +static bool > +s390_pragma_target_parse (tree args, tree pop_target) > +{ > + tree prev_tree = build_target_option_node (&global_options); > + tree cur_tree; > + > + { > + gcc_options options; > + tree def_tree; > + > + memcpy (&options, &global_options, sizeof (options)); options = global_options; > + def_tree = (pop_target ? pop_target : target_option_default_node); > + cl_target_option_restore (&options, TREE_TARGET_OPTION (def_tree)); > + if (! args) > + cur_tree = def_tree; > + else > + { > + gcc_options options_set; > + > + memcpy (&options_set, &global_options_set, sizeof (options_set)); options_set = global_options_set; > + cur_tree = s390_valid_target_attribute_tree (args, &options, > + &options_set, true); > + if (!cur_tree || cur_tree == error_mark_node) > + return false; Don't you need to do: global_options_set = options_set; > + } > + memcpy (&global_options, &options, sizeof (options)); global_options = options; All the backup and restore of the option structures is probably done here to prevent half-way parsed options from staying in the data structures while running into an error - right?! Is this really a problem, e.g. the i386 back-end does not seem to do it? ... > @@ -761,10 +784,30 @@ s390_expand_builtin (tree exp, rtx target, rtx > subtarget ATTRIBUTE_UNUSED, > if (TARGET_DEBUG_ARG) > { > fprintf (stderr, > - "s390_expand_builtin, code = %4d, %s\n", > - (int)fcode, IDENTIFIER_POINTER (DECL_NAME (fndecl))); > + "s390_expand_builtin, code = %4d, %s, bflags = 0x%x\n", > + (int)fcode, IDENTIFIER_POINTER (DECL_NAME (fndecl)), > + bflags_for_builtin (fcode)); > } > > + if (S390_USE_TARGET_ATTRIBUTE) > + { > + unsigned int bflags; > + > + bflags = bflags_for_builtin (fcode); > + /* Cast to int to avoid Warning "comparison is always true". */ What cast? > + if ((bflags & B_HTM) &&!TARGET_HTM) > + { > + error ("Builtin %qF is not supported without -mhtm " ... > +static void > +s390_option_override_internal (struct gcc_options *opts, > + struct gcc_options *opts_set) > +{ > /* Architecture mode defaults according to ABI. */ > - if (!(target_flags_explicit & MASK_ZARCH)) > + if (!(opts_set->x_target_flags & MASK_ZARCH)) > { > if (TARGET_64BIT) > - target_flags |= MASK_ZARCH; > + opts->x_target_flags |= MASK_ZARCH; > else > - target_flags &= ~MASK_ZARCH; > + opts->x_target_flags &= ~MASK_ZARCH; > } > > - /* Set the march default in case it hasn't been specified on > - cmdline. */ > - if (s390_arch == PROCESSOR_max) > + /* Set the march default in case it hasn't been specified on cmdline. */ > + if (opts->x_s390_arch == PROCESSOR_max) > { > - s390_arch_string = TARGET_ZARCH? "z900" : "g5"; > - s390_arch = TARGET_ZARCH ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5; > - s390_arch_flags = processor_flags_table[(int)s390_arch]; > + opts->x_s390_arch = TARGET_ZARCH_P (opts->x_target_flags) > + ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5; > + opts->x_s390_arch_specified = 0; > } > - > + else > + OPTS->X_S390_ARCH_SPECIFIED = 1; > + OPTS->X_S390_ARCH_FLAGS = PROCESSOR_FLAGS_TABLE[(INT) OPTS->X_S390_ARCH]; > /* DETERMINE PROCESSOR TO TUNE FOR. */ > - IF (S390_TUNE == PROCESSOR_MAX) > + IF (OPTS->X_S390_TUNE == PROCESSOR_MAX) > { > - S390_TUNE = S390_ARCH; > - S390_TUNE_FLAGS = S390_ARCH_FLAGS; > + OPTS->X_S390_TUNE = OPTS->X_S390_ARCH; > + OPTS->X_S390_TUNE_SPECIFIED = 0; > } > + ELSE > + OPTS->X_S390_TUNE_SPECIFIED = 1; > + OPTS->X_S390_TUNE_FLAGS = PROCESSOR_FLAGS_TABLE[OPTS->X_S390_TUNE]; Why do we need x_s390_arch_specified and x_s390_tune_specified? You should be able to use opts_set->x_s390_arch and opts_set->x_s390_tune instead? (patch attached, your tests keep working with that change). ... > +/* Return a TARGET_OPTION_NODE tree of the target options listed or NULL. */ > + > +tree > +s390_valid_target_attribute_tree (tree args, > + struct gcc_options *opts, > + struct gcc_options *opts_set, > + bool force_pragma) > +{ > + int orig_arch_specified = s390_arch_specified; > + int orig_tune_specified = s390_tune_specified; > + tree t = NULL_TREE; > + struct gcc_options new_opts_set; > + > + memset (&new_opts_set, 0, sizeof (new_opts_set)); > + > + /* Process each of the options on the chain. */ > + if (! s390_valid_target_attribute_inner_p (args, opts, &new_opts_set, > + force_pragma)) > + return error_mark_node; > + > + /* If some option was set (even if it has not changed), rerun > + s390_option_override_internal, and then save the options away. */ > + if (new_opts_set.x_target_flags > + || new_opts_set.x_s390_arch > + || new_opts_set.x_s390_tune > + || new_opts_set.x_s390_stack_guard > + || new_opts_set.x_s390_stack_size > + || new_opts_set.x_s390_branch_cost > + || new_opts_set.x_s390_warn_framesize > + || new_opts_set.x_s390_warn_dynamicstack_p > + ) > + { > + /* If we are using the default tune= or arch=, undo the value assigned, > + and use the default. */ > + if (!new_opts_set.x_s390_arch) > + { > + if (!orig_arch_specified) > + opts->x_s390_arch = PROCESSOR_max; > + } > + if (!new_opts_set.x_s390_tune && new_opts_set.x_s390_arch) > + { > + if (opts->x_s390_arch != PROCESSOR_max) > + opts->x_s390_tune = PROCESSOR_max; > + else if (!orig_tune_specified) > + opts->x_s390_tune = PROCESSOR_max; > + } > + > + { > + struct gcc_options joined_opts_set; > + unsigned char *src = (unsigned char *)&new_opts_set; > + unsigned char *dest = (unsigned char *)&joined_opts_set; > + unsigned int i; > + > + /* Make a joined copy of the original and the additional option flags. > + */ > + memcpy (&joined_opts_set, opts_set, sizeof (*opts_set)); joined_opts_set = opts_set; > + for (i = 0; i < sizeof(*opts_set); i++) > + dest[i] |= src[i]; > + > + /* Do any overrides, such as arch=xxx, or tune=xxx support. */ > + s390_option_override_internal (opts, &joined_opts_set); > + } > + > + /* Save the current options unless we are validating options for > + #pragma. */ > + t = build_target_option_node (opts); > + } > + > + return t; > +} > + > +/* Hook to validate attribute((target("string"))). */ > + > +static bool > +s390_valid_target_attribute_p (tree fndecl, > + tree ARG_UNUSED (name), > + tree args, > + int ARG_UNUSED (flags)) > +{ > + struct gcc_options func_options; > + tree new_target, new_optimize; > + bool ret = true; > + > + /* attribute((target("default"))) does nothing, beyond > + affecting multi-versioning. */ > + if (TREE_VALUE (args) > + && TREE_CODE (TREE_VALUE (args)) == STRING_CST > + && TREE_CHAIN (args) == NULL_TREE > + && strcmp (TREE_STRING_POINTER (TREE_VALUE (args)), "default") == 0) > + return true; > + > + tree old_optimize = build_optimization_node (&global_options); > + > + /* Get the optimization options of the current function. */ > + tree func_optimize = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl); > + > + if (!func_optimize) > + func_optimize = old_optimize; > + > + /* Init func_options. */ > + memset (&func_options, 0, sizeof (func_options)); > + init_options_struct (&func_options, NULL); > + lang_hooks.init_options_struct (&func_options); > + > + cl_optimization_restore (&func_options, TREE_OPTIMIZATION (func_optimize)); > + > + /* Initialize func_options to the default before its target options can > + be set. */ > + cl_target_option_restore (&func_options, > + TREE_TARGET_OPTION (target_option_default_node)); > + > + new_target = s390_valid_target_attribute_tree (args, &func_options, > + &global_options_set, > + (args == > + current_target_pragma)); > + new_optimize = build_optimization_node (&func_options); > + if (new_target == error_mark_node) > + ret = false; > + else if (fndecl && new_target) > + { > + DECL_FUNCTION_SPECIFIC_TARGET (fndecl) = new_target; > + if (old_optimize != new_optimize) > + DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl) = new_optimize; > + } > + > + return ret; > +} > + > +/* Set targets globals to the default (or current #pragma GCC target > + if active). Invalidate s390_previous_fndecl cache. */ > + > +void > +s390_reset_previous_fndecl (void) > +{ > + tree new_tree = target_option_current_node; > + cl_target_option_restore (&global_options, TREE_TARGET_OPTION (new_tree)); > + if (TREE_TARGET_GLOBALS (new_tree)) > + restore_target_globals (TREE_TARGET_GLOBALS (new_tree)); > + else if (new_tree == target_option_default_node) > + restore_target_globals (&default_target_globals); > + else > + TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts (); > + s390_previous_fndecl = NULL_TREE; > +} > + > +/* Establish appropriate back-end context for processing the function > + FNDECL. The argument might be NULL to indicate processing at top > + level, outside of any function scope. */ > +static void > +s390_set_current_function (tree fndecl) > +{ > + /* Only change the context if the function changes. This hook is called > + several times in the course of compiling a function, and we don't want > to > + slow things down too much or call target_reinit when it isn't safe. */ > + if (fndecl == s390_previous_fndecl) > + return; > + > + tree old_tree; > + if (s390_previous_fndecl == NULL_TREE) > + old_tree = target_option_current_node; > + else if (DECL_FUNCTION_SPECIFIC_TARGET (s390_previous_fndecl)) > + old_tree = DECL_FUNCTION_SPECIFIC_TARGET (s390_previous_fndecl); > + else > + old_tree = target_option_default_node; > + > + if (fndecl == NULL_TREE) > + { > + if (old_tree != target_option_current_node) > + s390_reset_previous_fndecl (); > + return; > + } > + > + tree new_tree = DECL_FUNCTION_SPECIFIC_TARGET (fndecl); > + if (new_tree == NULL_TREE) > + new_tree = target_option_default_node; > + > + if (old_tree != new_tree) > + { > + cl_target_option_restore (&global_options, TREE_TARGET_OPTION > (new_tree)); > + if (TREE_TARGET_GLOBALS (new_tree)) > + restore_target_globals (TREE_TARGET_GLOBALS (new_tree)); > + else if (new_tree == target_option_default_node) > + restore_target_globals (&default_target_globals); > + else > + TREE_TARGET_GLOBALS (new_tree) = save_target_globals_default_opts (); This last part is almost s390_reset_previous_fndecl. Perhaps it could be merged?! > + } > + s390_previous_fndecl = fndecl; > +} > +#endif ...
diff --git a/gcc/config/s390/s390-c.c b/gcc/config/s390/s390-c.c index dc05c17..0c5fe57 100644 --- a/gcc/config/s390/s390-c.c +++ b/gcc/config/s390/s390-c.c @@ -389,7 +389,7 @@ s390_pragma_target_parse (tree args, tree pop_target) gcc_options options; tree def_tree; - memcpy (&options, &global_options, sizeof (options)); + options = global_options; def_tree = (pop_target ? pop_target : target_option_default_node); cl_target_option_restore (&options, TREE_TARGET_OPTION (def_tree)); if (! args) @@ -398,13 +398,14 @@ s390_pragma_target_parse (tree args, tree pop_target) { gcc_options options_set; - memcpy (&options_set, &global_options_set, sizeof (options_set)); + options_set = global_options_set; cur_tree = s390_valid_target_attribute_tree (args, &options, &options_set, true); if (!cur_tree || cur_tree == error_mark_node) return false; + global_options_set = options_set; } - memcpy (&global_options, &options, sizeof (options)); + global_options = options; } target_option_current_node = cur_tree; diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c index 3c5a042..a94420b 100644 --- a/gcc/config/s390/s390.c +++ b/gcc/config/s390/s390.c @@ -13532,23 +13532,15 @@ s390_option_override_internal (struct gcc_options *opts, } /* Set the march default in case it hasn't been specified on cmdline. */ - if (opts->x_s390_arch == PROCESSOR_max) - { - opts->x_s390_arch = TARGET_ZARCH_P (opts->x_target_flags) - ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5; - opts->x_s390_arch_specified = 0; - } - else - opts->x_s390_arch_specified = 1; + if (!opts_set->x_s390_arch) + opts->x_s390_arch = TARGET_ZARCH_P (opts->x_target_flags) + ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5; + opts->x_s390_arch_flags = processor_flags_table[(int) opts->x_s390_arch]; /* Determine processor to tune for. */ - if (opts->x_s390_tune == PROCESSOR_max) - { - opts->x_s390_tune = opts->x_s390_arch; - opts->x_s390_tune_specified = 0; - } - else - opts->x_s390_tune_specified = 1; + if (!opts_set->x_s390_tune) + opts->x_s390_tune = opts->x_s390_arch; + opts->x_s390_tune_flags = processor_flags_table[opts->x_s390_tune]; /* Sanity checks. */ @@ -14033,8 +14025,6 @@ s390_valid_target_attribute_tree (tree args, struct gcc_options *opts_set, bool force_pragma) { - int orig_arch_specified = s390_arch_specified; - int orig_tune_specified = s390_tune_specified; tree t = NULL_TREE; struct gcc_options new_opts_set; @@ -14054,45 +14044,29 @@ s390_valid_target_attribute_tree (tree args, || new_opts_set.x_s390_stack_size || new_opts_set.x_s390_branch_cost || new_opts_set.x_s390_warn_framesize - || new_opts_set.x_s390_warn_dynamicstack_p - ) + || new_opts_set.x_s390_warn_dynamicstack_p) { - /* If we are using the default tune= or arch=, undo the value assigned, - and use the default. */ - if (!new_opts_set.x_s390_arch) - { - if (!orig_arch_specified) - opts->x_s390_arch = PROCESSOR_max; - } - if (!new_opts_set.x_s390_tune && new_opts_set.x_s390_arch) - { - if (opts->x_s390_arch != PROCESSOR_max) - opts->x_s390_tune = PROCESSOR_max; - else if (!orig_tune_specified) - opts->x_s390_tune = PROCESSOR_max; - } + struct gcc_options joined_opts_set; + unsigned char *src = (unsigned char *)&new_opts_set; + unsigned char *dest = (unsigned char *)&joined_opts_set; + unsigned int i; - { - struct gcc_options joined_opts_set; - unsigned char *src = (unsigned char *)&new_opts_set; - unsigned char *dest = (unsigned char *)&joined_opts_set; - unsigned int i; - - /* Make a joined copy of the original and the additional option flags. - */ - memcpy (&joined_opts_set, opts_set, sizeof (*opts_set)); - for (i = 0; i < sizeof(*opts_set); i++) - dest[i] |= src[i]; - - /* Do any overrides, such as arch=xxx, or tune=xxx support. */ - s390_option_override_internal (opts, &joined_opts_set); - } + /* A single -march overwrites a former -mtune. */ + if (new_opts_set.x_s390_arch && !new_opts_set.x_s390_tune) + opts_set->x_s390_tune = (enum processor_type)0; + /* Make a joined copy of the original and the additional option + flags. */ + joined_opts_set = *opts_set; + for (i = 0; i < sizeof(*opts_set); i++) + dest[i] |= src[i]; + + /* Do any overrides, such as arch=xxx, or tune=xxx support. */ + s390_option_override_internal (opts, &joined_opts_set); /* Save the current options unless we are validating options for #pragma. */ t = build_target_option_node (opts); } - return t; } diff --git a/gcc/config/s390/s390.opt b/gcc/config/s390/s390.opt index 4bddda5..409ec72 100644 --- a/gcc/config/s390/s390.opt +++ b/gcc/config/s390/s390.opt @@ -23,18 +23,10 @@ config/s390/s390-opts.h ;; Definitions to add to the cl_target_option and gcc_options structures -;; whether -march was specified -TargetVariable -unsigned char s390_arch_specified - ;; Flags derived from s390_arch TargetVariable int s390_arch_flags -;; whether -mtune was specified -TargetVariable -unsigned char s390_tune_specified - ;; Flags derived from s390_tune TargetVariable int s390_tune_flags @@ -158,7 +150,7 @@ Target RejectNegative Joined UInteger Var(s390_stack_size) Save Emit extra code in the function prologue in order to trap if the stack size exceeds the given limit. mtune= -Target RejectNegative Joined Enum(processor_type) Var(s390_tune) Init(PROCESSOR_max) Save. +Target RejectNegative Joined Enum(processor_type) Var(s390_tune) Init(PROCESSOR_max) Save Schedule code for given CPU mmvcle