[Bug target/104882] New: [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[Bug target/104882] New: [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[acoplan at gcc dot gnu.org]

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

            Bug ID: 104882
           Summary: [12 Regression] MVE: Wrong code at -O2 since
                    r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf
           Product: gcc
           Version: 12.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: ---

Created attachment 52608
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52608&action=edit
broken assembly output

The following code:

int i;
char src[1072];
char dst[72];
int main() {
  for (i = 0; i < 128; i++)
    src[i] = i;
  __builtin_memcpy(dst, src, 7);
  for (i = 0; i < 7; i++)
    if (dst[i] != i)
      __builtin_abort();
}

is miscompiled at -O2 since vectorization was enabled at -O2. With -O2
-ftree-vectorize, it is miscompiled earlier, starting with:

commit 046a3beb1673bf4a61c131373b6a5e84158e92bf
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Thu Jun 3 15:35:50 2021

    arm: Auto-vectorization for MVE: add pack/unpack patterns

It looks like we do some dubious packing of vector elements before storing to
src. If I change the last loop to print the elements of dst instead, I see:

0 8 4 12 1 9 5

it should of course print:

0 1 2 3 4 5 6.

The broken code is attached. The testcase above was reduced from
gcc/testsuite/gcc.c-torture/execute/memcpy-1.c.

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[acoplan at gcc dot gnu.org]

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

--- Comment #1 from Alex Coplan <acoplan at gcc dot gnu.org> ---
For completeness, the options -march=armv8.1-m.main+mve -mfloat-abi=hard -O2
-ftree-vectorize are required to reproduce.

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2022-03-16

--- Comment #2 from Christophe Lyon <clyon at gcc dot gnu.org> ---
My understanding is that MVE's vmovn instructions do not work like Neon's.

If q0 = { 0x33333333, 0x22222222, 0x11111111, 0 } ( 4x32 bits)
   q1 = { 0x77777777, 0x66666666, 0x55555555, 0x44444444 }

With Neon:
vmovn.i32 d4, q0 gives:
d4 = { 0x3333, 0x2222, 0x1111, 0 } (4x16 bits)
vmovn.i32 d5, q1 gives:
d5 = { 0x7777, 0x6666, 0x5555, 0x4444 }
thus q2 = { 0x7777, 0x6666, 0x5555, 0x4444, 0x3333, 0x2222, 0x1111, 0 }

But with MVE:
vmovnb.i32 q2, q0 gives:
q2 = { 0x????, 0x3333, 0x????, 0x2222, 0x????, 0x1111, 0x????, 0 } (8x16 bits,
only the bottom bits of each 32 bits element are updated)
vmovnt.i32 q2, q1 then gives:
q2 = { 0x7777, 0x3333, 0x6666, 0x2222, 0x5555, 0x1111, 0x4444, 0 } (only the
top bits are updated)

This means that the input should be shuffled before using MVE's vmovn[bt] to
have
q0 = { 0x66666666, 0x44444444, 0x22222222, 0 }
q1 = { 0x77777777, 0x55555555, 0x33333333, 0x11111111 }

since MVE's vmovn do not seem to naturally map to GCC's vec_pack_trunc

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[clyon at gcc dot gnu.org]

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

--- Comment #3 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Revert patch posted:
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592136.html

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:3ab5c8cd03d92bf4ec41e351820349d92fbc40c4

commit r12-7818-g3ab5c8cd03d92bf4ec41e351820349d92fbc40c4
Author: Christophe Lyon <christophe.lyon@arm.com>
Date:   Fri Mar 18 08:30:00 2022 +0000

    arm: Revert Auto-vectorization for MVE: add pack/unpack patterns PR
target/104882

    This reverts commit r12-1434-g046a3beb1673bf to fix PR target/104882.

    As discussed in the PR, it turns out that the MVE ISA has no natural
    mapping with GCC's vec_pack_trunc / vec_unpack standard patterns, unlike
    Neon or SVE for instance.

    This patch also adds the executable testcase provided in the PR.
    This test passes at -O3 because the generated code does not need
    to use the pack/unpack patterns, hence the use of -O2 which now
    triggers vectorization since a few months ago.

    2022-03-18  Christophe Lyon  <christohe.lyon@arm.com>

            PR target/104882
            Revert
            2021-06-11  Christophe Lyon  <christophe.lyon@linaro.org>

            gcc/
            * config/arm/mve.md (mve_vec_unpack<US>_lo_<mode>): Delete.
            (mve_vec_unpack<US>_hi_<mode>): Delete.
            (@mve_vec_pack_trunc_lo_<mode>): Delete.
            (mve_vmovntq_<supf><mode>): Remove '@' prefix.
            * config/arm/neon.md (vec_unpack<US>_hi_<mode>): Move back
            from vec-common.md.
            (vec_unpack<US>_lo_<mode>): Likewise.
            (vec_pack_trunc_<mode>): Rename from
            neon_quad_vec_pack_trunc_<mode>.
            * config/arm/vec-common.md (vec_unpack<US>_hi_<mode>): Delete.
            (vec_unpack<US>_lo_<mode>): Delete.
            (vec_pack_trunc_<mode>): Delete.

            PR target/104882
            gcc/testsuite/
            * gcc.target/arm/simd/mve-vclz.c: Update expected results.
            * gcc.target/arm/simd/mve-vshl.c: Likewise.
            * gcc.target/arm/simd/mve-vec-pack.c: Delete.
            * gcc.target/arm/simd/mve-vec-unpack.c: Delete.
            * gcc.target/arm/simd/pr104882.c: New test.

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[clyon at gcc dot gnu.org]

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

Christophe Lyon <clyon at gcc dot gnu.org> changed:

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

--- Comment #5 from Christophe Lyon <clyon at gcc dot gnu.org> ---
Fixed on trunk.

[Bug target/104882] [12 Regression] MVE: Wrong code at -O2 since r12-1434-g046a3beb1673bf4a61c131373b6a5e84158e92bf

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:220008eafaaed7433b1c18e394279391e885a138

commit r13-6455-g220008eafaaed7433b1c18e394279391e885a138
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Fri Mar 3 15:59:14 2023 -0300

    [PR104882] [arm] require mve hw for mve run test

    The pr104882.c test is an execution test, but arm_v8_1m_mve_ok only
    tests for compile-time support.  Add a requirement for mve hardware.


    for  gcc/testsuite/ChangeLog

            PR target/104882
            * gcc.target/arm/simd/pr104882.c: Require mve hardware.