[Bug rtl-optimization/105546] New: load introduction when copying a struct

[Bug rtl-optimization/105546] New: load introduction when copying a struct

[absoler at smail dot nju.edu.cn]

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

            Bug ID: 105546
           Summary: load introduction when copying a struct
           Product: gcc
           Version: 11.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: absoler at smail dot nju.edu.cn
  Target Milestone: ---

Created attachment 52948
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52948&action=edit
case file

Hi, here's the code:

...

struct S0 {
   unsigned short  f0;
   unsigned short  f1;
   short  f2;
   unsigned char  f3;
   long long  f4;
   long long  f5;
};

short g_344 = 0xC307L;

struct S0  func_1(void)
{ 
    struct S0 l_346 = {65526UL,1UL,-8L,0xA1L,0x2F513C84A35AAE1BLL,-6L};


    if (g_344)
    { 
        struct S0 l_345 = {0x0AB2L,0x6D27L,-1L,0xABL,3L,0x3E72F31FF82C1E35LL};
        return l_345;
    }
    else
    { 
        return l_346;
    }
}

...

with option -O1, gcc-11.3.0 generate the following code:

Dump of assembler code for function func_1:
   0x0000000000401126 <+0>:     mov    %rdi,%rax
   0x0000000000401129 <+3>:     cmpw   $0x1,0x2eff(%rip)        # 0x404030
<g_344>
   0x0000000000401131 <+11>:    sbb    %edi,%edi
   0x0000000000401133 <+13>:    and    $0xf544,%di
   0x0000000000401138 <+18>:    add    $0xab2,%di
   0x000000000040113d <+23>:    cmpw   $0x1,0x2eeb(%rip)        # 0x404030
<g_344>
   0x0000000000401145 <+31>:    sbb    %esi,%esi
   0x0000000000401147 <+33>:    and    $0x92da,%si
   0x000000000040114c <+38>:    add    $0x6d27,%si
   0x0000000000401151 <+43>:    cmpw   $0x1,0x2ed7(%rip)        # 0x404030
<g_344>
   0x0000000000401159 <+51>:    sbb    %ecx,%ecx
   0x000000000040115b <+53>:    and    $0xfffffff9,%ecx
   0x000000000040115e <+56>:    sub    $0x1,%ecx
   0x0000000000401161 <+59>:    cmpw   $0x1,0x2ec7(%rip)        # 0x404030
<g_344>
   0x0000000000401169 <+67>:    sbb    %edx,%edx
   0x000000000040116b <+69>:    and    $0xfffffff6,%edx
   0x000000000040116e <+72>:    sub    $0x55,%edx
   0x0000000000401171 <+75>:    cmpw   $0x0,0x2eb7(%rip)        # 0x404030
<g_344>
   0x0000000000401179 <+83>:    movabs $0x2f513c84a35aae1b,%r9
   0x0000000000401183 <+93>:    mov    $0x3,%r8d
   0x0000000000401189 <+99>:    cmovne %r8,%r9
   0x000000000040118d <+103>:   movabs $0x3e72f31ff82c1e35,%r10
   0x0000000000401197 <+113>:   mov    $0xfffffffffffffffa,%r8
   0x000000000040119e <+120>:   cmovne %r10,%r8
   0x00000000004011a2 <+124>:   mov    %di,(%rax)
   0x00000000004011a5 <+127>:   mov    %si,0x2(%rax)
   0x00000000004011a9 <+131>:   mov    %cx,0x4(%rax)
   0x00000000004011ad <+135>:   mov    %dl,0x6(%rax)
   0x00000000004011b0 <+138>:   mov    %r9,0x8(%rax)
   0x00000000004011b4 <+142>:   mov    %r8,0x10(%rax)
   0x00000000004011b8 <+146>:   retq

it seems the last 4 loads are unneed and can be optimized away, which bring
vulnerabilities when facing concurrency and may decrease performance

[Bug rtl-optimization/105546] load introduction when initializing a struct

[absoler at smail dot nju.edu.cn]

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

--- Comment #1 from absoler at smail dot nju.edu.cn ---
and it's the load of g_344 that we care about.

