[Bug c/98852] New: [11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[Bug c/98852] New: [11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[rsandifo at gcc dot gnu.org]

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

            Bug ID: 98852
           Summary: [11 Regression] Conditional expression wrongly
                    rejected for arm_neon.h vectors
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rsandifo at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64*-*-*

#include <arm_neon.h>

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

is wrongly rejected for C, but not C++.  This is extracted from
comment 8 of PR96377.

[Bug c/98852] [11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug c/98852] [10/11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
      Known to fail|                            |10.3.0
   Target Milestone|11.0                        |10.4
            Summary|[11 Regression] Conditional |[10/11 Regression]
                   |expression wrongly rejected |Conditional expression
                   |for arm_neon.h vectors      |wrongly rejected for
                   |                            |arm_neon.h vectors
           Priority|P3                          |P2
   Last reconfirmed|                            |2021-04-09
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
On the GCC 10 branch I see

t.c: In function 'foo':
t.c:6:20: error: type mismatch in conditional expression
    6 |   return c ? x + 1 : y;
      |                    ^

so the problem is there as well (maybe it was accepted in some earlier GCC 10
release).  P2.  Please fill out a known-to-work version.

[Bug c/98852] [10/11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[rsandifo at gcc dot gnu.org]

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

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 #2 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
There's a related C++ problem:

#include <arm_neon.h>

typedef int32_t vint32 __attribute__((vector_size(16)));
typedef uint32_t vuint32 __attribute__((vector_size(16)));

template<typename T> struct id;
template<> struct id<vint32> { static const int value = 1; };
template<> struct id<vuint32> { static const int value = 2; };
template<> struct id<int32x4_t> { static const int value = 3; };
template<> struct id<uint32x4_t> { static const int value = 4; };

#define CHECK_TYPE(EXPR, TYPE) \
  static_assert(id<decltype(EXPR)>::value == id<TYPE>::value, "foo")

void
f (vint32 sg, vuint32 ug, int32x4_t sn, uint32x4_t un)
{
  CHECK_TYPE (sg + un, uint32x4_t);
  CHECK_TYPE (ug + sn, vuint32); // Fails in 10.2+
  CHECK_TYPE (sn + ug, vuint32); // Fails in 10.2+
  CHECK_TYPE (un + sg, uint32x4_t);
}

[Bug c/98852] [10/11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 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:a3317f7b3c02907a122f89879e5b6e90c386e64d

commit r11-8191-ga3317f7b3c02907a122f89879e5b6e90c386e64d
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Thu Apr 15 11:37:38 2021 +0100

    c: Don't drop vector attributes that affect type identity [PR98852]

    <arm_neon.h> types are distinct from GNU vector types in at least
    their mangling.  However, there used to be nothing explicit in the
    VECTOR_TYPE itself to indicate the difference: we simply treated them
    as distinct TYPE_MAIN_VARIANTs.  This caused problems like the ones
    reported in PR95726.

    The fix for that PR was to add type attributes to the <arm_neon.h>
    types, in order to maintain the distinction between them and GNU
    vectors.  However, this in turn caused PR98852, where c_common_type
    would unconditionally drop the attributes on the source types.
    This meant that:

       <arm_neon.h> vector + <arm_neon.h> vector

    had a GNU vector type rather than an <arm_neon.h> vector type.

    See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96377#c2 for
    Jakub's analysis of the history of this c_common_type code.
    TBH I'm not sure which case the build_type_attribute_variant
    code is handling, but I think we should at least avoid dropping
    attributes that affect type identity.

    I've tried to audit the C and target-specific attributes to look
    for other types that might be affected by this, but I couldn't
    see any.  We are only dealing with:

      gcc_assert (code1 == VECTOR_TYPE || code1 == COMPLEX_TYPE
                  || code1 == FIXED_POINT_TYPE || code1 == REAL_TYPE
                  || code1 == INTEGER_TYPE);

    which excludes most affects_type_identity attributes.  The closest
    was s390_vector_bool, but the handler for that attribute changes
    the type node and drops the attribute itself (*no_add_attrs = true).

    I put the main list handling into a separate function
    (remove_attributes_matching) because a later patch will need it
    for something else.

    gcc/
            PR c/98852
            * attribs.h (affects_type_identity_attributes): Declare.
            * attribs.c (remove_attributes_matching): New function.
            (affects_type_identity_attributes): Likewise.

    gcc/c/
            PR c/98852
            * c-typeck.c (c_common_type): Do not drop attributes that
            affect type identity.

    gcc/testsuite/
            PR c/98852
            * gcc.target/aarch64/advsimd-intrinsics/pr98852.c: New test.

[Bug c/98852] [10/11 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[cvs-commit at gcc dot gnu.org]

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

--- 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:1696fc1ea01d5c9dce96b5d3122921aab9308f59

commit r11-8192-g1696fc1ea01d5c9dce96b5d3122921aab9308f59
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Thu Apr 15 11:37:39 2021 +0100

    c++: Tweak merging of vector attributes that affect type identity [PR98852]

    <arm_neon.h> types are distinct from GNU vector types in at least
    their mangling.  However, there used to be nothing explicit in the
    VECTOR_TYPE itself to indicate the difference: we simply treated them
    as distinct TYPE_MAIN_VARIANTs.  This caused problems like the ones
    reported in PR95726.

    The fix for that PR was to add type attributes to the <arm_neon.h>
    types, in order to maintain the distinction between them and GNU
    vectors.  However, this in turn caused PR98852, where cp_common_type
    would merge the type attributes from the two source types and attach
    the result to the common type.  For example:

       unsigned vector with no attribute + signed vector with attribute X

    would get converted to:

       unsigned vector with attribute X

    That isn't what we want in this case, since X describes the mangling
    of the original type.  But even if we dropped the mangling from X and
    worked it out from context, we would still have a situation in which
    the common type was provably distinct from both of the source types:
    it would take its <arm_neon.h>-ness from one side and its signedness
    from the other.  I guess there are other cases where the common type
    doesn't match either side, but I'm not sure it's the obvious behaviour
    here.  It's also different from GCC 10.1 and earlier, where the unsigned
    vector âwonâ in its original form.

    This patch instead merges only the attributes that don't affect type
    identity.  For now I've restricted it to vector types, since we're so
    close to GCC 11, but it might make sense to use this elsewhere.

    I've tried to audit the C and target-specific attributes to look for
    other types that might be affected by this, but I couldn't see any.
    The closest was s390_vector_bool, but the handler for that attribute
    changes the type node and drops the attribute itself
    (*no_add_attrs = true).

    gcc/
            PR c++/98852
            * attribs.h (restrict_type_identity_attributes_to): Declare.
            * attribs.c (restrict_type_identity_attributes_to): New function.

    gcc/cp/
            PR c++/98852
            * typeck.c (merge_type_attributes_from): New function.
            (cp_common_type): Use it for vector types.

[Bug c/98852] [10 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[rsandifo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11 Regression]          |[10 Regression] Conditional
                   |Conditional expression      |expression wrongly rejected
                   |wrongly rejected for        |for arm_neon.h vectors
                   |arm_neon.h vectors          |

--- Comment #5 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed on trunk so far.  Backports will need to be slightly different.

[Bug c/98852] [10 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug c/98852] [10 Regression] Conditional expression wrongly rejected for arm_neon.h vectors

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
      Known to work|                            |11.1.0
      Known to fail|                            |10.5.0
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|10.5                        |11.0

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for GCC 11.