[Bug target/112787] New: Codegen regression of large GCC vector extensions when enabling SVE

[Bug target/112787] New: Codegen regression of large GCC vector extensions when enabling SVE

[avieira at gcc dot gnu.org]

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

            Bug ID: 112787
           Summary: Codegen regression of large GCC vector extensions when
                    enabling SVE
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: avieira at gcc dot gnu.org
  Target Milestone: ---

When compiling:

typedef int __attribute__((__vector_size__ (64))) vec;

vec fn (vec a, vec b)
{
    return a + b;
}

with '-O2 -march=armv8-a' vs '-O2 -march=armv8-a+sve' the codegen defaults to
scalar rather than using Advanced SIMD vectors.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[avieira at gcc dot gnu.org]

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

avieira at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |avieira at gcc dot gnu.org
   Last reconfirmed|                            |2023-11-30
             Target|                            |aarch64

--- Comment #1 from avieira at gcc dot gnu.org ---
The problem is veclower tries to find the largest vector type it can use for a
particular element type, which when SVE is enabled without a specified vector
length will always be a VLA type.  This then fails the check of it having less
elements than the type being used to do the computation, given that a VLA
element count is never 'known_lt' a constant one.

I am currently testing a patch that makes sure the mode selected does not have
more elements than the type we are trying to compute, given that it wouldn't be
used anyway.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think you should make sure to not use VLA vectors in lowering since lowering
doesn't really support using those (much similar to BB SLP).

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andre Simoes Dias Vieira
<avieira@gcc.gnu.org>:

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

commit r14-6752-ga3ff76278efe006dc0b50249c8e5baf565bff56b
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Wed Dec 20 16:41:52 2023 +0000

    veclower: improve selection of vector mode when lowering [PR 112787]

    This patch addresses the issue reported in PR target/112787 by improving
the
    compute type selection.  We do this by not considering types with more
elements
    than the type we are lowering since we'd reject such types anyway.

    gcc/ChangeLog:

            PR target/112787
            * tree-vect-generic.cc (type_for_widest_vector_mode): Change
function to
            use original vector type and check widest vector mode has at most
the
            same number of elements.
            (get_compute_type): Pass original vector type rather than the
element
            type to type_for_widest_vector_mode and remove now obsolete check
for
            the number of elements.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/pr112787.c: New test.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |14.0

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

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Andre Simoes Dias Vieira
<avieira@gcc.gnu.org>:

https://gcc.gnu.org/g:35486d68fc5b67d1b85c56e3ba149ba71c576550

commit r13-8341-g35486d68fc5b67d1b85c56e3ba149ba71c576550
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Wed Dec 20 16:41:52 2023 +0000

    veclower: improve selection of vector mode when lowering [PR 112787]

    This patch addresses the issue reported in PR target/112787 by improving
the
    compute type selection.  We do this by not considering types with more
elements
    than the type we are lowering since we'd reject such types anyway.

    gcc/ChangeLog:

            PR target/112787
            * tree-vect-generic.cc (type_for_widest_vector_mode): Change
function to
            use original vector type and check widest vector mode has at most
the
            same number of elements.
            (get_compute_type): Pass original vector type rather than the
element
            type to type_for_widest_vector_mode and remove now obsolete check
for
            the number of elements.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/pr112787.c: New test.

    (cherry picked from commit a3ff76278efe006dc0b50249c8e5baf565bff56b)

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Andre Simoes Dias Vieira
<avieira@gcc.gnu.org>:

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

commit r12-10165-ge84c06f801fbf8376fe43719675a409f588f68bd
Author: Andre Vieira <andre.simoesdiasvieira@arm.com>
Date:   Wed Dec 20 16:41:52 2023 +0000

    veclower: improve selection of vector mode when lowering [PR 112787]

    This patch addresses the issue reported in PR target/112787 by improving
the
    compute type selection.  We do this by not considering types with more
elements
    than the type we are lowering since we'd reject such types anyway.

    gcc/ChangeLog:

            PR target/112787
            * tree-vect-generic.cc (type_for_widest_vector_mode): Change
function to
            use original vector type and check widest vector mode has at most
the
            same number of elements.
            (get_compute_type): Pass original vector type rather than the
element
            type to type_for_widest_vector_mode and remove now obsolete check
for
            the number of elements.

    gcc/testsuite/ChangeLog:

            * gcc.target/aarch64/pr112787.c: New test.

    (cherry picked from commit a3ff76278efe006dc0b50249c8e5baf565bff56b)

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
                 CC|                            |ebotcazou at gcc dot gnu.org
             Status|RESOLVED                    |REOPENED

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
This has introduced regressions on the 12 and 13 branches for x86-64/Linux:

