[Bug middle-end/111657] New: Memory copy with structure assignment from named address space is not working

[Bug middle-end/111657] New: Memory copy with structure assignment from named address space is not working

[ubizjak at gmail dot com]

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

            Bug ID: 111657
           Summary: Memory copy with structure assignment from named
                    address space is not working
           Product: gcc
           Version: 12.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ubizjak at gmail dot com
  Target Milestone: ---

Taken from [1]. Compile the following testcase with -O2 -mno-sse:

--cut here--
struct a
{
  long arr[30];
};

__seg_gs struct a m;

void
foo (struct a *dst)
{
  *dst = m;
}
--cut here--

the produced assembly:

foo:
.LFB0:
        xorl    %eax, %eax
        cmpq    $240, %rax
        jnb     .L5
.L2:
        movzbl  %gs:m(%rax), %edx
        movb    %dl, (%rdi,%rax)
        addq    $1, %rax
        cmpq    $240, %rax
        jb      .L2
.L5:
        ret

As rightfully said in [1]:

"...and look at the end result. It's complete and utter sh*t:

<...>

to the point that I can only go "WTF"?

I mean, it's not just that it does the copy one byte at a time. It
literally compares %rax to $240 just after it has cleared it. I look
at that code, and I go "a five-year old with a crayon could have done
better".

[1]
https://lore.kernel.org/lkml/CAHk-=wh+cfn58XxMLnG6dH+Eb9-2dYfABXJF2FtSZ+vfqVvWzA@mail.gmail.com/

[Bug middle-end/111657] Memory copy with structure assignment from named address space is not working

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |79649

--- Comment #1 from Uroš Bizjak <ubizjak at gmail dot com> ---
Looks like another issue with IVopts (PR79649).


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79649
[Bug 79649] Memset pattern in named address space crashes compiler or generates
wrong code

[Bug middle-end/111657] Memory copy with structure assignment from named address space is not working

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
         Depends on|79649                       |
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-10-01
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is unrelated to PR 79649 as IV-OPTS does nothing here.

if we do `-mstringop-strategy=loop`, it works fine but
`-mstringop-strategy=libcall` is broken.

Seems like the middle-end expansion for the copy for the libcall case does
worse than the target specific loop.
Now the backend could select the target specific loop for this rather than the
libcall case ....


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79649
[Bug 79649] Memset pattern in named address space crashes compiler or generates
wrong code

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |target

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The easiest way to improve this is to have decide_alg select loop for the
have_as case if !TARGET_SSE && !TARGET_AVX...
Which means this is a target issue.

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ubizjak at gmail dot com
             Status|NEW                         |ASSIGNED

--- Comment #4 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 56030
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56030&action=edit
Propsed patch

Proposed patch declares libcall algorithm unavailable to non-default address
spaces and falls back to a loop if everything else fails. The following
testcase:

--cut here--
struct a { long arr[30]; };

__thread struct a t;
void foo (struct a *dst) { *dst = t; }

__seg_gs struct a s;
void bar (struct a *dst) { *dst = s; }
--cut here--

now compiles (-O2 -mno-sse) to:

foo:
        movq    %fs:0, %rdx
        movl    $30, %ecx
        leaq    t@tpoff(%rdx), %rsi
        rep movsq
        ret

bar:
        xorl    %eax, %eax
.L4:
        movl    %eax, %edx
        addl    $8, %eax
        movq    %gs:s(%rdx), %rcx
        movq    %rcx, (%rdi,%rdx)
        cmpl    $240, %eax
        jb      .L4
        ret

(rep movsq copies only from the default ds: address space)

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[ubizjak at gmail dot com]

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

--- Comment #5 from Uroš Bizjak <ubizjak at gmail dot com> ---
I have tried to compile with -mtune=nocona that has:

static stringop_algs nocona_memcpy[2] = {
  {libcall, {{12, loop_1_byte, false}, {-1, rep_prefix_4_byte, false}}},
  {libcall, {{32, loop, false}, {20000, rep_prefix_8_byte, false},
             {100000, unrolled_loop, false}, {-1, libcall, false}}}};

and compiler produces code as expected in both cases (use unrolled_loop when
rep movsq is unavailable):

foo:
        movq    %fs:0, %rdx
        leaq    t@tpoff(%rdx), %rsi
        movl    $30, %ecx
        rep movsq
        ret

bar:
        xorl    %edx, %edx
.L4:
        movl    %edx, %eax
        movq    %gs:s(%rax), %r9
        movq    %gs:s+8(%rax), %r8
        movq    %gs:s+16(%rax), %rsi
        movq    %gs:s+24(%rax), %rcx
        movq    %r9, (%rdi,%rax)
        movq    %r8, 8(%rdi,%rax)
        movq    %rsi, 16(%rdi,%rax)
        movq    %rcx, 24(%rdi,%rax)
        addl    $32, %edx
        cmpl    $224, %edx
        jb      .L4
        addq    %rdx, %rdi
        movq    %gs:s(%rdx), %rax
        movq    %rax, (%rdi)
        movq    %gs:s+8(%rdx), %rax
        movq    %rax, 8(%rdi)
        ret

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:

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

commit r14-4418-gc6bff80d786919f2f64e8a9f3179d6d36888bdb3
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Thu Oct 5 17:40:37 2023 +0200

    i386: Improve memory copy from named address space [PR111657]

    The stringop strategy selection algorithm falls back to a libcall strategy
    when it exhausts its pool of available strategies.  The memory area copy
    function (memcpy) is not availabe from the system library for non-default
    address spaces, so the compiler emits the most trivial byte-at-a-time
    copy loop instead.

    The compiler should instead emit an optimized copy loop as a fallback for
    non-default address spaces.

            PR target/111657

    gcc/ChangeLog:

            * config/i386/i386-expand.cc (alg_usable_p): Reject libcall
            strategy for non-default address spaces.
            (decide_alg): Use loop strategy as a fallback strategy for
            non-default address spaces.

    gcc/testsuite/ChangeLog:

            * gcc.target/i386/pr111657.c: New test.

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|---                         |14.0
         Resolution|---                         |FIXED

--- Comment #7 from Uroš Bizjak <ubizjak at gmail dot com> ---
Fixed.

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'd say it is a user error to invoke memcpy/memset etc. with pointers to
non-default address spaces, and for aggregate copies the middle-end should
ensure that the copying is not done using library calls; is that the case and
the problem was just that optab expansion was allowed for the structure copies
and the backend decided to use libcall in that case?

[Bug target/111657] Memory copy with structure assignment from named address space should be improved

[ubizjak at gmail dot com]

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

--- Comment #9 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jakub Jelinek from comment #8)
> I'd say it is a user error to invoke memcpy/memset etc. with pointers to
> non-default address spaces, and for aggregate copies the middle-end should
> ensure that the copying is not done using library calls; is that the case
> and the problem was just that optab expansion was allowed for the structure
> copies and the backend decided to use libcall in that case?

Yes, the stringop selection mechanism chose libcall strategy. However, the call
to memcpy is unavailable for non-default address space, so the middle-end
expanded the call into most trivial byte-copy loop. The patch just teaches
stringop selection to use optimized copy loop as a last resort with non-default
address spaces instead.