diff mbox series

[x86_64] Improved insv of DImode/DFmode {high, low}parts into TImode.

Message ID 03e401d9b5a9$661e42e0$325ac8a0$@nextmovesoftware.com
State New
Headers show
Series [x86_64] Improved insv of DImode/DFmode {high, low}parts into TImode. | expand

Commit Message

Roger Sayle July 13, 2023, 4:45 p.m. UTC
This is the next piece towards a fix for (the x86_64 ABI issues affecting)
PR 88873.  This patch generalizes the recent tweak to ix86_expand_move
for setting the highpart of a TImode reg from a DImode source using
*insvti_highpart_1, to handle both DImode and DFmode sources, and also
use the recently added *insvti_lowpart_1 for setting the lowpart.

Although this is another intermediate step (not yet a fix), towards
enabling *insvti and *concat* patterns to be candidates for TImode STV
(by using V2DI/V2DF instructions), it already improves things a little.

For the test case from PR 88873

typedef struct { double x, y; } s_t;
typedef double v2df __attribute__ ((vector_size (2 * sizeof(double))));

s_t foo (s_t a, s_t b, s_t c)
{
  return (s_t) { fma(a.x, b.x, c.x), fma (a.y, b.y, c.y) };
}


With -O2 -march=cascadelake, GCC currently generates:

Before (29 instructions):
        vmovq   %xmm2, -56(%rsp)
        movq    -56(%rsp), %rdx
        vmovq   %xmm4, -40(%rsp)
        movq    $0, -48(%rsp)
        movq    %rdx, -56(%rsp)
        movq    -40(%rsp), %rdx
        vmovq   %xmm0, -24(%rsp)
        movq    %rdx, -40(%rsp)
        movq    -24(%rsp), %rsi
        movq    -56(%rsp), %rax
        movq    $0, -32(%rsp)
        vmovq   %xmm3, -48(%rsp)
        movq    -48(%rsp), %rcx
        vmovq   %xmm5, -32(%rsp)
        vmovq   %rax, %xmm6
        movq    -40(%rsp), %rax
        movq    $0, -16(%rsp)
        movq    %rsi, -24(%rsp)
        movq    -32(%rsp), %rsi
        vpinsrq $1, %rcx, %xmm6, %xmm6
        vmovq   %rax, %xmm7
        vmovq   %xmm1, -16(%rsp)
        vmovapd %xmm6, %xmm3
        vpinsrq $1, %rsi, %xmm7, %xmm7
        vfmadd132pd     -24(%rsp), %xmm7, %xmm3
        vmovapd %xmm3, -56(%rsp)
        vmovsd  -48(%rsp), %xmm1
        vmovsd  -56(%rsp), %xmm0
        ret

After (20 instructions):
        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

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.  No testcase yet, as the above code will hopefully
change dramatically with the next pieces.  Ok for mainline?


2023-07-13  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * config/i386/i386-expand.cc (ix86_expand_move): Generalize special
        case inserting of 64-bit values into a TImode register, to handle
        both DImode and DFmode using either *insvti_lowpart_1
        or *isnvti_highpart_1.


Thanks again,
Roger
--

Comments

