[Bug tree-optimization/113964] New: repeat copy of struct

[Bug tree-optimization/113964] New: repeat copy of struct

[absoler at smail dot nju.edu.cn]

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

            Bug ID: 113964
           Summary: repeat copy of struct
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: absoler at smail dot nju.edu.cn
  Target Milestone: ---

here's the code:
```
struct S1 {
   uint32_t  f0;
   uint8_t  f1;
   uint64_t  f2;
   uint64_t  f3;
   int32_t  f4;
};

union U8 {
   struct S1  f0;
   int32_t  f1;
   int64_t  f2;
   uint8_t  f3;
   const int64_t  f4;
};

/* --- GLOBAL VARIABLES --- */
struct S1 g_16 = {4294967293UL,1UL,1UL,0xA9C1C73B017290B1LL,0x5ADF851FL};
union U8 g_37 = {{1UL,1UL,0x2361AE7D51263067LL,0xEEFD7F9B64A47447LL,0L}};
struct S1 g_50 =
{0x0CFC2012L,1UL,0x43E1243B3BE7B8BBLL,0x03C5CEC10C1A6FE1LL,1L};


/* --- FORWARD DECLARATIONS --- */

void func_32(union U8 e) {
  e.f3 = e.f0.f4;
  g_16 = e.f0 = g_50;
}
```

and compiled with gcc-13.2.0 -O2, generated binary code is:

```
0000000000401400 <func_32>:
func_32():
/root/myCSmith/test/output2.c:54
  401400:       movzbl 0x2c79(%rip),%eax        # 404080 <g_50>
  401407:       movdqa 0x2c71(%rip),%xmm0        # 404080 <g_50>
  40140f:       movdqa 0x2c79(%rip),%xmm1        # 404090 <g_50+0x10>
  401417:       movups %xmm0,0x8(%rsp)
  40141c:       mov    %al,0x8(%rsp)
  401420:       mov    0x2c72(%rip),%eax        # 404098 <g_50+0x18>
  401426:       movups %xmm1,0x18(%rsp)
  40142b:       movdqu 0x8(%rsp),%xmm2
  401431:       mov    %eax,0x20(%rsp)
  401435:       movdqu 0x18(%rsp),%xmm3
  40143b:       movaps %xmm2,0x2c7e(%rip)        # 4040c0 <g_16>
  401442:       movaps %xmm3,0x2c87(%rip)        # 4040d0 <g_16+0x10>
/root/myCSmith/test/output2.c:55
  401449:       retq   
  40144a:       nopw   0x0(%rax,%rax,1)
```

we can see that g_50.f0 and g_50.f4 have been repeatedly copy to e.

[Bug tree-optimization/113964] [11/12/13/14/15 Regression] repeat copy of struct

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|repeat copy of struct       |[11/12/13/14/15 Regression]
                   |                            |repeat copy of struct
      Known to fail|                            |4.6.4, 4.7.1, 4.9.0, 5.1.0
      Known to work|                            |4.1.2, 4.4.7, 4.5.3
   Last reconfirmed|                            |2024-02-17
   Target Milestone|---                         |11.5
           Keywords|                            |missed-optimization
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed. There is no DSE before esra .... And ESRA goes wrong because of
that.


Before ESRA we have:
```

  _1 = e.f0.f4;
  _2 = (unsigned char) _1;
  e.f3 = _2;
  e.f0 = g_50;
  g_16 = MEM[(const struct S1 &)&e];
```

And SRA thinks it should do:
```
Created a replacement for e offset: 0, size: 8: e$f3D.4642
Created a replacement for e offset: 192, size: 32: e$f0$f4D.4643
```

But the store to e.f3 is dead due to the store to e.f0.

I suspect this should be defered until GCC 15 and maybe we should try to reorg
the optimization pipeline there ...

[Bug tree-optimization/113964] [11/12/13/14/15 Regression] repeat copy of struct

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu.org
           Priority|P3                          |P2

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
No, I think the issue is that ESRA leaves e.f0 alone:

  e$f3_7 = e.f3;
  e$f0$f4_8 = e.f0.f4;
  _1 = e$f0$f4_8;
  _2 = (unsigned char) _1;
  e$f3_9 = _2;
  e.f0 = g_50;
  e$f3_10 = MEM <uint8_t> [(struct S1 *)&g_50];
  e$f0$f4_11 = MEM <int32_t> [(struct S1 *)&g_50 + 24B];
  MEM <uint8_t> [(union U8 *)&e] = e$f3_10;
  MEM <int32_t> [(union U8 *)&e + 24B] = e$f0$f4_11;
  g_16 = e.f0;

it looks like it materializes the e.f0 = g_15 copy but fails to elide that
(maybe assuming sth else will?)?  And then for some reason the final
g_16 = e.f90 copy isn't replaced?!

So somehow SRAs heuristics go off.

Martin?

[Bug tree-optimization/113964] [11/12/13/14/15 Regression] repeat copy of struct

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:8cd0d29270d4ed86c69b80c08de66dcb6c1e22fe

commit r14-9813-g8cd0d29270d4ed86c69b80c08de66dcb6c1e22fe
Author: Martin Jambor <mjambor@suse.cz>
Date:   Fri Apr 5 18:18:39 2024 +0200

    ipa: Force args obtined through pass-through maps to the expected type (PR
113964)

    Interactions of IPA-CP and IPA-SRA on the same data is a rather big
    source of issues, I'm afraid.  PR 113964 is a situation where IPA-CP
    propagates an unsigned short in a union parameter into a function
    which itself calls a different function which has a same union
    parameter and both these union parameters are split with IPA-SRA.  The
    leaf function however uses a signed short member of the union.

    In the calling function, we get the unsigned constant as the
    replacement for the union and it is then passed in the call without
    any type compatibility checks.  Apparently on riscv64 it matters
    whether the parameter is signed or unsigned short and so the leaf
    function can see different values.

    Fixed by using useless_type_conversion_p at the appropriate place and
    if it fails, use force_value_to type as elsewhere in similar
    situations.

    gcc/ChangeLog:

    2024-04-04  Martin Jambor  <mjambor@suse.cz>

            PR ipa/113964
            * ipa-param-manipulation.cc (ipa_param_adjustments::modify_call):
            Force values obtined through pass-through maps to the expected
            split type.

    gcc/testsuite/ChangeLog:

    2024-04-04  Patrick O'Neill  <patrick@rivosinc.com>
                Martin Jambor  <mjambor@suse.cz>

            PR ipa/113964
            * gcc.dg/ipa/pr114247.c: New test.

[Bug tree-optimization/113964] [11/12/13/14/15 Regression] repeat copy of struct

[jamborm at gcc dot gnu.org]

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

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Oops. I made a mistake, the commit above fixes PR 114247, sorry :-/
This one is the next in my queue.  Sorry again.

[Bug tree-optimization/113964] [11/12/13/14/15 Regression] repeat copy of struct

[jamborm at gcc dot gnu.org]

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

--- Comment #5 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> No, I think the issue is that ESRA leaves e.f0 alone:
> 
>   e$f3_7 = e.f3;
>   e$f0$f4_8 = e.f0.f4;
>   _1 = e$f0$f4_8;
>   _2 = (unsigned char) _1;
>   e$f3_9 = _2;
>   e.f0 = g_50;
>   e$f3_10 = MEM <uint8_t> [(struct S1 *)&g_50];
>   e$f0$f4_11 = MEM <int32_t> [(struct S1 *)&g_50 + 24B];
>   MEM <uint8_t> [(union U8 *)&e] = e$f3_10;
>   MEM <int32_t> [(union U8 *)&e + 24B] = e$f0$f4_11;
>   g_16 = e.f0;
> 
> it looks like it materializes the e.f0 = g_15 copy but fails to elide that
> (maybe assuming sth else will?)?  And then for some reason the final
> g_16 = e.f90 copy isn't replaced?!
> 
> So somehow SRAs heuristics go off.
> 
> Martin?

