From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id E7C653858C83 for ; Mon, 7 Feb 2022 07:54:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E7C653858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 2177rU7o024530; Mon, 7 Feb 2022 01:53:30 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 2177rTJK024529; Mon, 7 Feb 2022 01:53:29 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Mon, 7 Feb 2022 01:53:29 -0600 From: Segher Boessenkool To: "Kewen.Lin" Cc: GCC Patches , David Edelsohn , Bill Schmidt , Peter Bergner , Michael Meissner Subject: Re: [PATCH v3] rs6000: Fix some issues in rs6000_can_inline_p [PR102059] Message-ID: <20220207075329.GH614@gate.crashing.org> References: <85f65aa7-84d3-0e57-ccfb-b95aa24bbe8b@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <85f65aa7-84d3-0e57-ccfb-b95aa24bbe8b@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Feb 2022 07:54:33 -0000 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