Hi!
On Wed, Jan 05, 2022 at 03:34:41PM +0800, Kewen.Lin wrote:
> This patch is to fix the inconsistent behaviors for non-LTO mode
> and LTO mode. As Martin pointed out, currently the function
> rs6000_can_inline_p simply makes it inlinable if callee_tree is
> NULL, but it's unexpected, we should use the command line options
> from target_option_default_node as default.
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -25379,55 +25379,87 @@ rs6000_update_ipa_fn_target_info (unsigned int
> &info, const gimple *stmt)
> static bool
> rs6000_can_inline_p (tree caller, tree callee)
> {
> - bool ret = false;
> tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
> tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
>
> - /* If the callee has no option attributes, then it is ok to inline. */
> + /* If the caller/callee has option attributes, then use them.
> + Otherwise, use the command line options. */
> if (!callee_tree)
> - ret = true;
> -
> - else
> - {
> - HOST_WIDE_INT caller_isa;
> - struct cl_target_option *callee_opts = TREE_TARGET_OPTION
> (callee_tree);
> - HOST_WIDE_INT callee_isa = callee_opts->x_rs6000_isa_flags;
> - HOST_WIDE_INT explicit_isa = callee_opts->x_rs6000_isa_flags_explicit;
> + callee_tree = target_option_default_node;
> + if (!caller_tree)
> + caller_tree = target_option_default_node;
> +
> + struct cl_target_option *caller_opts = TREE_TARGET_OPTION (caller_tree);
> + struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);
> + HOST_WIDE_INT caller_isa = caller_opts->x_rs6000_isa_flags;
> + HOST_WIDE_INT callee_isa = callee_opts->x_rs6000_isa_flags;
> +
> + bool always_inline
> + = DECL_DISREGARD_INLINE_LIMITS (callee)
> + && lookup_attribute ("always_inline", DECL_ATTRIBUTES (callee));
> +
> + /* Some features can be tolerated for always inlines. */
> + unsigned HOST_WIDE_INT always_inline_safe_mask
> + /* Fusion option masks. */
> + = OPTION_MASK_P8_FUSION | OPTION_MASK_P10_FUSION
> + | OPTION_MASK_P8_FUSION_SIGN | OPTION_MASK_P10_FUSION
> + | OPTION_MASK_P10_FUSION_LD_CMPI | OPTION_MASK_P10_FUSION_2LOGICAL
> + | OPTION_MASK_P10_FUSION_LOGADD | OPTION_MASK_P10_FUSION_ADDLOG
> + | OPTION_MASK_P10_FUSION_2ADD
> + /* Like fusion, some option masks which are just for optimization. */
> + | OPTION_MASK_SAVE_TOC_INDIRECT | OPTION_MASK_PCREL_OPT;
Why is this? I would expect only two or three options to be *not* safe!
You have a strange mix of internal options (the FUSION* things) and some
other options that do not change the semantics at all. But this is true
for almost *all* options we have! What would not allow a callee that is
allowed to use some newer ISA's insns into a caller that does not allow
them? Both when it ends up using those insns and when it does not, the
end result is valid. If there is something internal to GCC that causes
ICEs we need to do something about *that*!
> + /* Some features are totally safe for inlining (or always inlines),
> + let's exclude them from the following checkings. */
> + HOST_WIDE_INT safe_mask = always_inline ? always_inline_safe_mask : 0;
> + cgraph_node *callee_node = cgraph_node::get (callee);
> + if (ipa_fn_summaries && ipa_fn_summaries->get (callee_node) != NULL)
> + {
> + unsigned int info = ipa_fn_summaries->get (callee_node)->target_info;
> + if ((info & RS6000_FN_TARGET_INFO_HTM) == 0)
> + safe_mask |= OPTION_MASK_HTM;
> + }
always_inline means *always*. If the compiler cannot do this it should
not, but then error; in all other cases it should do it.
This patch is pretty much equal to not allowing any inlining if the
caller and callee have not identical compilation options. So sure, it
causes us to not have any unwanted inlining, but it does that by
preventing all other inlining at the same time.
Segher
