[gcc r11-8192] c++: Tweak merging of vector attributes that affect type identity [PR98852]

[gcc r11-8192] c++: Tweak merging of vector attributes that affect type identity [PR98852]

[Richard Sandiford]

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.

Diff:
---
 gcc/attribs.c                                      |  23 +++++
 gcc/attribs.h                                      |   1 +
 gcc/cp/typeck.c                                    |  15 ++-
 .../advsimd-intrinsics/advsimd-intrinsics.exp      |  72 ++++++++++++++
 .../aarch64/advsimd-intrinsics/pr98852.C           | 110 +++++++++++++++++++++
 5 files changed, 219 insertions(+), 2 deletions(-)

diff --git a/gcc/attribs.c b/gcc/attribs.c
index 2fb29541f3f..3ffa1b6bc81 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -1420,6 +1420,29 @@ affects_type_identity_attributes (tree attrs, bool value)
   return remove_attributes_matching (attrs, predicate);
 }
 
+/* Remove attributes that affect type identity from ATTRS unless the
+   same attributes occur in OK_ATTRS.  */
+
+tree
+restrict_type_identity_attributes_to (tree attrs, tree ok_attrs)
+{
+  auto predicate = [ok_attrs](const_tree attr,
+			      const attribute_spec *as) -> bool
+    {
+      if (!as || !as->affects_type_identity)
+	return true;
+
+      for (tree ok_attr = lookup_attribute (as->name, ok_attrs);
+	   ok_attr;
+	   ok_attr = lookup_attribute (as->name, TREE_CHAIN (ok_attr)))
+	if (simple_cst_equal (TREE_VALUE (ok_attr), TREE_VALUE (attr)) == 1)
+	  return true;
+
+      return false;
+    };
+  return remove_attributes_matching (attrs, predicate);
+}
+
 /* Return a type like TTYPE except that its TYPE_ATTRIBUTE
    is ATTRIBUTE.
 
diff --git a/gcc/attribs.h b/gcc/attribs.h
index eadb1d0fac9..df78eb152f9 100644
--- a/gcc/attribs.h
+++ b/gcc/attribs.h
@@ -66,6 +66,7 @@ extern bool attribute_value_equal (const_tree, const_tree);
 extern int comp_type_attributes (const_tree, const_tree);
 
 extern tree affects_type_identity_attributes (tree, bool = true);
+extern tree restrict_type_identity_attributes_to (tree, tree);
 
 /* Default versions of target-overridable functions.  */
 extern tree merge_decl_attributes (tree, tree);
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 11dee7d8753..50d0f1e6a62 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -261,6 +261,17 @@ original_type (tree t)
   return cp_build_qualified_type (t, quals);
 }
 
+/* Merge the attributes of type OTHER_TYPE into the attributes of type TYPE
+   and return a variant of TYPE with the merged attributes.  */
+
+static tree
+merge_type_attributes_from (tree type, tree other_type)
+{
+  tree attrs = targetm.merge_type_attributes (type, other_type);
+  attrs = restrict_type_identity_attributes_to (attrs, TYPE_ATTRIBUTES (type));
+  return cp_build_type_attribute_variant (type, attrs);
+}
+
 /* Return the common type for two arithmetic types T1 and T2 under the
    usual arithmetic conversions.  The default conversions have already
    been applied, and enumerated types converted to their compatible
@@ -320,9 +331,9 @@ cp_common_type (tree t1, tree t2)
       /* When we get here we should have two vectors of the same size.
 	 Just prefer the unsigned one if present.  */
       if (TYPE_UNSIGNED (t1))
-	return build_type_attribute_variant (t1, attributes);
+	return merge_type_attributes_from (t1, t2);
       else
-	return build_type_attribute_variant (t2, attributes);
+	return merge_type_attributes_from (t2, t1);
     }
 
   /* If only one is real, use it as the result.  */
