From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x531.google.com (mail-ed1-x531.google.com [IPv6:2a00:1450:4864:20::531]) by sourceware.org (Postfix) with ESMTPS id E2AA13858C33 for ; Thu, 20 Jul 2023 07:49:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E2AA13858C33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x531.google.com with SMTP id 4fb4d7f45d1cf-5217ad95029so532858a12.2 for ; Thu, 20 Jul 2023 00:49:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689839368; x=1690444168; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=R/SSa+m+3svS3t8glcjb1C+ArdOS5IpjBaUO9GHQfnM=; b=JjEkmfwS6TFScWTORMery7YRRBSUkamdD4J7uk0ax4qKz1EV9zHDGLEkEO+vUbh/up tDmaxBzczazYjW6qxhNP3FgyzloYLWo4QC24iVYZAoQ7K878PHFyfjCbGKrOQpgN9wHD oJkyk+ZfU6NOhjbK+UYlIu4QwElKSd7k8cTc9omLKOQl288pmK7k1PlNO+YSOAeU3+XT 6eiJi9jSo6VXQZR+OXGWNc8EoE24WJWr4BhNUjb4g6RXaDLf5D+5wfGQDjILxho4vGl5 OkJV6nsBS1CyM5f2hy56b9ia4fwS6+jpMKsNdmBty2+T/TLm8K9DRVW2G67gOTQhlxuF I6Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689839368; x=1690444168; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=R/SSa+m+3svS3t8glcjb1C+ArdOS5IpjBaUO9GHQfnM=; b=TWcswKy+yoksdVLgDeNx4LMloh99LUVpyH8EJ02nQjV6LqJnmLPghybiHVbMGwSpqN 34MLaxaaXMowokNVN5FH8ADWbBEP6QAG2k8W+3hbhJhS75lz0dlQC32+pCqZ16XVKaY8 KezpVnwXwxPd3grkVk45SBTbwICDEeL3X6FakahLaK9FaYjQ39aDDbkJU5KrVTGusGbp UgtkZE24R9m/MjqMRf+RqzXuIx3QtX8hwgItBfElOyQx+allIpCrgXN8IExH9wyXo7Dz ei0+6dD52syHqnFJYDNSbYsgEg8popWQ5sdxuSfw6bu1+Bm7i0kMuWqZIjUxu/7CKqIf j9fw== X-Gm-Message-State: ABy/qLbVRyuTRb18KeDWaNXdkwqLqPj92y1AoftkB6QQJlFTDHLyDhJT ANmYpFEjyaCRF2ifGJso8LElvpkQqQDRzVSdoiKyqQDyTvM= X-Google-Smtp-Source: APBJJlG1lKzmQxQTwkWfw+Up5Yrn+2/qlHzHQdlRXFPke1MUJTqLUEPBOMkQHm+R0dyVVwZIpzC0j3Yn3shcpMdghKA= X-Received: by 2002:aa7:c616:0:b0:51e:2305:931 with SMTP id h22-20020aa7c616000000b0051e23050931mr4125897edq.22.1689839368320; Thu, 20 Jul 2023 00:49:28 -0700 (PDT) MIME-Version: 1.0 References: <009201d9ba7c$a64374d0$f2ca5e70$@nextmovesoftware.com> <002501d9badd$f5675930$e0360b90$@nextmovesoftware.com> In-Reply-To: <002501d9badd$f5675930$e0360b90$@nextmovesoftware.com> From: Uros Bizjak Date: Thu, 20 Jul 2023 09:49:17 +0200 Message-ID: Subject: Re: [x86_64 PATCH] More TImode parameter passing improvements. To: Roger Sayle Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,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: On Thu, Jul 20, 2023 at 9:44=E2=80=AFAM Roger Sayle wrote: > > > Hi Uros, > > > From: Uros Bizjak > > Sent: 20 July 2023 07:50 > > > > On Wed, Jul 19, 2023 at 10:07=E2=80=AFPM Roger Sayle > > wrote: > > > > > > This patch is the next piece of a solution to the x86_64 ABI issues i= n > > > PR 88873. This splits the *concat3_3 define_insn_and_spli= t > > > into two patterns, a TARGET_64BIT *concatditi3_3 and a !TARGET_64BIT > > > *concatsidi3_3. This allows us to add an additional alternative to > > > the the 64-bit version, enabling the register allocator to perform > > > this operation using SSE registers, which is implemented/split after > > > reload using vec_concatv2di. > > > > > > To demonstrate the improvement, the test case from PR88873: > > > > > > typedef struct { double x, y; } s_t; > > > > > > s_t foo (s_t a, s_t b, s_t c) > > > { > > > return (s_t){ __builtin_fma(a.x, b.x, c.x), __builtin_fma (a.y, b.y= , > > > c.y) }; } > > > > > > when compiled with -O2 -march=3Dcascadelake, currently generates: > > > > > > foo: vmovq %xmm2, -56(%rsp) > > > movq -56(%rsp), %rax > > > vmovq %xmm3, -48(%rsp) > > > vmovq %xmm4, -40(%rsp) > > > movq -48(%rsp), %rcx > > > vmovq %xmm5, -32(%rsp) > > > vmovq %rax, %xmm6 > > > movq -40(%rsp), %rax > > > movq -32(%rsp), %rsi > > > vpinsrq $1, %rcx, %xmm6, %xmm6 > > > vmovq %xmm0, -24(%rsp) > > > vmovq %rax, %xmm7 > > > vmovq %xmm1, -16(%rsp) > > > vmovapd %xmm6, %xmm2 > > > vpinsrq $1, %rsi, %xmm7, %xmm7 > > > vfmadd132pd -24(%rsp), %xmm7, %xmm2 > > > vmovapd %xmm2, -56(%rsp) > > > vmovsd -48(%rsp), %xmm1 > > > vmovsd -56(%rsp), %xmm0 > > > ret > > > > > > with this change, we avoid many of the reloads via memory, > > > > > > foo: vpunpcklqdq %xmm3, %xmm2, %xmm7 > > > vpunpcklqdq %xmm1, %xmm0, %xmm6 > > > vpunpcklqdq %xmm5, %xmm4, %xmm2 > > > vmovdqa %xmm7, -24(%rsp) > > > vmovdqa %xmm6, %xmm1 > > > movq -16(%rsp), %rax > > > vpinsrq $1, %rax, %xmm7, %xmm4 > > > vmovapd %xmm4, %xmm6 > > > vfmadd132pd %xmm1, %xmm2, %xmm6 > > > vmovapd %xmm6, -24(%rsp) > > > vmovsd -16(%rsp), %xmm1 > > > vmovsd -24(%rsp), %xmm0 > > > ret > > > > > > > > > This patch has been tested on x86_64-pc-linux-gnu with make bootstrap > > > and make -k check, both with and without --target_board=3Dunix{-m32} > > > with no new failures. Ok for mainline? > > > > > > > > > 2023-07-19 Roger Sayle > > > > > > gcc/ChangeLog > > > * config/i386/i386-expand.cc (ix86_expand_move): Don't call > > > force_reg, to use SUBREG rather than create a new pseudo when > > > inserting DFmode fields into TImode with insvti_{high,low}par= t. > > > (*concat3_3): Split into two define_insn_and_split= ... > > > (*concatditi3_3): 64-bit implementation. Provide alternative > > > that allows register allocation to use SSE registers that is > > > split into vec_concatv2di after reload. > > > (*concatsidi3_3): 32-bit implementation. > > > > > > gcc/testsuite/ChangeLog > > > * gcc.target/i386/pr88873.c: New test case. > > > > diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expa= nd.cc > > index f9b0dc6..9c3febe 100644 > > --- a/gcc/config/i386/i386-expand.cc > > +++ b/gcc/config/i386/i386-expand.cc > > @@ -558,7 +558,7 @@ ix86_expand_move (machine_mode mode, rtx > > operands[]) > > op0 =3D SUBREG_REG (op0); > > tmp =3D gen_rtx_AND (TImode, copy_rtx (op0), tmp); > > if (mode =3D=3D DFmode) > > - op1 =3D force_reg (DImode, gen_lowpart (DImode, op1)); > > + op1 =3D gen_lowpart (DImode, op1); > > > > Please note that gen_lowpart will ICE when op1 is a SUBREG. This is the= reason > > that we need to first force a SUBREG to a register and then perform gen= _lowpart, > > and it is necessary to avoid ICE. > > The good news is that we know op1 is a register, as this is tested by > "&& REG_P (op1)" on line 551. You'll also notice that I'm not removing > the force_reg from before the call to gen_lowpart, but removing the call > to force_reg after the call to gen_lowpart. When I originally wrote this= , > the hope was that placing this SUBREG in its own pseudo would help > with register allocation/CSE. Unfortunately, increasing the number of > pseudos (in this case) increases compile-time (due to quadratic behaviour > in LRA), as shown by PR rtl-optimization/110587, and keeping the DF->DI > conversion in a SUBREG inside the insvti_{high,low}part allows the > register allocator to see the DF->DI->TI sequence in a single pattern, > and hence choose to keep the TI mode in SSE registers, rather than use > a pair of reloads, to write the DF value to memory, then read it back as > a scalar in DImode, and perhaps the same again to go the other way. This was my only concern with the patch, with that cleared, the patch is OK= . Thanks, Uros.