[Bug target/96377] New: [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[Bug target/96377] New: [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

            Bug ID: 96377
           Summary: [10/11 Regression] GCC 10.2/11 doesn't build Linux
                    kernel anymore
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
  Target Milestone: ---

The following testcase reduced from Linux kernel's crypto/aegis128-neon-inner.c
started to be rejected with r11-1741-g31427b974ed7b7dd54e28fec595e731bf6eea8ba
and r10-8501-g932e9140d3268cf2033c1c3e93219541c53fcd29

#include <arm_neon.h>

struct aegis128_state {
 uint8x16_t v[5];
};

void foo(const void *key, const void *iv, const void *const0, const void
*const1)
{
 uint8x16_t k = vld1q_u8(key);
 uint8x16_t kiv = k ^ vld1q_u8(iv);
 struct aegis128_state st = {{
  kiv,
  vld1q_u8(const1),
  vld1q_u8(const0),
  k ^ vld1q_u8(const0),
  k ^ vld1q_u8(const1),
 }};
}

The error is:
error: incompatible types when initializing type ‘unsigned char’ using type
‘uint8x16_t’ {aka ‘__Uint8x16_t’}
(twice)

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org
   Last reconfirmed|                            |2020-07-29
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
           Priority|P3                          |P1
   Target Milestone|---                         |10.3

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the problem is that c_common_type does:
742       if (TYPE_ATTRIBUTES (t1) != NULL_TREE)
743         t1 = build_type_attribute_variant (t1, NULL_TREE);
744     
745       if (TYPE_ATTRIBUTES (t2) != NULL_TREE)
746         t2 = build_type_attribute_variant (t2, NULL_TREE);
which strips off the "Advanced SIMD type" attribute from both of the types.
C++ FE doesn't do anything like that.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This dates back to
https://gcc.gnu.org/legacy-ml/gcc-patches/2004-06/msg00288.html
Dunno, do we want to never strip attributes from VECTOR_TYPEs and only strip
them that way from non-VECTOR_TYPEs?  Or only strip some attributes and keep
others?
Or just add a if (t1 == t2) return t1; before the attribute stripping (perhaps
for VECTOR_TYPEs only)?
Note, due to the typedef __Uint8x16_t uint8x16_t; on this testcase t1 and t2
are equal and are the typedef file, and because they are unqualified, we don't
use TYPE_MAIN_VARIANT (which is the __Uint8x16_t type).
What is the desirable behavior for the common type if one is the aarch64
specific vector and the other generic vector with the same mode?
The C++ behavior for vector types in cp_common_type is that it will merge
attributes from both types, and if one is TYPE_UNSIGNED, pick that one,
otherwise pick the other one.  So in that case the ARM specific vector type
wins.

I must say I'm surprised that the 10.2 version of the patch also matters,
perhaps the FE isn't using the target hook to compare attributes, but rather
just compares TYPE_MAIN_VARIANT or something similar to determine if the
initializer is initializing a whole vector or an element of the vector.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For those that need a quick workaround for the kernel, I think
  (uint8x16_t) (k ^ vld1q_u8(const0)),
  (uint8x16_t) (k ^ vld1q_u8(const1)),
instead of
  k ^ vld1q_u8(const0),
  k ^ vld1q_u8(const1),
will do the job.  And it is a question if portable code can use binary
operators on the ARM specific vector types rather than intrinsics or the normal
generic vectors instead.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

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

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

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Gah.  Thanks for the report and analysis.  Like you say,
I was hoping that adding the attributes in 10.2 but not
making them matter for type compatibility would have
been pretty conservative, but obviously it wasn't
conservative enough.

I'll take a look tomorrow.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
I think this is bound up with the question whether:

typedef int v4si __attribute__((vector_size(16)));
typedef short v8hi __attribute__((vector_size(16)));

struct s {
  v8hi x;
  v4si y;
};

void
foo (v4si i, v8hi h)
{
  struct s x1 = { i, i };
  struct s x2 = { h, h };
  struct s x3 = { i, h };
  struct s x4 = { h, i };
}

should be valid with -flax-vector-conversions.  g++ and clang
think it should be, but gcc doesn't accept it.

IMO process_init_element shouldn't recurse into vector types
if the initialisation value is also a vector type.  We should
treat the vector value as initialising the whole type and report
an error if they're not compatible.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org

--- Comment #6 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Joseph, Marek, any thoughts on comment #5?

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess that is reasonable thing to do, if the two vector types aren't really
compatible one will get an error.
But then, for trunk, won't the stripping of the attributes from vector types
still mean that comp_type_attributes will return false when comparing the
destination vector type (the uint8x16_t one) with the one from the binary
expression (the same with stripped attribute, i.e. essentially a generic
vector) and thus the initialization will be considered erroneous?

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

--- Comment #8 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> I guess that is reasonable thing to do, if the two vector types aren't
> really compatible one will get an error.
> But then, for trunk, won't the stripping of the attributes from vector types
> still mean that comp_type_attributes will return false when comparing the
> destination vector type (the uint8x16_t one) with the one from the binary
> expression (the same with stripped attribute, i.e. essentially a generic
> vector) and thus the initialization will be considered erroneous?

The initialization itself seems to work, since for vectors
compatibility depends on vector_types_compatible_p.

But the stripping of the attributes does still affect:

#include <arm_neon.h>

uint8x16_t
foo (int c, uint8x16_t x, uint8x16_t y)
{
  return c ? x + 1 : y;
}

which is wrongly rejected for C, but not C++.  So I guess for trunk
we need both fixes.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

--- Comment #9 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Created attachment 48964
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48964&action=edit
Patch for the initialisation problem

I'm testing this patch to fix the initialisation side of the problem.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[joseph at codesourcery dot com]

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

--- Comment #10 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
On Thu, 30 Jul 2020, rsandifo at gcc dot gnu.org wrote:

> IMO process_init_element shouldn't recurse into vector types
> if the initialisation value is also a vector type.  We should
> treat the vector value as initialising the whole type and report
> an error if they're not compatible.

Yes, that seems correct.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:7d599ad27b9bcf5165f87710f1abc64bbabd06ae

commit r11-2481-g7d599ad27b9bcf5165f87710f1abc64bbabd06ae
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Sat Aug 1 12:41:28 2020 +0100

    c: Fix bogus vector initialisation error [PR96377]

    One of the problems in this PR was that if we had:

      vector_type1 array[] = { vector_value1 };

    process_init_element would only treat vector_value1 as initialising
    a vector_type1 if they had the same TYPE_MAIN_VARIANT.  This has
    several problems:

    (1) It gives confusing error messages if the vector types are
        incompatible.  (Tested by gcc.dg/pr96377-1.c.)

    (2) It means that we reject code that should be valid with
        -flax-vector-conversions.  (Tested by gcc.dg/pr96377-2.c.)

    (3) On arm and aarch64 targets, it means that we reject some
        initializers that mix Advanced SIMD and standard GNU vectors.
        These vectors have traditionally had different TYPE_MAIN_VARIANTs
        because they have different mangling schemes.  (Tested by
        gcc.dg/pr96377-[3-6].c.)

    (4) It means that we reject SVE initializers that should be valid.
        (Tested by gcc.target/aarch64/sve/gnu_vectors_[34].c.)

    (5) After r11-1741-g:31427b974ed7b7dd54e2 we reject:

          arm_neon_type1 array[] = { k ^ arm_neon_value1 };

        because applying the binary operator to arm_neon_value1 strips
        the "Advanced SIMD type" attributes that were added in that patch.
        Stripping the attributes is problematic for other reasons though,
        so that still needs to be fixed separately.

    g++.target/aarch64/sve/gnu_vectors_[34].C already pass.

    gcc/c/
            PR c/96377
            * c-typeck.c (process_init_element): Split test for whether to
            recurse into a record, union or array into...
            (initialize_elementwise_p): ...this new function.  Don't recurse
            into a vector type if the initialization value is also a vector.

    gcc/testsuite/
            PR c/96377
            * gcc.dg/pr96377-1.c: New test.
            * gcc.dg/pr96377-2.c: Likewise.
            * gcc.dg/pr96377-3.c: Likewise.
            * gcc.dg/pr96377-4.c: Likewise.
            * gcc.dg/pr96377-5.c: Likewise.
            * gcc.dg/pr96377-6.c: Likewise.
            * gcc.target/aarch64/pr96377-1.c: Likewise.
            * gcc.target/aarch64/sve/acle/general-c/gnu_vectors_3.c: Likewise.
            * gcc.target/aarch64/sve/acle/general-c/gnu_vectors_4.c: Likewise.
            * g++.target/aarch64/sve/acle/general-c++/gnu_vectors_3.C:
Likewise.
            * g++.target/aarch64/sve/acle/general-c++/gnu_vectors_4.C:
Likewise.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 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:a216daaa30bc8949086a16e7656f2025b692d03c

commit r10-8562-ga216daaa30bc8949086a16e7656f2025b692d03c
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Mon Aug 3 09:48:36 2020 +0100

    c: Fix bogus vector initialisation error [PR96377]

    One of the problems in this PR was that if we had:

      vector_type1 array[] = { vector_value1 };

    process_init_element would only treat vector_value1 as initialising
    a vector_type1 if they had the same TYPE_MAIN_VARIANT.  This has
    several problems:

    (1) It gives confusing error messages if the vector types are
        incompatible.  (Tested by gcc.dg/pr96377-1.c.)

    (2) It means that we reject code that should be valid with
        -flax-vector-conversions.  (Tested by gcc.dg/pr96377-2.c.)

    (3) On arm and aarch64 targets, it means that we reject some
        initializers that mix Advanced SIMD and standard GNU vectors.
        These vectors have traditionally had different TYPE_MAIN_VARIANTs
        because they have different mangling schemes.  (Tested by
        gcc.dg/pr96377-[3-6].c.)

    (4) It means that we reject SVE initializers that should be valid.
        (Tested by gcc.target/aarch64/sve/gnu_vectors_[34].c.)

    (5) After r11-1741-g:31427b974ed7b7dd54e2 we reject:

          arm_neon_type1 array[] = { k ^ arm_neon_value1 };

        because applying the binary operator to arm_neon_value1 strips
        the "Advanced SIMD type" attributes that were added in that patch.
        Stripping the attributes is problematic for other reasons though,
        so that still needs to be fixed separately.

    g++.target/aarch64/sve/gnu_vectors_[34].C already pass.

    gcc/c/
            PR c/96377
            * c-typeck.c (process_init_element): Split test for whether to
            recurse into a record, union or array into...
            (initialize_elementwise_p): ...this new function.  Don't recurse
            into a vector type if the initialization value is also a vector.

    gcc/testsuite/
            PR c/96377
            * gcc.dg/pr96377-1.c: New test.
            * gcc.dg/pr96377-2.c: Likewise.
            * gcc.dg/pr96377-3.c: Likewise.
            * gcc.dg/pr96377-4.c: Likewise.
            * gcc.dg/pr96377-5.c: Likewise.
            * gcc.dg/pr96377-6.c: Likewise.
            * gcc.target/aarch64/pr96377-1.c: Likewise.
            * gcc.target/aarch64/sve/acle/general-c/gnu_vectors_3.c: Likewise.
            * gcc.target/aarch64/sve/acle/general-c/gnu_vectors_4.c: Likewise.
            * g++.target/aarch64/sve/acle/general-c++/gnu_vectors_3.C:
Likewise.
            * g++.target/aarch64/sve/acle/general-c++/gnu_vectors_4.C:
Likewise.

    (cherry picked from commit 7d599ad27b9bcf5165f87710f1abc64bbabd06ae)

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yyc1992 at gmail dot com

--- Comment #13 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
*** Bug 96990 has been marked as a duplicate of this bug. ***

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rsandifo at gcc dot gnu.org]

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

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

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

--- Comment #15 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
The original problem is fixed for GCC 10 and the kernel
manifestation is fixed for trunk.  We still need to deal
with the testcase in comment 8, which is another manifestation
that affects trunk only.

[Bug target/96377] [10/11 Regression] GCC 10.2/11 doesn't build Linux kernel anymore

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
Please open a new bug for the remaining issue since this one was P1 blocking a
10.3 release.