diff --git a/gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp b/gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp
new file mode 100644
index 00000000000..268e2213b1f
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp
@@ -0,0 +1,72 @@
+# Copyright (C) 2014-2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't an ARM or AArch64 target.
+if {![istarget arm*-*-*]
+    && ![istarget aarch64*-*-*]} then {
+  return
+}
+
+# Load support procs.
+load_lib g++-dg.exp
+
+# Initialize `dg'.
+load_lib c-torture.exp
+
+dg-init
+
+# The default action for a test is 'run'.  Save current default.
+global dg-do-what-default
+set save-dg-do-what-default ${dg-do-what-default}
+
+# For ARM, make sure that we have a target compatible with NEON, and do
+# not attempt to run execution tests if the hardware doesn't support it.
+if {[istarget arm*-*-*]} then {
+    if {![check_effective_target_arm_neon_ok]} then {
+      return
+    }
+    if {![is-effective-target arm_neon_hw]} then {
+        set dg-do-what-default compile
+    } else {
+        set dg-do-what-default run
+    }
+} else {
+    set dg-do-what-default run
+}
+
+torture-init
+set-torture-options $C_TORTURE_OPTIONS {{}} $LTO_TORTURE_OPTIONS
+
+# Make sure Neon flags are provided, if necessary.  Use fp16 if we can.
+# Use fp16 arithmetic operations if the hardware supports it.
+if {[check_effective_target_arm_v8_2a_fp16_neon_hw]} then {
+  set additional_flags [add_options_for_arm_v8_2a_fp16_neon ""]
+} elseif {[check_effective_target_arm_neon_fp16_ok]} then {
+  set additional_flags [add_options_for_arm_neon_fp16 ""]
+} else {
+  set additional_flags [add_options_for_arm_neon ""]
+}
+
+# Main loop.
+gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] \
+	       "" ${additional_flags}
+
+# All done.
+set dg-do-what-default ${save-dg-do-what-default}
+torture-finish
+dg-finish
diff --git a/gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/pr98852.C b/gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/pr98852.C
new file mode 100644
index 00000000000..0cb5c89c74f
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/pr98852.C
@@ -0,0 +1,110 @@
+/* { dg-do compile } */
+
+#include <arm_neon.h>
+
+using int32_elt = __typeof(((int32x4_t *) nullptr)[0][0]);
+using uint32_elt = __typeof(((uint32x4_t *) nullptr)[0][0]);
+
+typedef int32_elt gnu_int32x4_t __attribute__((vector_size(16)));
+typedef uint32_elt gnu_uint32x4_t __attribute__((vector_size(16)));
+
+template<typename T> struct id;
+template<> struct id<gnu_int32x4_t> { static const int value = 1; };
+template<> struct id<gnu_uint32x4_t> { 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 (gnu_int32x4_t sg, gnu_uint32x4_t ug, int32x4_t sn, uint32x4_t un, bool c)
+{
+  CHECK_TYPE (sg, gnu_int32x4_t);
+  CHECK_TYPE (ug, gnu_uint32x4_t);
+  CHECK_TYPE (sn, int32x4_t);
+  CHECK_TYPE (un, uint32x4_t);
+
+  CHECK_TYPE (sg + 1, gnu_int32x4_t);
+  CHECK_TYPE (ug + 1, gnu_uint32x4_t);
+  CHECK_TYPE (sn + 1, int32x4_t);
+  CHECK_TYPE (un + 1, uint32x4_t);
+
+  CHECK_TYPE (1 + sg, gnu_int32x4_t);
+  CHECK_TYPE (1 + ug, gnu_uint32x4_t);
+  CHECK_TYPE (1 + sn, int32x4_t);
+  CHECK_TYPE (1 + un, uint32x4_t);
+
+  CHECK_TYPE (sg + sg, gnu_int32x4_t);
+  CHECK_TYPE (ug + ug, gnu_uint32x4_t);
+  CHECK_TYPE (sn + sn, int32x4_t);
+  CHECK_TYPE (un + un, uint32x4_t);
+
+  // In C++, unlike C, the behavior is to prefer unsigned types over
+  // signed types.
+  CHECK_TYPE (sg + ug, gnu_uint32x4_t);
+  CHECK_TYPE (ug + sg, gnu_uint32x4_t);
+  CHECK_TYPE (sn + un, uint32x4_t);
+  CHECK_TYPE (un + sn, uint32x4_t);
+
+  // That being the case, it seems more consistent to do the same thing
+  // for mixed GNU and arm_neon.h operations.
+  CHECK_TYPE (sg + un, uint32x4_t);
+  CHECK_TYPE (un + sg, uint32x4_t);
+  CHECK_TYPE (sn + ug, gnu_uint32x4_t);
+  CHECK_TYPE (ug + sn, gnu_uint32x4_t);
+
+  // If the types have the same signedness, the traditional behavior is
+  // to pick the first type if it is unsigned and the second type otherwise.
+  // This is not necessarily sensible, but dates back to at least GCC 9.1.
+  // We could probably change it.
+  CHECK_TYPE (sg + sn, int32x4_t);
+  CHECK_TYPE (sn + sg, gnu_int32x4_t);
+  CHECK_TYPE (un + ug, uint32x4_t);
+  CHECK_TYPE (ug + un, gnu_uint32x4_t);
+
+  CHECK_TYPE (c ? sg + sg : sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug + ug : ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn + sn : sn, int32x4_t);
+  CHECK_TYPE (c ? un + un : un, uint32x4_t);
+
+  CHECK_TYPE (c ? sg + 1 : sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug + 1 : ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn + 1 : sn, int32x4_t);
+  CHECK_TYPE (c ? un + 1 : un, uint32x4_t);
+
+  CHECK_TYPE (c ? 1 + sg : sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? 1 + ug : ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? 1 + sn : sn, int32x4_t);
+  CHECK_TYPE (c ? 1 + un : un, uint32x4_t);
+
+  CHECK_TYPE (c ? sg : sg + sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug : ug + ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn : sn + sn, int32x4_t);
+  CHECK_TYPE (c ? un : un + un, uint32x4_t);
+
+  CHECK_TYPE (c ? sg : sg + 1, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug : ug + 1, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn : sn + 1, int32x4_t);
+  CHECK_TYPE (c ? un : un + 1, uint32x4_t);
+
+  CHECK_TYPE (c ? sg : 1 + sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug : 1 + ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn : 1 + sn, int32x4_t);
+  CHECK_TYPE (c ? un : 1 + un, uint32x4_t);
+
+  CHECK_TYPE (c ? sg + sg : sg + sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug + ug : ug + ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn + sn : sn + sn, int32x4_t);
+  CHECK_TYPE (c ? un + un : un + un, uint32x4_t);
+
+  CHECK_TYPE (c ? sg + sg : sg + 1, gnu_int32x4_t);
+  CHECK_TYPE (c ? ug + ug : ug + 1, gnu_uint32x4_t);
+  CHECK_TYPE (c ? sn + sn : sn + 1, int32x4_t);
+  CHECK_TYPE (c ? un + un : un + 1, uint32x4_t);
+
+  CHECK_TYPE (c ? 1 + sg : sg + sg, gnu_int32x4_t);
+  CHECK_TYPE (c ? 1 + ug : ug + ug, gnu_uint32x4_t);
+  CHECK_TYPE (c ? 1 + sn : sn + sn, int32x4_t);
+  CHECK_TYPE (c ? 1 + un : un + un, uint32x4_t);
+}