[Bug target/99960] New: MVE: Wrong code storing V2DI vector

[Bug target/99960] New: MVE: Wrong code storing V2DI vector

[acoplan at gcc dot gnu.org]

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

            Bug ID: 99960
           Summary: MVE: Wrong code storing V2DI vector
           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: ---

For the following testcase:

typedef long long v2di __attribute((vector_size(16)));
void foo(v2di *p)
{
  *p = (v2di){ 1, 2 };
}

we miscompile it at -O2 -march=armv8.1-m.main+mve -mfloat-abi=hard. Looking at
the generated code:

foo:
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, uses_anonymous_args = 0
        @ link register save eliminated.
        vldr.64 d6, .L3
        vldr.64 d7, .L3+8
        vstr.64 d6, [r0]
        bx      lr
.L4:
        .align  3
.L3:
        .word   1
        .word   0
        .word   2
        .word   0
        .size   foo, .-foo

it looks like we're missing a store of d7. Here's a full testcase:

typedef long long v2di __attribute((vector_size(16)));

__attribute((noipa))
void stvec (v2di *p) {
  *p = (v2di){ 1, 2 };
}

int main () {
  v2di v;
  stvec(&v);
  if (v[1] != 2)
    __builtin_abort ();
}

I originally noticed this while investigating an execution failure for
gcc.c-torture/execute/pr92618.c. FWIW, clang does:

foo:
        adr     r1, .LCPI0_0
        vldrw.u32       q0, [r1]
        vstrw.32        q0, [r0]
        bx      lr
.LCPI0_0:
        .long   1                               @ 0x1
        .long   0                               @ 0x0
        .long   2                               @ 0x2
        .long   0                               @ 0x0

[Bug target/99960] MVE: Wrong code storing V2DI vector

[acoplan at gcc dot gnu.org]

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

--- Comment #1 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Looks like both loads and stores are wrong in V2DImode:

typedef long long __attribute((vector_size(16))) v2di;
v2di load(v2di *p) { return *p; }
void store(v2di *p, v2di v) { *p = v; }

gives:

load:
        vldr.64 d0, [r0]
        bx      lr
store:
        vstr.64 d0, [r0]
        bx      lr

clang does:

load:
        vldrw.u32       q0, [r0]
        bx      lr
store:
        vstrw.32        q0, [r0]
        bx      lr

It looks like the output code for *mve_mov<mode> needs tweaking.

[Bug target/99960] MVE: Wrong code storing V2DI vector

[acoplan at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-04-20
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |acoplan at gcc dot gnu.org

--- Comment #2 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Taking a look at this.

[Bug target/99960] MVE: Wrong code storing V2DI vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:7596c762137f26f495b53ec93471273887832e31

commit r12-654-g7596c762137f26f495b53ec93471273887832e31
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Mon May 10 09:46:45 2021 +0100

    arm: Fix wrong code with MVE V2DImode loads and stores [PR99960]

    As the PR shows, we currently miscompile V2DImode loads and stores for
    MVE.  We're currently using 64-bit loads/stores, but need to be using
    128-bit vector loads and stores. Fixed thusly.

    Some intrinsics tests were checking that we (incorrectly) used the
    64-bit loads/stores: these have been updated.

    gcc/ChangeLog:

            PR target/99960
            * config/arm/mve.md (*mve_mov<mode>): Simplify output code. Use
            vldrw.u32 and vstrw.32 for V2D[IF]mode loads and stores.

    gcc/testsuite/ChangeLog:

            PR target/99960
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_s64.c:
            Update now that we're (correctly) using full 128-bit vector
            loads/stores.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_u64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_z_s64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_z_u64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vuninitializedq_int.c: Likewise.
            * gcc.target/arm/mve/intrinsics/vuninitializedq_int1.c:
            Likewise.

[Bug target/99960] MVE: Wrong code storing V2DI vector

[acoplan at gcc dot gnu.org]

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

--- Comment #4 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed on trunk. Needs backporting to 11 and 10.

[Bug target/99960] MVE: Wrong code storing V2DI vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:32d4fa7d3792566b378ba0de003d50c0301de3a0

commit r11-8460-g32d4fa7d3792566b378ba0de003d50c0301de3a0
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Mon May 10 09:46:45 2021 +0100

    arm: Fix wrong code with MVE V2DImode loads and stores [PR99960]

    As the PR shows, we currently miscompile V2DImode loads and stores for
    MVE.  We're currently using 64-bit loads/stores, but need to be using
    128-bit vector loads and stores. Fixed thusly.

    Some intrinsics tests were checking that we (incorrectly) used the
    64-bit loads/stores: these have been updated.

    gcc/ChangeLog:

            PR target/99960
            * config/arm/mve.md (*mve_mov<mode>): Simplify output code. Use
            vldrw.u32 and vstrw.32 for V2D[IF]mode loads and stores.

    gcc/testsuite/ChangeLog:

            PR target/99960
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_s64.c:
            Update now that we're (correctly) using full 128-bit vector
            loads/stores.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_u64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_z_s64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_z_u64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vuninitializedq_int.c: Likewise.
            * gcc.target/arm/mve/intrinsics/vuninitializedq_int1.c:
            Likewise.

    (cherry picked from commit 7596c762137f26f495b53ec93471273887832e31)

[Bug target/99960] MVE: Wrong code storing V2DI vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:

https://gcc.gnu.org/g:59eb00c08db6683f6a69e3b9fd2743f00e187951

commit r10-9867-g59eb00c08db6683f6a69e3b9fd2743f00e187951
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Mon May 10 09:46:45 2021 +0100

    arm: Fix wrong code with MVE V2DImode loads and stores [PR99960]

    As the PR shows, we currently miscompile V2DImode loads and stores for
    MVE.  We're currently using 64-bit loads/stores, but need to be using
    128-bit vector loads and stores. Fixed thusly.

    Some intrinsics tests were checking that we (incorrectly) used the
    64-bit loads/stores: these have been updated.

    gcc/ChangeLog:

            PR target/99960
            * config/arm/mve.md (*mve_mov<mode>): Simplify output code. Use
            vldrw.u32 and vstrw.32 for V2D[IF]mode loads and stores.

    gcc/testsuite/ChangeLog:

            PR target/99960
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_s64.c:
            Update now that we're (correctly) using full 128-bit vector
            loads/stores.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_u64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_z_s64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vldrdq_gather_base_wb_z_u64.c:
            Likewise.
            * gcc.target/arm/mve/intrinsics/vuninitializedq_int.c: Likewise.
            * gcc.target/arm/mve/intrinsics/vuninitializedq_int1.c:
            Likewise.

    (cherry picked from commit 7596c762137f26f495b53ec93471273887832e31)

[Bug target/99960] MVE: Wrong code storing V2DI vector

[acoplan at gcc dot gnu.org]

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

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

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

--- Comment #7 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed.

[Bug target/99960] MVE: Wrong code storing V2DI vector

[pinskia at gcc dot gnu.org]

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

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

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