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: 86807 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp2351640vqo; Sun, 23 Apr 2023 13:15:05 -0700 (PDT) X-Google-Smtp-Source: AKy350amUI3Vdh8KLprtmvbw+gVJ1fUjo3a22FmcZx6iWSJni8v1XQaWpgIIz9KaDOmMdg1MmtWs X-Received: by 2002:aa7:c6d6:0:b0:504:894b:1bc9 with SMTP id b22-20020aa7c6d6000000b00504894b1bc9mr10342815eds.26.1682280904980; Sun, 23 Apr 2023 13:15:04 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682280904; cv=none; d=google.com; s=arc-20160816; b=KX5wVFYPCIoTzjzlblT55Pv1vb5tv0927d+mqtYzr5y2pCmDfhBVegtliJmLDm2Aa0 jmoajBRUFlaFJuX8O4D/YRsIWbZbopeyt3ACumlaLMl5BePIimzPO/HyK0gA1bni8KP/ pKUzw1mex2zdTcF521ZtbShYwmgDR1FYqNbTuUI0Wfi080BkGmQPWvLBgpTkZZaLfppj Kx0I1hUIVyerJGZidKKUySDUUUMe8sPWZPO0TalZIBCLf/hFtZg4p8tO4Zl4JxlwBtk+ KammF3GTgTpccXwdjsAMGOymVB1Rv9RMxAGoTf6t+GmS893b8QfarvNfRTaLy9XrQhTc 5K1w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-language:thread-index :mime-version:message-id:date:subject:cc:to:from:dkim-signature :dmarc-filter:delivered-to; bh=GEUVLV9ZXoxvoO+h//HAPK9tHKCQP/CHMWFZDp25pO8=; b=wXlkegoWGk2c2QJ/ZB9urbs7Dr60tdN7W1+O+xEyO/duYgnSpvsb16BH4ERZNv87JX XK35gNAIensyT8ZbB0mEahEyvhe3HqLfnk5IsW2QAJKsW30MxYCEY2cFEz7wCtKniBUT v9qFw1fHE3mkZi9SUu3q9RgOnNQk4FLbv1r5LrGNrEkNaQr9OyF8mZQZFE82wR1ki0N7 /GSdBPsRL/64CEuZnazjTJL3ZM7N3aUC1k7x2Fu5P3vf9y12FdxsKxhPO/RNGs26vgl0 YA8FDa472t78Kr2PmlWZNm95avfRbiYb8j4u9wz1mkgsXh1a6z+nhpUbQNGSRnpRIwwo IxEA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@nextmovesoftware.com header.s=default header.b=GvP51f5k; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id w15-20020a056402070f00b00504bad552b4si7676026edx.271.2023.04.23.13.15.04 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Apr 2023 13:15:04 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=fail header.i=@nextmovesoftware.com header.s=default header.b=GvP51f5k; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org" Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A95343857727 for ; Sun, 23 Apr 2023 20:14:55 +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+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763999382397912924?= X-GMAIL-MSGID: =?utf-8?q?1763999382397912924?= 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" } } */