From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Content-Type: multipart/mixed; boundary="----=_NextPart_000_02C5_01D9841C.4A8FA960" 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 List-Id: This is a multipart message in MIME format. ------=_NextPart_000_02C5_01D9841C.4A8FA960 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 -- ------=_NextPart_000_02C5_01D9841C.4A8FA960 Content-Type: text/plain; name="patchch.txt" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="patchch.txt" diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md=0A= index 5a064f3..bfa5378 100644=0A= --- a/gcc/config/i386/i386.md=0A= +++ b/gcc/config/i386/i386.md=0A= @@ -2036,7 +2036,10 @@=0A= (define_insn "*pushdi2_rex64"=0A= [(set (match_operand:DI 0 "push_operand" "=3D<,<,!<")=0A= (match_operand:DI 1 "general_no_elim_operand" "re*m,*v,n"))]=0A= - "TARGET_64BIT"=0A= + "TARGET_64BIT=0A= + && (!reload_completed=0A= + || !SSE_REG_P (operands[1])=0A= + || !optimize_insn_for_size_p ())"=0A= "@=0A= push{q}\t%1=0A= #=0A= @@ -2079,7 +2082,10 @@=0A= (define_insn "*pushsi2_rex64"=0A= [(set (match_operand:SI 0 "push_operand" "=3DX,X")=0A= (match_operand:SI 1 "nonmemory_no_elim_operand" "re,*v"))]=0A= - "TARGET_64BIT"=0A= + "TARGET_64BIT=0A= + && (!reload_completed=0A= + || !SSE_REG_P (operands[1])=0A= + || !optimize_insn_for_size_p ())"=0A= "@=0A= push{q}\t%q1=0A= #"=0A= @@ -2089,7 +2095,10 @@=0A= (define_insn "*pushsi2"=0A= [(set (match_operand:SI 0 "push_operand" "=3D<,<")=0A= (match_operand:SI 1 "general_no_elim_operand" "ri*m,*v"))]=0A= - "!TARGET_64BIT"=0A= + "!TARGET_64BIT=0A= + && (!reload_completed=0A= + || !SSE_REG_P (operands[1])=0A= + || !optimize_insn_for_size_p ())"=0A= "@=0A= push{l}\t%1=0A= #"=0A= diff --git a/gcc/testsuite/gcc.target/i386/pr109766.c = b/gcc/testsuite/gcc.target/i386/pr109766.c=0A= new file mode 100644=0A= index 0000000..e29f615=0A= --- /dev/null=0A= +++ b/gcc/testsuite/gcc.target/i386/pr109766.c=0A= @@ -0,0 +1,21 @@=0A= +/* { dg-do compile { target { ! ia32 } } } */=0A= +/* { dg-options "-Os" } */=0A= +#define $expr(...) (__extension__({__VA_ARGS__;}))=0A= +#define $regF0 $expr(register double x __asm("xmm0"); x)=0A= +#define $regF1 $expr(register double x __asm("xmm1"); x)=0A= +#define $regF2 $expr(register double x __asm("xmm2"); x)=0A= +#define $regF3 $expr(register double x __asm("xmm3"); x)=0A= +#define $regF4 $expr(register double x __asm("xmm4"); x)=0A= +#define $regF5 $expr(register double x __asm("xmm5"); x)=0A= +#define $regF6 $expr(register double x __asm("xmm6"); x)=0A= +#define $regF7 $expr(register double x __asm("xmm7"); x)=0A= +=0A= +void func(char const*Fmt, ...);=0A= +void callfunc(char const*Fmt, double D0, double D1, double D2, double = D3,=0A= + double D4, double D5, double D6, double = D7){=0A= + func(Fmt,$regF0,$regF1,$regF2,$regF3,$regF4,$regF5,$regF6,$regF7,=0A= + D0,D1,D2,D3,D4,D5,D6,D7);=0A= +}=0A= +=0A= +/* { dg-final { scan-assembler-times "pushq" 9 } } */=0A= +/* { dg-final { scan-assembler-not "leaq" } } */=0A= ------=_NextPart_000_02C5_01D9841C.4A8FA960--