[Bug target/98119] New: SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[Bug target/98119] New: SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[acoplan at gcc dot gnu.org]

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

            Bug ID: 98119
           Summary: SVE: Wrong code with -O1 -ftree-vectorize
                    -msve-vector-bits=512 -mtune=thunderx
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

AArch64 GCC miscompiles the following testcase:

_Bool a[34];
int main() {
  for (long b = 0; b < 2; ++b)
    for (long c = 0; c < 17; ++c)
      a[b * 2 + c] = 1;
  for (long c = 0; c < 7; ++c)
    if (!a[2 + c])
      __builtin_abort();
}

at -O1 -ftree-vectorize -march=armv8.2-a+sve -msve-vector-bits=512
-mtune=thunderx.

Removing any one of these flags, the issue goes away. Obviously, this is not a
sensible choice of -mtune given that we're asking for SVE, but it seems that
the scheduling should not result in a miscompile.

Looking at a snippet of the broken code:

main:
.LFB0:
        .cfi_startproc
        adrp    x2, .LANCHOR0
        add     x2, x2, :lo12:.LANCHOR0
        and     w3, w2, 63
        and     x0, x2, -64    // align x2 down
        add     w1, w3, 17
        whilelo p0.d, wzr, w1
        whilelo p1.d, wzr, w3
        not     p0.b, p0/z, p1.b
        mov     z0.b, #1
        st1b    z0.d, p0, [x0] // no-op (p0 all 0s)
        mov     w3, 8
        whilelo p0.d, w3, w1
        b.none  .L2
        add     x4, x0, 8
        st1b    z0.d, p0, [x4] // stores out-of-bounds
        add     x0, x0, 16
        mov     w3, 16
        whilelo p0.d, w3, w1
        b.none  .L2
        st1b    z0.d, p0, [x0]

We initially compute the address of our array (a) in x2, and then align this
down to the nearest 64-byte-aligned address, storing the result in x0. We then
add 8 to this, and store a vector to this address. But this address can be
out-of-bounds (suppose a is only 16-byte aligned). So things have already
started to go downhill by this point.

[Bug target/98119] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rsandifo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-12-03
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Confirmed.  I think this is likely to be a bad interaction
between unpacked vectors and the optimisation to use predication
when peeling for alignment.  It's really an optimisation/cost model
bug that we used unpacked stores for this testcase at all, but given
that we do, the number of peeled iterations is being calculated
incorrectly.

[Bug target/98119] [10/11 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|SVE: Wrong code with -O1    |[10/11 Regression] SVE:
                   |-ftree-vectorize            |Wrong code with -O1
                   |-msve-vector-bits=512       |-ftree-vectorize
                   |-mtune=thunderx             |-msve-vector-bits=512
                   |                            |-mtune=thunderx

--- Comment #2 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Started with r10-4749-gcc68f7c2dabbf9d90ebe025a11f6d6ed5d49a113, FWIW.

[Bug target/98119] [10/11 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.3

[Bug target/98119] [10/11 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rguenth at gcc dot gnu.org]

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

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

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

[Bug target/98119] [10/11 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rsandifo at gcc dot gnu.org]

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

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

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

--- Comment #3 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Mine.

[Bug target/98119] [10/11 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:1393938e4c7dab9306cdce5a73d93b242fc246ec

commit r11-7927-g1393938e4c7dab9306cdce5a73d93b242fc246ec
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Wed Mar 31 11:26:06 2021 +0100

    aarch64: Fix target alignment for SVE [PR98119]

    The vectoriser supports peeling for alignment using predication:
    we move back to the previous aligned boundary and make the skipped
    elements inactive in the first loop iteration.  As it happens,
    the costs for existing CPUs give an equal cost to aligned and
    unaligned accesses, so this feature is rarely used.

    However, the PR shows that when the feature was forced on, we were
    still trying to align to a full-vector boundary even when using
    partial vectors.

    gcc/
            PR target/98119
            * config/aarch64/aarch64.c
            (aarch64_vectorize_preferred_vector_alignment): Query the size
            of the provided SVE vector; do not assume that all SVE vectors
            have the same size.

    gcc/testsuite/
            PR target/98119
            * gcc.target/aarch64/sve/pr98119.c: New test.

[Bug target/98119] [10 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rsandifo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11 Regression] SVE:     |[10 Regression] SVE: Wrong
                   |Wrong code with -O1         |code with -O1
                   |-ftree-vectorize            |-ftree-vectorize
                   |-msve-vector-bits=512       |-msve-vector-bits=512
                   |-mtune=thunderx             |-mtune=thunderx

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed on trunk so far.

[Bug target/98119] [10 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.3                        |10.4

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.3 is being released, retargeting bugs to GCC 10.4.

[Bug target/98119] [10 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Richard Sandiford
<rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:367aa5ee879c6bbfc4bf7ae94c680f0614581661

commit r10-9759-g367aa5ee879c6bbfc4bf7ae94c680f0614581661
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Fri Apr 23 17:17:11 2021 +0100

    aarch64: Fix target alignment for SVE [PR98119]

    The vectoriser supports peeling for alignment using predication:
    we move back to the previous aligned boundary and make the skipped
    elements inactive in the first loop iteration.  As it happens,
    the costs for existing CPUs give an equal cost to aligned and
    unaligned accesses, so this feature is rarely used.

    However, the PR shows that when the feature was forced on, we were
    still trying to align to a full-vector boundary even when using
    partial vectors.

    gcc/
            PR target/98119
            * config/aarch64/aarch64.c
            (aarch64_vectorize_preferred_vector_alignment): Query the size
            of the provided SVE vector; do not assume that all SVE vectors
            have the same size.

    gcc/testsuite/
            PR target/98119
            * gcc.target/aarch64/sve/pr98119.c: New test.

    (cherry picked from commit 1393938e4c7dab9306cdce5a73d93b242fc246ec)

[Bug target/98119] [10 Regression] SVE: Wrong code with -O1 -ftree-vectorize -msve-vector-bits=512 -mtune=thunderx

[rsandifo at gcc dot gnu.org]

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

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

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

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed.