[Bug rtl-optimization/115576] New: [14 regression] Worse code generated for simple struct conversion

[Bug rtl-optimization/115576] New: [14 regression] Worse code generated for simple struct conversion

[jzwinck at gmail dot com]

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

            Bug ID: 115576
           Summary: [14 regression] Worse code generated for simple struct
                    conversion
           Product: gcc
           Version: 14.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jzwinck at gmail dot com
  Target Milestone: ---

Here is some simple code from a real application which converts one struct to a
similar but larger one:

    #include <cstdint>

    struct S64
    {
        uint64_t a;
        int8_t b;
        int8_t c;
        uint16_t d;
    };

    struct S32
    {
        uint32_t a;
        int8_t b;
        int8_t c;
        uint16_t d;

        S64 To64() const;
    };

    S64 S32::To64() const
    {
        return S64{a, b, c, d};
    }

GCC 13 and earlier emitted good code for this (as does Clang):

    S32::To64() const:
        mov     eax, DWORD PTR [rdi]
        mov     edx, DWORD PTR [rdi+4]
        ret

GCC 14 is much worse:

    S32::To64() const:
        xor     edx, edx
        mov     esi, DWORD PTR [rdi+4]
        mov     eax, DWORD PTR [rdi]
        movabs  rdi, -4294967296
        mov     rcx, rdx
        and     rcx, rdi
        or      rcx, rsi
        mov     rdx, rcx
        ret

Demo: https://godbolt.org/z/YbenMeEPq

[Bug c++/115576] [14 regression] Worse code generated for simple struct conversion

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |c++

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks like the padding is being zeroed.

Are you sure that is zeroing the padding here is what is expected of this.
Code?

[Bug c++/115576] [14/15 regression] Worse code generated for simple struct conversion

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-06-21
                 CC|                            |sayle at gcc dot gnu.org
      Known to work|                            |13.3.0
   Target Milestone|---                         |14.2
           Keywords|                            |missed-optimization,
                   |                            |needs-bisection
             Status|UNCONFIRMED                 |NEW
      Known to fail|                            |14.1.0, 15.0
             Target|X86_64                      |x86_64-*-*
           Priority|P3                          |P2
            Summary|[14 regression] Worse code  |[14/15 regression] Worse
                   |generated for simple struct |code generated for simple
                   |conversion                  |struct conversion

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  Probably the TImode enhancements.

[Bug target/115576] [14/15 regression] Worse code generated for simple struct conversion

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |target

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> Confirmed.  Probably the TImode enhancements.

Actually you might be right since the IR from the gimple level is the same
between GCC 13 and 14. And aarch64 code generation didn't change.

[Bug target/115576] [14/15 regression] Worse code generated for simple struct conversion

[user202729 at protonmail dot com]

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

user202729 <user202729 at protonmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |user202729 at protonmail dot com

--- Comment #4 from user202729 <user202729 at protonmail dot com> ---
Doing a binary search reveals that the patch that causes the issue is
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623766.html  .

Looking at the optimization process, the working is roughly the following:

Before the change, the expand pass generates code that uses (set (subreg ...)).
Then the split pass splits the register that is being used in subreg into two
registers, then the combine pass do "constant propagation" and simplifies the
code.

After the change, the expand pass generates code that uses (set reg (ior (and
reg 0xffff0000) value)). Thus the split pass cannot split the register (it does
not know how to deal with "and", and the combine pass is similarly stuck.

[Bug target/115576] [14/15 regression] Worse code generated for simple struct conversion since r14-2386-gbdf2737cda53a8

[user202729 at protonmail dot com]

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

--- Comment #5 from user202729 <user202729 at protonmail dot com> ---
I think this can be resolved by implementing some code in combine.cc to
replace:

(set:TI (reg:TI 101) (zero_extend:TI (...:DI ...)))
(set:DI (reg:DI ...) (subreg:DI (reg:TI 101) 8))

with

(set:TI (reg:TI 101) (zero_extend:TI (...:DI ...)))
(set:DI (reg:DI ...) (const_int 0))

This certainly is always an improvement, because 0 is simpler than extracting
the subregister.

After that, a few other passes of combine rescanning should be able to constant
fold the 0 forward.

Unfortunately, I don't know how to modify combine.cc or some other files to
handle this pattern. Can anyone give a suggestion? (maybe add a
define_peephole2 in common.md ? )

[Bug target/115576] [14/15 regression] Worse code generated for simple struct conversion since r14-2386-gbdf2737cda53a8

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Testcase in C:
struct S
{
  unsigned long long a;
  signed char b, c;
  unsigned short d;
};

struct T
{
  unsigned int a;
  signed char b, c;
  unsigned short d;
};

struct S
foo (const struct T *x)
{
  return (struct S) { x->a, x->b, x->c, x->d };
}

I'm afraid combine can't do much, as it generally tries to combine a pseudo
setter with its single user, that isn't something that is done here, most of
the TImode setters have 2+ users.
E.g. that
(insn 23 22 8 2 (set (reg:TI 101 [ D.2787 ])
        (const_int 0 [0])) "pr115576.c":18:10 -1
     (nil))
(insn 8 23 9 2 (set (reg:TI 101 [ D.2787 ])
        (ior:TI (and:TI (reg:TI 101 [ D.2787 ])
                (const_wide_int 0xffffffffffffffff0000000000000000))
            (zero_extend:TI (reg:DI 104 [ _2 ])))) "pr115576.c":18:10 140
{*insvti_lowpart_1}
     (expr_list:REG_DEAD (reg:DI 104 [ _2 ])
        (nil)))
is used in both
(insn 10 9 11 2 (set (reg:DI 106)
        (subreg:DI (reg:TI 101 [ D.2787 ]) 8)) "pr115576.c":18:10 88
{*movdi_internal}
     (nil))
and
(insn 14 13 19 2 (set (reg:TI 101 [ D.2787 ])
        (ior:TI (and:TI (reg:TI 101 [ D.2787 ])
                (const_wide_int 0x0ffffffffffffffff))
            (ashift:TI (zero_extend:TI (reg:DI 109))
                (const_int 64 [0x40])))) "pr115576.c":18:10 137
{*insvti_highpart_1}
     (expr_list:REG_DEAD (reg:DI 109)
        (nil)))

What would generally help is if nonzero_bits was able to deal with this and say
that all bits in (subreg:DI (reg:TI 101 [ D.2787 ]) 8) are zero.
But, nonzero_bits etc. uses unfortunately unsigned HOST_WIDE_INT rather than
say wide_int or offset_int or FIXED_WIDE_INT (128) or some other type that can
store more than 64 bits.
Though, say on 32-bit targets unsigned HOST_WIDE_INT is probably good enough.

[Bug target/115576] [14/15 regression] Worse code generated for simple struct conversion since r14-2386-gbdf2737cda53a8

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|14.2                        |14.3

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