From patchwork Thu May 11 14:21:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roger Sayle X-Patchwork-Id: 1780118 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=mLSy6fN5; 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 4QHDc70Tv9z214S for ; Fri, 12 May 2023 00:21:59 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 074453856DFC for ; Thu, 11 May 2023 14:21: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 97AEA3858C5F for ; Thu, 11 May 2023 14:21:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 97AEA3858C5F 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=R7QRZqKAfBVuahjfwVYlkp5FV/f0Wv80abTPXY/UCHw=; b=mLSy6fN526snZMdw6ktjiztz+G mI7N9qcAG9w2eRwV/2NV3c0EcFP8anZWoowTu4jjJl3H6q0Ohj0RWye7GlWH6BRZJFJkVWAwk7EPC ine+9HDuS0ejJJ349TWwDfD2R9VY7Zc35YdRguBWidp6VaTyr+mHMgoMXWmiLZgaiNn72O75Hq5+B nhyh2i7HtkpM2nFtxOHXYmPXmvogHeNh+HiBan8H1xIR+65/RnMFvoSzU5LGS7pYZqq/RVSYM0yvR NHje7unZtkJ17wAogfT/J0jtnG2NXyJvVzrYtff5yQ1x76PcSPHzNM0g5xOj/VZxEV/it+U8Ysh07 nxwBM3fw==; Received: from [185.62.158.67] (port=49576 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 1px7As-0000sZ-08; Thu, 11 May 2023 10:21:42 -0400 From: "Roger Sayle" To: "'GCC Patches'" Cc: "'Uros Bizjak'" Subject: [x86_64 PATCH] PR middle-end/109766: Prevent cprop_hardreg bloating code with -Os. Date: Thu, 11 May 2023 15:21:41 +0100 Message-ID: <02c401d98413$e8c91e80$ba5b5b80$@nextmovesoftware.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AdmEEy6RvSkY+SYMTvKh/t9mRyPLIA== 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=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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" PR 109766 is an interesting case of large code being generated on x86_64, caused by an interaction/conflict between register allocation and hardreg cprop, that's tricky to fix/resolve within the middle-end. The task/challenge is to push a DImode value in an SSE register on to the stack, when optimizing for size. GCC's register allocator makes the optimal choice to move the SSE register to a GPR, and then use push. So after reload we have: (insn 46 3 4 2 (set (reg:DF 1 dx [101]) (reg:DF 21 xmm1 [ D1 ])) "pr109766.c":15:74 151 {*movdf_internal} (nil)) (insn 28 27 29 2 (set (mem:DF (pre_dec:DI (reg/f:DI 7 sp)) [0 S8 A64]) (reg:DF 1 dx [101])) "pr109766.c":16:5 142 {*pushdf} (expr_list:REG_ARGS_SIZE (const_int 56 [0x38]) (nil))) which corresponds to the short 6 byte sequence: 66 48 0f 7e ca movq %xmm1,%rdx [5 bytes] 52 push %rdx [1 byte] The problem is that several passes later, after pro_and_epilogue has determined that the function doesn't need a stack frame, that the hard register cprop pass sees the above two instructions, including the initial register to register move, and decides to "simplify" it as: (insn 68 67 69 2 (set (mem:DI (pre_dec:DI (reg/f:DI 7 sp)) [0 S8 A64]) (reg:DI 21 xmm1 [101])) "pr109766.c":16:5 62 {*pushdi2_rex64} (expr_list:REG_ARGS_SIZE (const_int 56 [0x38]) (nil))) but as x86_64 doesn't directly support push from SSE registers, the above is split during split3 into: (insn 92 91 93 2 (set (reg/f:DI 7 sp) (plus:DI (reg/f:DI 7 sp) (const_int -8 [0xfffffffffffffff8]))) "pr109766.c":16:5 247 {*leadi} (expr_list:REG_ARGS_SIZE (const_int 56 [0x38]) (nil))) (insn 93 92 94 2 (set (mem:DI (reg/f:DI 7 sp) [0 S8 A64]) (reg:DI 21 xmm1 [101])) "pr109766.c":16:5 88 {*movdi_internal} (nil)) which corresponds to the bigger 10 byte sequence: 48 8d 64 24 f8 lea -0x8(%rsp),%rsp [5 bytes] 66 0f d6 0c 24 movq %xmm1,(%rsp) [5 bytes] Clearly the cprop_hardreg substitution is questionable with -Os, but how to prevent it is a challenge. One (labor intensive) approach might be to have regcprop.cc query the target's rtx_costs before performing this type of substitution, which only works if the backend is sufficiently parameterized. Unfortunately, i386 like many targets defines the rtx_cost of (set (dst) (src)) to be rtx_cost(dst) + rtx_cost(src), which misses the subtlety of pushing an SSE register to the stack. An alternate solution, which can be implemented entirely in the backend, is to prevent *pushdi2_rex64 being recognized (by cprop_hardreg) with an SSE hard register operand after reload when optimizing for size. This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and make -k check, both with and without --target_board=unix{-m32}, with no new failures. Ok for mainline? 2023-05-11 Roger Sayle gcc/ChangeLog PR middle-end/109766 * config/i386/i386.md (*pushdi_rex64): Disallow SSE registers after reload when optimizing for size. (*pushsi2_rex64): Likewise. (*pushsi2): Likewise. gcc/testsuite/ChangeLog PR middle-end/109766 * gcc.target/i386/pr109766.c: New test case. Thanks in advance, Roger diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 5a064f3..bfa5378 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -2036,7 +2036,10 @@ (define_insn "*pushdi2_rex64" [(set (match_operand:DI 0 "push_operand" "=<,<,!<") (match_operand:DI 1 "general_no_elim_operand" "re*m,*v,n"))] - "TARGET_64BIT" + "TARGET_64BIT + && (!reload_completed + || !SSE_REG_P (operands[1]) + || !optimize_insn_for_size_p ())" "@ push{q}\t%1 # @@ -2079,7 +2082,10 @@ (define_insn "*pushsi2_rex64" [(set (match_operand:SI 0 "push_operand" "=X,X") (match_operand:SI 1 "nonmemory_no_elim_operand" "re,*v"))] - "TARGET_64BIT" + "TARGET_64BIT + && (!reload_completed + || !SSE_REG_P (operands[1]) + || !optimize_insn_for_size_p ())" "@ push{q}\t%q1 #" @@ -2089,7 +2095,10 @@ (define_insn "*pushsi2" [(set (match_operand:SI 0 "push_operand" "=<,<") (match_operand:SI 1 "general_no_elim_operand" "ri*m,*v"))] - "!TARGET_64BIT" + "!TARGET_64BIT + && (!reload_completed + || !SSE_REG_P (operands[1]) + || !optimize_insn_for_size_p ())" "@ push{l}\t%1 #" diff --git a/gcc/testsuite/gcc.target/i386/pr109766.c b/gcc/testsuite/gcc.target/i386/pr109766.c new file mode 100644 index 0000000..e29f615 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr109766.c @@ -0,0 +1,21 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-options "-Os" } */ +#define $expr(...) (__extension__({__VA_ARGS__;})) +#define $regF0 $expr(register double x __asm("xmm0"); x) +#define $regF1 $expr(register double x __asm("xmm1"); x) +#define $regF2 $expr(register double x __asm("xmm2"); x) +#define $regF3 $expr(register double x __asm("xmm3"); x) +#define $regF4 $expr(register double x __asm("xmm4"); x) +#define $regF5 $expr(register double x __asm("xmm5"); x) +#define $regF6 $expr(register double x __asm("xmm6"); x) +#define $regF7 $expr(register double x __asm("xmm7"); x) + +void func(char const*Fmt, ...); +void callfunc(char const*Fmt, double D0, double D1, double D2, double D3, + double D4, double D5, double D6, double D7){ + func(Fmt,$regF0,$regF1,$regF2,$regF3,$regF4,$regF5,$regF6,$regF7, + D0,D1,D2,D3,D4,D5,D6,D7); +} + +/* { dg-final { scan-assembler-times "pushq" 9 } } */ +/* { dg-final { scan-assembler-not "leaq" } } */