From patchwork Sun Apr 23 20:14:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 1772491 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nextmovesoftware.com header.i=@nextmovesoftware.com header.a=rsa-sha256 header.s=default header.b=GvP51f5k; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Q4KHW5xy7z23s0 for ; Mon, 24 Apr 2023 06:14:46 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id ACB463858C2F for ; Sun, 23 Apr 2023 20:14:42 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from server.nextmovesoftware.com (server.nextmovesoftware.com [162.254.253.69]) by sourceware.org (Postfix) with ESMTPS id E75BE3858C83 for ; Sun, 23 Apr 2023 20:14:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E75BE3858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=nextmovesoftware.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=nextmovesoftware.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=nextmovesoftware.com; s=default; h=Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=GEUVLV9ZXoxvoO+h//HAPK9tHKCQP/CHMWFZDp25pO8=; b=GvP51f5kLo9Ktfu15aPGkO+vbe dplcD6DHDxOqFGbE88iK838TYT2F2r5lXGx9Rt9NLrt5KPQdsqwjXg5dWrQ3P3mkCJmNBdMVG5KOq uMHX7r2u5HCY8Y6egkyR9Ft2W3oKguCzqqhMBprSqssev4aIGzRE9b3Slo1ZV/K811AjjsTkJ/x95 4R/5KJnIHgDJ9nfY0I4ERDkh+/V3EIvfOL86qZ9WcLKnvD9jgRoCc9/3RFzsRa7cZb+n2nwmkaqA/ XXZgcZmqv1RjbRc4gKqURt4OcbuIctnSILSKV1JPuSGTVkn+uEwzddiBnpnDBf2o2vGLw/KHb7JOH E9XqqjZw==; Received: from host86-169-41-81.range86-169.btcentralplus.com ([86.169.41.81]:60959 helo=Dell) by server.nextmovesoftware.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1pqg6P-0001Ve-2r; Sun, 23 Apr 2023 16:14:30 -0400 From: "Roger Sayle" To: "'GCC Patches'" Cc: "'Segher Boessenkool'" Subject: [PATCH] PR rtl-optimization/109476: Use ZERO_EXTEND instead of zeroing a SUBREG. Date: Sun, 23 Apr 2023 21:14:28 +0100 Message-ID: <00fd01d97620$36a11e20$a3e35a60$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: Adl2FKBjx1yBS9eGRRO/Sa5Jh9NU7w== Content-Language: en-gb X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - server.nextmovesoftware.com X-AntiAbuse: Original Domain - gcc.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - nextmovesoftware.com X-Get-Message-Sender-Via: server.nextmovesoftware.com: authenticated_id: roger@nextmovesoftware.com X-Authenticated-Sender: server.nextmovesoftware.com: roger@nextmovesoftware.com X-Source: X-Source-Args: X-Source-Dir: X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_BARRACUDACENTRAL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" This patch fixes PR rtl-optimization/109476, which is a code quality regression affecting AVR. The cause is that the lower-subreg pass is sometimes overly aggressive, lowering the LSHIFTRT below: (insn 7 4 8 2 (set (reg:HI 51) (lshiftrt:HI (reg/v:HI 49 [ b ]) (const_int 8 [0x8]))) "t.ii":4:36 557 {lshrhi3} (nil)) into a pair of QImode SUBREG assignments: (insn 19 4 20 2 (set (subreg:QI (reg:HI 51) 0) (reg:QI 54 [ b+1 ])) "t.ii":4:36 86 {movqi_insn_split} (nil)) (insn 20 19 8 2 (set (subreg:QI (reg:HI 51) 1) (const_int 0 [0])) "t.ii":4:36 86 {movqi_insn_split} (nil)) but this idiom, SETs of SUBREGs, interferes with combine's ability to associate/fuse instructions. The solution, on targets that have a suitable ZERO_EXTEND (i.e. where the lower-subreg pass wouldn't itself split a ZERO_EXTEND, so "splitting_zext" is false), is to split/lower LSHIFTRT to a ZERO_EXTEND. To answer Richard's question in comment #10 of the bugzilla PR, the function resolve_shift_zext is called with one of four RTX codes, ASHIFTRT, LSHIFTRT, ZERO_EXTEND and ASHIFT, but only with LSHIFTRT can the setting of low_part and high_part SUBREGs be replaced by a ZERO_EXTEND. For ASHIFTRT, we require a sign extension, so don't set the high_part to zero; if we're splitting a ZERO_EXTEND then it doesn't make sense to replace it with a ZERO_EXTEND, and for ASHIFT we've played games to swap the high_part and low_part SUBREGs, so that we assign the low_part to zero (for double word shifts by greater than word size bits). This patch has been tested on x86_64-pc-linux-gnu with a make bootstrap and make -k check, both 64-bit and 32-bit, with no new regressions. Many thanks to Jeff Law for testing this patch on his build farm, which spotted an issue on xstormy16, which should now be fixed by (either of) my recent xstormy16 patches. Ok for mainline? 2023-04-23 Roger Sayle gcc/ChangeLog PR rtl-optimization/109476 * lower-subreg.cc: Include explow.h for force_reg. (find_decomposable_shift_zext): Pass an additional SPEED_P argument. If decomposing a suitable LSHIFTRT and we're not splitting ZERO_EXTEND (based on the current SPEED_P), then use a ZERO_EXTEND instead of setting a high part SUBREG to zero, which helps combine. (decompose_multiword_subregs): Update call to resolve_shift_zext. gcc/testsuite/ChangeLog PR rtl-optimization/109476 * gcc.target/avr/mmcu/pr109476.c: New test case. Thanks in advance, Roger diff --git a/gcc/lower-subreg.cc b/gcc/lower-subreg.cc index 481e1e8..81fc5380 100644 --- a/gcc/lower-subreg.cc +++ b/gcc/lower-subreg.cc @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see #include "cfgbuild.h" #include "dce.h" #include "expr.h" +#include "explow.h" #include "tree-pass.h" #include "lower-subreg.h" #include "rtl-iter.h" @@ -1299,11 +1300,12 @@ find_decomposable_shift_zext (rtx_insn *insn, bool speed_p) /* Decompose a more than word wide shift (in INSN) of a multiword pseudo or a multiword zero-extend of a wordmode pseudo into a move - and 'set to zero' insn. Return a pointer to the new insn when a - replacement was done. */ + and 'set to zero' insn. SPEED_P says whether we are optimizing + for speed or size, when checking if a ZERO_EXTEND is preferable. + Return a pointer to the new insn when a replacement was done. */ static rtx_insn * -resolve_shift_zext (rtx_insn *insn) +resolve_shift_zext (rtx_insn *insn, bool speed_p) { rtx set; rtx op; @@ -1378,14 +1380,29 @@ resolve_shift_zext (rtx_insn *insn) dest_reg, GET_CODE (op) != ASHIFTRT); } - if (dest_reg != src_reg) - emit_move_insn (dest_reg, src_reg); - if (GET_CODE (op) != ASHIFTRT) - emit_move_insn (dest_upper, CONST0_RTX (word_mode)); - else if (INTVAL (XEXP (op, 1)) == 2 * BITS_PER_WORD - 1) - emit_move_insn (dest_upper, copy_rtx (src_reg)); + /* Consider using ZERO_EXTEND instead of setting DEST_UPPER to zero + if this is considered reasonable. */ + if (GET_CODE (op) == LSHIFTRT + && GET_MODE (op) == twice_word_mode + && REG_P (SET_DEST (set)) + && !choices[speed_p].splitting_zext) + { + rtx tmp = force_reg (word_mode, copy_rtx (src_reg)); + tmp = simplify_gen_unary (ZERO_EXTEND, twice_word_mode, tmp, word_mode); + emit_move_insn (SET_DEST (set), tmp); + } else - emit_move_insn (dest_upper, upper_src); + { + if (dest_reg != src_reg) + emit_move_insn (dest_reg, src_reg); + if (GET_CODE (op) != ASHIFTRT) + emit_move_insn (dest_upper, CONST0_RTX (word_mode)); + else if (INTVAL (XEXP (op, 1)) == 2 * BITS_PER_WORD - 1) + emit_move_insn (dest_upper, copy_rtx (src_reg)); + else + emit_move_insn (dest_upper, upper_src); + } + insns = get_insns (); end_sequence (); @@ -1670,7 +1687,7 @@ decompose_multiword_subregs (bool decompose_copies) { rtx_insn *decomposed_shift; - decomposed_shift = resolve_shift_zext (insn); + decomposed_shift = resolve_shift_zext (insn, speed_p); if (decomposed_shift != NULL_RTX) { insn = decomposed_shift; diff --git a/gcc/testsuite/gcc.target/avr/mmcu/pr109476.c b/gcc/testsuite/gcc.target/avr/mmcu/pr109476.c new file mode 100644 index 0000000..6e2269a --- /dev/null +++ b/gcc/testsuite/gcc.target/avr/mmcu/pr109476.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-Os -mmcu=avrxmega3" } */ + +unsigned short foo(unsigned char a, unsigned short b) { + return (unsigned char)((b >> 8) + 0) * a ; +} + +/* { dg-final { scan-assembler-times "mul" 1 } } */ +/* { dg-final { scan-assembler-times "mov" 1 } } */ +/* { dg-final { scan-assembler-not "add" } } */ +/* { dg-final { scan-assembler-not "ldi" } } */