I am afraid this is just another example of what flow-insensitive SRA cannot
optimize well.  I'll keep it in the list of testcases to hopefully one day
improve on when we make it flow sensitive.

[Bug tree-optimization/113964] [11/12/13/14/15 Regression] repeat copy of struct

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Martin Jambor
<jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:5c3238b0d55ec13a2430aa606e2bfed9432e97ac

commit r13-8620-g5c3238b0d55ec13a2430aa606e2bfed9432e97ac
Author: Martin Jambor <mjambor@suse.cz>
Date:   Fri Apr 19 16:48:12 2024 +0200

    ipa: Force args obtined through pass-through maps to the expected type (PR
113964)

    Interactions of IPA-CP and IPA-SRA on the same data is a rather big
    source of issues, I'm afraid.  PR 113964 is a situation where IPA-CP
    propagates an unsigned short in a union parameter into a function
    which itself calls a different function which has a same union
    parameter and both these union parameters are split with IPA-SRA.  The
    leaf function however uses a signed short member of the union.

    In the calling function, we get the unsigned constant as the
    replacement for the union and it is then passed in the call without
    any type compatibility checks.  Apparently on riscv64 it matters
    whether the parameter is signed or unsigned short and so the leaf
    function can see different values.

    Fixed by using useless_type_conversion_p at the appropriate place and
    if it fails, use force_value_to type as elsewhere in similar
    situations.

    gcc/ChangeLog:

    2024-04-04  Martin Jambor  <mjambor@suse.cz>

            PR ipa/113964
            * ipa-param-manipulation.cc (ipa_param_adjustments::modify_call):
            Force values obtined through pass-through maps to the expected
            split type.

    gcc/testsuite/ChangeLog:

    2024-04-04  Patrick O'Neill  <patrick@rivosinc.com>
                Martin Jambor  <mjambor@suse.cz>

            PR ipa/113964
            * gcc.dg/ipa/pr114247.c: New test.

    (cherry picked from commit 8cd0d29270d4ed86c69b80c08de66dcb6c1e22fe)

[Bug tree-optimization/113964] [11/12/13/14/15/15 Regression] repeat copy of struct

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Martin Jambor
<jamborm@gcc.gnu.org>:

https://gcc.gnu.org/g:44191982c6bd41db1c9d126ea2f15febec3c1f81

commit r12-10442-g44191982c6bd41db1c9d126ea2f15febec3c1f81
Author: Martin Jambor <mjambor@suse.cz>
Date:   Tue May 14 14:13:36 2024 +0200

    ipa: Force args obtined through pass-through maps to the expected type (PR
114247)

    Interactions of IPA-CP and IPA-SRA on the same data is a rather big
    source of issues, I'm afraid.  PR 113964 is a situation where IPA-CP
    propagates an unsigned short in a union parameter into a function
    which itself calls a different function which has a same union
    parameter and both these union parameters are split with IPA-SRA.  The
    leaf function however uses a signed short member of the union.

    In the calling function, we get the unsigned constant as the
    replacement for the union and it is then passed in the call without
    any type compatibility checks.  Apparently on riscv64 it matters
    whether the parameter is signed or unsigned short and so the leaf
    function can see different values.

    Fixed by using useless_type_conversion_p at the appropriate place and
    if it fails, use force_value_to type as elsewhere in similar
    situations.

    gcc/ChangeLog:

    2024-04-04  Martin Jambor  <mjambor@suse.cz>

            PR ipa/114247
            * ipa-param-manipulation.cc (ipa_param_adjustments::modify_call):
            Force values obtined through pass-through maps to the expected
            split type.

    gcc/testsuite/ChangeLog:

    2024-04-04  Patrick O'Neill  <patrick@rivosinc.com>
                Martin Jambor  <mjambor@suse.cz>

            PR ipa/114247
            * gcc.dg/ipa/pr114247.c: New test.

    (cherry picked from commit 8cd0d29270d4ed86c69b80c08de66dcb6c1e22fe)