| 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 |
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 --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));
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 --