FAIL: g++.dg/opt/pr91838.C  -std=c++14  scan-assembler
pxor\\\\s+%xmm0,\\\\s+%xmm0
FAIL: g++.dg/opt/pr91838.C  -std=c++17  scan-assembler
pxor\\\\s+%xmm0,\\\\s+%xmm0
FAIL: g++.dg/opt/pr91838.C  -std=c++20  scan-assembler
pxor\\\\s+%xmm0,\\\\s+%xmm0

https://gcc.gnu.org/pipermail/gcc-testresults/2024-March/809244.html

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=91838

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Eric Botcazou from comment #7)
> This has introduced regressions on the 12 and 13 branches for x86-64/Linux:
> 
> FAIL: g++.dg/opt/pr91838.C  -std=c++14  scan-assembler
> pxor\\\\s+%xmm0,\\\\s+%xmm0
> FAIL: g++.dg/opt/pr91838.C  -std=c++17  scan-assembler
> pxor\\\\s+%xmm0,\\\\s+%xmm0
> FAIL: g++.dg/opt/pr91838.C  -std=c++20  scan-assembler
> pxor\\\\s+%xmm0,\\\\s+%xmm0
> 
> https://gcc.gnu.org/pipermail/gcc-testresults/2024-March/809244.html

The IR looks like:
  _4 = BIT_FIELD_REF <x_1(D), 16, 0>;
  _5 = _4 >> 8;

Well that is "undefined" in the IR.
That requires r14-2821-gd1c072a1c3411a6fe29900750b38210af8451eeb and
r14-2985-g04aa0edcace22a to be backported too.
I guess this testcase should just be xfailed on the GCC 13 and 12 branches
even.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #8)
> (In reply to Eric Botcazou from comment #7)
> > This has introduced regressions on the 12 and 13 branches for x86-64/Linux:
> > 
> > FAIL: g++.dg/opt/pr91838.C  -std=c++14  scan-assembler
> > pxor\\\\s+%xmm0,\\\\s+%xmm0
> > FAIL: g++.dg/opt/pr91838.C  -std=c++17  scan-assembler
> > pxor\\\\s+%xmm0,\\\\s+%xmm0
> > FAIL: g++.dg/opt/pr91838.C  -std=c++20  scan-assembler
> > pxor\\\\s+%xmm0,\\\\s+%xmm0
> > 
> > https://gcc.gnu.org/pipermail/gcc-testresults/2024-March/809244.html
> 
> The IR looks like:
>   _4 = BIT_FIELD_REF <x_1(D), 16, 0>;
>   _5 = _4 >> 8;
> 
> Well that is "undefined" in the IR.
> That requires r14-2821-gd1c072a1c3411a6fe29900750b38210af8451eeb and
> r14-2985-g04aa0edcace22a to be backported too.
> I guess this testcase should just be xfailed on the GCC 13 and 12 branches
> even.

Also note bug 91838 comment #17 specifically talks about this could happen
even.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[avieira at gcc dot gnu.org]

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

--- Comment #10 from avieira at gcc dot gnu.org ---
First of all, apologies for this! I don't know why I didn't test this on x86_64
too, I usually do for such backports.

Anyway I checked locally and backporting: 
r14-2821-gd1c072a1c3411a6fe29900750b38210af8451eeb seems to be enough for
gcc-12, I'm testing it on gcc-13 and running full regression tests on both
x86_64 and aarch64 and will get back to you.

@Andrew what made you think we also needed r14-2985-g04aa0edcace22a ? Not to
say we may not want to backport it, but just trying to figure out why it's
needed for this particular case.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[pinskia at gcc dot gnu.org]

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to avieira from comment #10)
> First of all, apologies for this! I don't know why I didn't test this on
> x86_64 too, I usually do for such backports.
> 
> Anyway I checked locally and backporting: 
> r14-2821-gd1c072a1c3411a6fe29900750b38210af8451eeb seems to be enough for
> gcc-12, I'm testing it on gcc-13 and running full regression tests on both
> x86_64 and aarch64 and will get back to you.
> 
> @Andrew what made you think we also needed r14-2985-g04aa0edcace22a ? Not to
> say we may not want to backport it, but just trying to figure out why it's
> needed for this particular case.

Because r14-2821 introduced PR 110838 (see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110838#c1) which was fixed by
r14-2985 .

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[avieira at gcc dot gnu.org]

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

--- Comment #12 from avieira at gcc dot gnu.org ---
Sorry, missed that comment, thanks! I'll test backporting both.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[avieira at gcc dot gnu.org]

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

--- Comment #13 from avieira at gcc dot gnu.org ---
They have both been backported, @Eric the tests should be passing again now.

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> They have both been backported, @Eric the tests should be passing again now.

Confirmed, thanks a lot!

[Bug target/112787] Codegen regression of large GCC vector extensions when enabling SVE

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|14.0                        |13.3