[Bug target/105546] [11/12/13 Regression] ifconversion introduces many compares with loads

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Target|                            |x86_64-linux-gnu
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-05-10
   Target Milestone|---                         |11.4
            Summary|load introduction when      |[11/12/13 Regression]
                   |initializing a struct       |ifconversion introduces
                   |                            |many compares with loads
          Component|middle-end                  |target

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is ifconversion changing:
  if (g_344.0_1 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 3>; [50.00%]

  <bb 3> [local count: 536870912]:

  <bb 4> [local count: 1073741824]:
  # _16 = PHI <4499926296329723445(2), -6(3)>
  # _18 = PHI <3(2), 3409572933270154779(3)>
  # _20 = PHI <171(2), 161(3)>
  # _22 = PHI <-1(2), -8(3)>
  # _24 = PHI <27943(2), 1(3)>
  # _26 = PHI <2738(2), 65526(3)>

Into straight line code but keeps the load around.

THis is a target issue.

[Bug target/105546] [11/12/13 Regression] ifconversion introduces many compares with loads

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I believe it's also reported elsewhere that the x86 backend "likes"
re-materialized loads a lot.

[Bug target/105546] [11/12/13 Regression] ifconversion introduces many compares with loads

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
And it's sinking (of common stores) that turns

  <bb 2> [local count: 1073741824]:
  g_344.0_1 = g_344;
  if (g_344.0_1 != 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 4>; [50.00%]

  <bb 3> [local count: 536870913]:
  <retval>.f0 = 2738;
  <retval>.f1 = 27943;
  <retval>.f2 = -1;
  <retval>.f3 = 171;
  <retval>.f4 = 3;
  <retval>.f5 = 4499926296329723445;
  goto <bb 5>; [100.00%]

  <bb 4> [local count: 536870913]:
  <retval>.f0 = 65526;
  <retval>.f1 = 1;
  <retval>.f2 = -8;
  <retval>.f3 = 161;
  <retval>.f4 = 3409572933270154779;
  <retval>.f5 = -6;

  <bb 5> [local count: 1073741824]:
  return <retval>;

into

  <bb 2> [local count: 1073741824]:
  g_344.0_1 = g_344;
  if (g_344.0_1 != 0)
    goto <bb 4>; [50.00%]
  else
    goto <bb 3>; [50.00%]

  <bb 3> [local count: 536870913]:

  <bb 4> [local count: 1073741824]:
  # _16 = PHI <4499926296329723445(2), -6(3)>
  # _18 = PHI <3(2), 3409572933270154779(3)>
  # _20 = PHI <171(2), 161(3)>
  # _22 = PHI <-1(2), -8(3)>
  # _24 = PHI <27943(2), 1(3)>
  # _26 = PHI <2738(2), 65526(3)>
  <retval>.f0 = _26;
  <retval>.f1 = _24;
  <retval>.f2 = _22;
  <retval>.f3 = _20;
  <retval>.f4 = _18;
  <retval>.f5 = _16;
  return <retval>;

without that (-fno-tree-sink) we'd get

func_1:
.LFB0:
        .cfi_startproc
        movq    %rdi, %rax
        cmpw    $0, g_344(%rip)
        je      .L2
        movw    $2738, (%rdi)
        movw    $27943, 2(%rdi)
        movw    $-1, 4(%rdi)
        movb    $-85, 6(%rdi)
        movq    $3, 8(%rdi)
        movabsq $4499926296329723445, %rdx
        movq    %rdx, 16(%rdi)
        ret
.L2:
        movw    $-10, (%rdi)
        movw    $1, 2(%rdi)
        movw    $-8, 4(%rdi)
        movb    $-95, 6(%rdi)
        movabsq $3409572933270154779, %rcx
        movq    %rcx, 8(%rdi)
        movq    $-6, 16(%rdi)
        ret

or at -O2 now with vectorizing

func_1:
.LFB0:
        .cfi_startproc
        cmpw    $0, g_344(%rip)
        movq    %rdi, %rax
        je      .L2
        movdqa  .LC1(%rip), %xmm0
        movl    $-1, %ecx
        movl    $1831275186, (%rdi)
        movw    %cx, 4(%rdi)
        movb    $-85, 6(%rdi)
        movups  %xmm0, 8(%rdi)
        ret
        .p2align 4,,10
        .p2align 3
.L2:
        movdqa  .LC3(%rip), %xmm0
        movl    $-8, %edx
        movl    $131062, (%rdi)
        movw    %dx, 4(%rdi)
        movb    $-95, 6(%rdi)
        movups  %xmm0, 8(%rdi)
        ret

we could probably improve things by storing into the padding but GIMPLE
doesn't know it is allowed to do that.

sinking notes

          /* Insert a PHI to merge differing stored values if necessary.
             Note that in general inserting PHIs isn't a very good idea as
             it makes the job of coalescing and register allocation harder.
             Even common SSA uses on the rhs/lhs might extend their lifetime
             across multiple edges by this code motion which makes
             register allocation harder.  */

but we don't limit ourselves in the number of PHI nodes to create.
Of course since we have two sinking passes now we'd get inconsistent
results here, also since vectorization sits inbetween the two.

[Bug target/105546] [11/12/13 Regression] ifconversion introduces many compares with loads

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 53729
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53729&action=edit
patch to limit sinking

I was playing with this, limiting sinking to two PHI nodes in this case but
then with two passes we still get four stores sunk so it doesn't really help.

out-of-SSA could do the reverse transform for expensive constant PHI args.

[Bug target/105546] [11/12/13 Regression] ifconversion introduces many compares with loads

[roger at nextmovesoftware dot com]

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

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roger at nextmovesoftware dot com

--- Comment #6 from Roger Sayle <roger at nextmovesoftware dot com> ---
The duplication of comparisons during if-conversion was tackled by this patch:
https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609296.html
but unfortunately logic errors with that patch caused PR target/108292.

[Bug target/105546] [11/12/13/14 Regression] ifconversion introduces many compares with loads

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |11.5

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.4 is being released, retargeting bugs to GCC 11.5.