Uros Bizjak July 14, 2023, 7:36 a.m. UTC | #1
On Thu, Jul 13, 2023 at 6:45 PM Roger Sayle <roger@nextmovesoftware.com> wrote:
>
>
> This is the next piece towards a fix for (the x86_64 ABI issues affecting)
> PR 88873.  This patch generalizes the recent tweak to ix86_expand_move
> for setting the highpart of a TImode reg from a DImode source using
> *insvti_highpart_1, to handle both DImode and DFmode sources, and also
> use the recently added *insvti_lowpart_1 for setting the lowpart.
>
> Although this is another intermediate step (not yet a fix), towards
> enabling *insvti and *concat* patterns to be candidates for TImode STV
> (by using V2DI/V2DF instructions), it already improves things a little.
>
> For the test case from PR 88873
>
> typedef struct { double x, y; } s_t;
> typedef double v2df __attribute__ ((vector_size (2 * sizeof(double))));
>
> s_t foo (s_t a, s_t b, s_t c)
> {
>   return (s_t) { fma(a.x, b.x, c.x), fma (a.y, b.y, c.y) };
> }
>
>
> With -O2 -march=cascadelake, GCC currently generates:
>
> Before (29 instructions):
>         vmovq   %xmm2, -56(%rsp)
>         movq    -56(%rsp), %rdx
>         vmovq   %xmm4, -40(%rsp)
>         movq    $0, -48(%rsp)
>         movq    %rdx, -56(%rsp)
>         movq    -40(%rsp), %rdx
>         vmovq   %xmm0, -24(%rsp)
>         movq    %rdx, -40(%rsp)
>         movq    -24(%rsp), %rsi
>         movq    -56(%rsp), %rax
>         movq    $0, -32(%rsp)
>         vmovq   %xmm3, -48(%rsp)
>         movq    -48(%rsp), %rcx
>         vmovq   %xmm5, -32(%rsp)
>         vmovq   %rax, %xmm6
>         movq    -40(%rsp), %rax
>         movq    $0, -16(%rsp)
>         movq    %rsi, -24(%rsp)
>         movq    -32(%rsp), %rsi
>         vpinsrq $1, %rcx, %xmm6, %xmm6
>         vmovq   %rax, %xmm7
>         vmovq   %xmm1, -16(%rsp)
>         vmovapd %xmm6, %xmm3
>         vpinsrq $1, %rsi, %xmm7, %xmm7
>         vfmadd132pd     -24(%rsp), %xmm7, %xmm3
>         vmovapd %xmm3, -56(%rsp)
>         vmovsd  -48(%rsp), %xmm1
>         vmovsd  -56(%rsp), %xmm0
>         ret
>
> After (20 instructions):
>         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
>
> 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.  No testcase yet, as the above code will hopefully
> change dramatically with the next pieces.  Ok for mainline?
>
>
> 2023-07-13  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * config/i386/i386-expand.cc (ix86_expand_move): Generalize special
>         case inserting of 64-bit values into a TImode register, to handle
>         both DImode and DFmode using either *insvti_lowpart_1
>         or *isnvti_highpart_1.

LGTM, but please watch out for fallout.

Thanks,
Uros.
diff mbox series

Patch

diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index 92ffa4b..fe87f8e 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -542,22 +542,39 @@  ix86_expand_move (machine_mode mode, rtx operands[])
 	}
     }
 
-  /* Use *insvti_highpart_1 to set highpart of TImode register.  */
+  /* Special case inserting 64-bit values into a TImode register.  */
   if (TARGET_64BIT
-      && mode == DImode
+      && (mode == DImode || mode == DFmode)
       && SUBREG_P (op0)
-      && SUBREG_BYTE (op0) == 8
       && GET_MODE (SUBREG_REG (op0)) == TImode
       && REG_P (SUBREG_REG (op0))
       && REG_P (op1))
     {
-      wide_int mask = wi::mask (64, false, 128);
-      rtx tmp = immed_wide_int_const (mask, TImode);
-      op0 = SUBREG_REG (op0);
-      tmp = gen_rtx_AND (TImode, copy_rtx (op0), tmp);
-      op1 = gen_rtx_ZERO_EXTEND (TImode, op1);
-      op1 = gen_rtx_ASHIFT (TImode, op1, GEN_INT (64));
-      op1 = gen_rtx_IOR (TImode, tmp, op1);
+      /* Use *insvti_lowpart_1 to set lowpart.  */
+      if (SUBREG_BYTE (op0) == 0)
+	{
+	  wide_int mask = wi::mask (64, true, 128);
+	  rtx tmp = immed_wide_int_const (mask, TImode);
+	  op0 = SUBREG_REG (op0);
+	  tmp = gen_rtx_AND (TImode, copy_rtx (op0), tmp);
+	  if (mode == DFmode)
+	    op1 = force_reg (DImode, gen_lowpart (DImode, op1));
+	  op1 = gen_rtx_ZERO_EXTEND (TImode, op1);
+	  op1 = gen_rtx_IOR (TImode, tmp, op1);
+	}
+      /* Use *insvti_highpart_1 to set highpart.  */
+      else if (SUBREG_BYTE (op0) == 8)
+	{
+	  wide_int mask = wi::mask (64, false, 128);
+	  rtx tmp = immed_wide_int_const (mask, TImode);
+	  op0 = SUBREG_REG (op0);
+	  tmp = gen_rtx_AND (TImode, copy_rtx (op0), tmp);
+	  if (mode == DFmode)
+	    op1 = force_reg (DImode, gen_lowpart (DImode, op1));
+	  op1 = gen_rtx_ZERO_EXTEND (TImode, op1);
+	  op1 = gen_rtx_ASHIFT (TImode, op1, GEN_INT (64));
+	  op1 = gen_rtx_IOR (TImode, tmp, op1);
+	}
     }
 
   emit_insn (gen_rtx_SET (op0, op1));