[Bug target/101934] New: [11 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[Bug target/101934] New: [11 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[rsandifo at gcc dot gnu.org]

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

            Bug ID: 101934
           Summary: [11 Regression] aarch64 memset code creates unaligned
                    stores for -mstrict-align
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rsandifo at gcc dot gnu.org
  Target Milestone: ---

For:

struct s { char x[95]; };
void foo (struct s *);
void bar (void) { struct s s1 = {}; foo (&s1); }

compiled with -Os -mstrict-align, GCC produces:

bar:
        movi    v0.4s, 0
        stp     x29, x30, [sp, -112]!
        mov     x29, sp
        add     x0, sp, 16
        str     q0, [sp, 80]
        stp     q0, q0, [sp, 16]
        stp     q0, q0, [sp, 48]
        str     q0, [sp, 95]         // <-------- unaligned
        bl      foo
        ldp     x29, x30, [sp], 112
        ret

The *_RATIO macros are already set up to handle STRICT_ALIGNMENT
differently, so presumably we should just make aarch64_expand_setmem
punt in that case.

[Bug target/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.2.0
   Target Milestone|---                         |11.3
      Known to work|                            |10.3.0
             Target|                            |aarch64*-*-*

[Bug target/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-08-31
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug target/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---

(insn 10 9 0 (set (mem/c:V16QI (plus:DI (reg:DI 92)
                (const_int 79 [0x4f])) [0 s1+79 S16 A8])
        (reg:V16QI 93)) "/app/example.cpp":4:28 -1
     (nil))

[Bug middle-end/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Definitely caused by r11-4973.
It is this part which does the incorrect part:
      /* Do certain trailing copies as overlapping if it's going to be
         cheaper.  i.e. less instructions to do so.  For instance doing a 15
         byte copy it's more efficient to do two overlapping 8 byte copies than
         8 + 4 + 2 + 1.  */
      if (n > 0 && n < copy_limit / 2)
        {
          next_mode = smallest_mode_for_size (n, MODE_INT);
          int n_bits = GET_MODE_BITSIZE (next_mode).to_constant ();
          gcc_assert (n_bits <= mode_bits);
          dst = aarch64_move_pointer (dst, (n - n_bits) / BITS_PER_UNIT);
          n = n_bits;
        }


Maybe that should have a STRICT_ALIGNMENT check too.

[Bug middle-end/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Yes adding && !STRICT_ALIGNMENT to that if is enough.

So I will go ahead assign this to me and will post a patch in the next couple
of days.

[Bug middle-end/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 51384
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51384&action=edit
Patch which fixes the problem

Still need to add a testcase though.

[Bug middle-end/101934] [11/12 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

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

commit r12-3303-ga45786e9a31f995087d8cb42bc3a4fe06911e588
Author: Andrew Pinski <apinski@marvell.com>
Date:   Tue Aug 31 04:41:14 2021 +0000

    Fix target/101934: aarch64 memset code creates unaligned stores for
-mstrict-align

    The problem here is the aarch64_expand_setmem code did not check
    STRICT_ALIGNMENT if it is creating an overlapping store.
    This patch adds that check and the testcase works.

    gcc/ChangeLog:

            PR target/101934
            * config/aarch64/aarch64.c (aarch64_expand_setmem):
            Check STRICT_ALIGNMENT before creating an overlapping
            store.

    gcc/testsuite/ChangeLog:

            PR target/101934
            * gcc.target/aarch64/memset-strict-align-1.c: New test.

[Bug middle-end/101934] [11 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12 Regression] aarch64  |[11 Regression] aarch64
                   |memset code creates         |memset code creates
                   |unaligned stores for        |unaligned stores for
                   |-mstrict-align              |-mstrict-align
      Known to work|                            |12.0

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed on the trunk so far.  Will let sit on the trunk for at least a week
before backporting it.

[Bug middle-end/101934] [11 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Andrew Pinski
<pinskia@gcc.gnu.org>:

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

commit r11-9001-gf00530266f89b28e8286cdd2f587e046a27d2193
Author: Andrew Pinski <apinski@marvell.com>
Date:   Tue Aug 31 04:41:14 2021 +0000

    Fix target/101934: aarch64 memset code creates unaligned stores for
-mstrict-align

    The problem here is the aarch64_expand_setmem code did not check
    STRICT_ALIGNMENT if it is creating an overlapping store.
    This patch adds that check and the testcase works.

    gcc/ChangeLog:

            PR target/101934
            * config/aarch64/aarch64.c (aarch64_expand_setmem):
            Check STRICT_ALIGNMENT before creating an overlapping
            store.

    gcc/testsuite/ChangeLog:

            PR target/101934
            * gcc.target/aarch64/memset-strict-align-1.c: New test.

    (cherry picked from commit a45786e9a31f995087d8cb42bc3a4fe06911e588)

[Bug middle-end/101934] [11 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.

[Bug middle-end/101934] [11 Regression] aarch64 memset code creates unaligned stores for -mstrict-align

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |felix at breitweiser dot de

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 103100 has been marked as a duplicate of this bug. ***