[Bug target/96562] New: Rather poor assembly generated for copy-list-initialization in return statement.

[Bug target/96562] New: Rather poor assembly generated for copy-list-initialization in return statement.

[maxim.yegorushkin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

            Bug ID: 96562
           Summary: Rather poor assembly generated for
                    copy-list-initialization in return statement.
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: maxim.yegorushkin at gmail dot com
  Target Milestone: ---

Rather poor assembly generated for trivial code.

The following code:

    template<class P, class SizeT>
    struct Span {
        P begin_;
        SizeT size_;
    };

    Span<char*, unsigned> f(char* p, char* q) {
        return {p, static_cast<unsigned>(q - p)};
    }

When compiled with gcc-6.1 to gcc-10.2 with options "-O3 -march=skylake
-mtune=skylake" produces unexpectedly long and sub-optimal assembly code:

    f(unsigned char*, unsigned char*):
        mov     QWORD PTR [rsp-16], 0
        mov     QWORD PTR [rsp-24], rdi
        sub     rsi, rdi
        vmovdqa xmm1, XMMWORD PTR [rsp-24]
        vpinsrd xmm0, xmm1, esi, 2
        vmovdqa XMMWORD PTR [rsp-24], xmm0
        mov     rax, QWORD PTR [rsp-24]
        mov     rdx, QWORD PTR [rsp-16]
        ret

clang with the same options produces the expected assembly:

    f(unsigned char*, unsigned char*):
        mov     rdx, rsi
        mov     rax, rdi
        sub     edx, eax
        ret

Is there a way to make gcc produce the expected assembly, please?

https://gcc.godbolt.org/z/bacGW8

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[maxim.yegorushkin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #1 from Maxim Egorushkin <maxim.yegorushkin at gmail dot com> ---
Correction:

    Span<unsigned char*, unsigned> f(unsigned char* p, unsigned char* q) {
        return {p, static_cast<unsigned>(q - p)};
    }

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-08-11

--- Comment #2 from H.J. Lu <hjl.tools at gmail dot com> ---
Add -mavx to -O2 triggers this.

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #3 from Hongtao.liu <crazylht at gmail dot com> ---
a simple c testcase

typedef struct
{
  unsigned char* p;
  unsigned int a;
}st;

st foo (unsigned char* p, unsigned char* q)
{
  return {p, (unsigned int)(q-p)};
}


There's two issues here.
1. gcc use memory to move from xmm to gpr.
---
        vmovdqa XMMWORD PTR [rsp-24], xmm0
        mov     rax, QWORD PTR [rsp-24]
        mov     rdx, QWORD PTR [rsp-16]
---

2. gcc use vpinsrd to initialize st.a which is suboptimal after reload.

(insn 9 24 23 2 (set (reg:V4SI 20 xmm0 [89])
        (vec_merge:V4SI (vec_duplicate:V4SI (reg:SI 4 si [88]))
            (reg:V4SI 21 xmm1 [94])
            (const_int 4 [0x4]))) "../test.c":9:42 4387 {sse4_1_pinsrd}
     (nil))

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
in ix86_expand_pinsr with
src:(reg:DI 88)
dst:(subreg:DI (reg:TI 84 [ D.1940 ]) 8)
pos: 64
size: 32

it goes into
---
20360 
20361           case E_SImode:
20362             if (!TARGET_SSE4_1)
20363               return false;
20364             dstmode = V4SImode;
20365             pinsr = gen_sse4_1_pinsrd;
20366             break;
---

Seems optimal in pass_expand, but need extra instructions for movment between
gpr and xmm in pass_reload.

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[maxim.yegorushkin at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #5 from Maxim Egorushkin <maxim.yegorushkin at gmail dot com> ---
(In reply to H.J. Lu from comment #2)
> Add -mavx to -O2 triggers this.

The bug seems to be caused by -msse4.1, -mno-sse4.1 fixes it.

Changing size from `unsigned` to `unsigned long` makes the bug disappear.

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
I'm testing this patch

diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
index e194214804b..29809d69782 100644
--- a/gcc/config/i386/i386-expand.c
+++ b/gcc/config/i386/i386-expand.c
@@ -20333,7 +20333,6 @@ ix86_expand_pinsr (rtx *operands)
     case E_V4SImode:
     case E_V2DImode:
     case E_V1TImode:
-    case E_TImode:
       {
        machine_mode srcmode, dstmode;
        rtx (*pinsr)(rtx, rtx, rtx, rtx);

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:7d5de349d21479d7ec61dd0153e6f0958ad7384f

commit r11-2733-g7d5de349d21479d7ec61dd0153e6f0958ad7384f
Author: liuhongt <hongtao.liu@intel.com>
Date:   Wed Aug 12 10:48:17 2020 +0800

    Don't use pinsr/pextr for struct initialization/extraction.

    gcc/
            PR target/96562
            PR target/93897
            * config/i386/i386-expand.c (ix86_expand_pinsr): Don't use
            pinsr for TImode.
            (ix86_expand_pextr): Don't use pextr for TImode.

    gcc/testsuite/
            * gcc.target/i386/pr96562-1.c: New test.

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by hongtao Liu
<liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:a49452d964e3bbd1d9aa0d809355f41347b3ec05

commit r10-8636-ga49452d964e3bbd1d9aa0d809355f41347b3ec05
Author: liuhongt <hongtao.liu@intel.com>
Date:   Wed Aug 12 10:48:17 2020 +0800

    Don't use pinsr/pextr for struct initialization/extraction.

    gcc/
            PR target/96562
            PR target/93897
            * config/i386/i386-expand.c (ix86_expand_pinsr): Don't use
            pinsr for TImode.
            (ix86_expand_pextr): Don't use pextr for TImode.

    gcc/testsuite/
            * gcc.target/i386/pr96562-1.c: New test.

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[crazylht at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

--- Comment #9 from Hongtao.liu <crazylht at gmail dot com> ---
Fixed in GCC11, backport to GCC10.

[Bug target/96562] Rather poor assembly generated for copy-list-initialization in return statement.

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96562

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
      Known to work|                            |10.2.1

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.