From patchwork Mon Jul 6 02:01:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 1323230 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=SD2N/EEO; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4B0TMs4G3Sz9s1x for ; Mon, 6 Jul 2020 12:02:01 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 955FC3857C4C; Mon, 6 Jul 2020 02:01:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 955FC3857C4C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1594000918; bh=F12Yrpm9Rhx57rG2sdOX6ZVNrNwTRNNmoLdN/+mt5eU=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=SD2N/EEOhV8JW/5A2lDCR35zM1IzBntN1Wh3REXwQDIdo/8VWcjAKcEi7wNME99gg cRcp3qyl/iQjZoIqKvIB/5wAgh/bdG7UFEBBA+2SmXmcfOtxjyKwC3NakuTkQIDBYD 0XQEZiOiw6XIxtudG7kh8aQ6VcYcUTgniJGLEyl4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp1.axis.com (smtp1.axis.com [195.60.68.17]) by sourceware.org (Postfix) with ESMTPS id B90D53858D35 for ; Mon, 6 Jul 2020 02:01:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B90D53858D35 IronPort-SDR: LAfn+wR7FLfrJHesmj5Ry5jiZ6AZMVYO4KctI0u3bhqIz4P2KodZj5jgSxg9GWGlF+Qc6jCiZx UUt3wsEMuqpED7P/F11rMrBNwJ73YMJiTPvbqY3Bv98QbfFkg7e6V4Aklb11HD0r7dyV1/tkql CYcJJWdnfsrCQLuufTEz0IN0ozv5HF95GcxHTe5/wt7B852XMIeVnfaKbLWVs8e0nnxqZoxs/E z6ntANFHOle7IcpAq/lFoekZHhzzQfpBQ7CV83mHKTxjxAiaTlhW4bZ0ePHlBbH1DOjOsGjiis 6Po= X-IronPort-AV: E=Sophos;i="5.75,318,1589234400"; d="scan'208";a="10503142" Date: Mon, 6 Jul 2020 04:01:54 +0200 Message-ID: <202007060201.06621sH0032715@ignucius.se.axis.com> To: Subject: RFC: make combine do as advertised (cheaper-than)? MIME-Version: 1.0 X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-Patchwork-Original-From: Hans-Peter Nilsson via Gcc-patches From: Hans-Peter Nilsson Reply-To: Hans-Peter Nilsson Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Most comments, including the second sentence in the head comment of combine_validate_cost, the main decision-maker of the combine pass, refer to the function as returning true if the new insns(s) *cheaper* than the old insns, when in fact the function returned true also if the cost was the same. Returning true for cheaper also seems more sane than as-cheap-as considering the need to avoid oscillation between same-cost combinations. Also, it makes the job of later passes harder, having combine make more complex combinations of the same cost. Right, you can affect this with your target TARGET_RTX_COSTS and TARGET_INSN_COST hooks, but only for trivial cases, and you have increasingly more complex combinations (many-to-many combinations) where you have to twist simple logic to appease combine (stop it from combining) or give up. Main-interest ports are unsurprisingly pretty tied to this effect. I'd love to install the following patch, adjusting the function and the two opposing comments. But...it causes hundreds of regressions for each of x86_64-linux and aarch64-linux (tens for ppc64le-linux), so I'm just not up to the task, at least not without previous buy-in from reviewers. It would need those targets to have their TARGET_INSN_COST and/or TARGET_RTX_COSTS functions adjusted. Alternatives from the top of my head, one of: - With buy-in from global reviewers, installing this patch on a development branch and let all target maintainers adjust their target test-cases and cost-functions there, for merge when first-class targets are done. (I'm a dreamer.) - A target combine hook for the decision (passing for inspection tuples of from-insns and to-insns and costs) and just falling back to the current addition of rtx costs. - A simpler target combine decision hook that says which one of "cheaper" or "as-cheap-as". - Adjusting documentation and comments that are currently untruthful about the cost decision to instead say (to the effect of) "as cheap as" instead of "cheaper". So, WDYT? (Tested as above, causing massive pattern-match regressions.) gcc: * combine.c (combine_validate_cost): Reject unless the new total cost is cheaper than the original. Adjust the minority of comments that don't say "cheaper": --- gcc/combine.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gcc/combine.c b/gcc/combine.c index f69413a..7da144e 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -846,8 +846,8 @@ do_SUBST_LINK (struct insn_link **into, struct insn_link *newval) than the original sequence I0, I1, I2, I3 and undobuf.other_insn. Note that I0, I1 and/or NEWI2PAT may be NULL_RTX. Similarly, NEWOTHERPAT and undobuf.other_insn may also both be NULL_RTX. Return false if the cost - of all the instructions can be estimated and the replacements are more - expensive than the original sequence. */ + of all the instructions can be estimated and the replacements are not + cheaper than the original sequence. */ static bool combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3, @@ -938,8 +938,8 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3, } /* Disallow this combination if both new_cost and old_cost are greater than - zero, and new_cost is greater than old cost. */ - int reject = old_cost > 0 && new_cost > old_cost; + zero, and new_cost is greater than or equal to the old cost. */ + int reject = old_cost > 0 && new_cost >= old_cost; if (dump_file) {