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

2021-04-14 Thread Richard Sandiford via Gcc-patches
 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 
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 -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).

Tested on aarch64-linux-gnu, arm-linux-gnueabihf, armeb-eabi and
x86_64-linux-gnu.  OK for trunk?  The bug also occurs on GCC 10 branch,
but we'll need a slightly different fix there.

Richard


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.
---
 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(-)
 create mode 100644 
gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/advsimd-intrinsics.exp
 create mode 100644 
gcc/testsuite/g++.target/aarch64/advsimd-intrinsics/pr98852.C

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)

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

2021-04-14 Thread Jeff Law via Gcc-patches



On 4/14/2021 9:36 AM, Richard Sandiford via Gcc-patches wrote:

 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 
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 -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).

Tested on aarch64-linux-gnu, arm-linux-gnueabihf, armeb-eabi and
x86_64-linux-gnu.  OK for trunk?  The bug also occurs on GCC 10 branch,
but we'll need a slightly different fix there.

Richard


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.


OK

jeff