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.
This commit is contained in:
Richard Sandiford 2021-04-15 11:37:38 +01:00
parent b5f644a98b
commit a3317f7b3c
4 changed files with 193 additions and 2 deletions

View File

@ -1366,6 +1366,60 @@ comp_type_attributes (const_tree type1, const_tree type2)
return targetm.comp_type_attributes (type1, type2);
}
/* PREDICATE acts as a function of type:
(const_tree attr, const attribute_spec *as) -> bool
where ATTR is an attribute and AS is its possibly-null specification.
Return a list of every attribute in attribute list ATTRS for which
PREDICATE is true. Return ATTRS itself if PREDICATE returns true
for every attribute. */
template<typename Predicate>
tree
remove_attributes_matching (tree attrs, Predicate predicate)
{
tree new_attrs = NULL_TREE;
tree *ptr = &new_attrs;
const_tree start = attrs;
for (const_tree attr = attrs; attr; attr = TREE_CHAIN (attr))
{
tree name = get_attribute_name (attr);
const attribute_spec *as = lookup_attribute_spec (name);
const_tree end;
if (!predicate (attr, as))
end = attr;
else if (start == attrs)
continue;
else
end = TREE_CHAIN (attr);
for (; start != end; start = TREE_CHAIN (start))
{
*ptr = tree_cons (TREE_PURPOSE (start),
TREE_VALUE (start), NULL_TREE);
TREE_CHAIN (*ptr) = NULL_TREE;
ptr = &TREE_CHAIN (*ptr);
}
start = TREE_CHAIN (attr);
}
gcc_assert (!start || start == attrs);
return start ? attrs : new_attrs;
}
/* If VALUE is true, return the subset of ATTRS that affect type identity,
otherwise return the subset of ATTRS that don't affect type identity. */
tree
affects_type_identity_attributes (tree attrs, bool value)
{
auto predicate = [value](const_tree, const attribute_spec *as) -> bool
{
return bool (as && as->affects_type_identity) == value;
};
return remove_attributes_matching (attrs, predicate);
}
/* Return a type like TTYPE except that its TYPE_ATTRIBUTE
is ATTRIBUTE.

View File

@ -65,6 +65,8 @@ extern bool attribute_value_equal (const_tree, const_tree);
warning to be generated). */
extern int comp_type_attributes (const_tree, const_tree);
extern tree affects_type_identity_attributes (tree, bool = true);
/* Default versions of target-overridable functions. */
extern tree merge_decl_attributes (tree, tree);
extern tree merge_type_attributes (tree, tree);

View File

@ -740,10 +740,16 @@ c_common_type (tree t1, tree t2)
t2 = TYPE_MAIN_VARIANT (t2);
if (TYPE_ATTRIBUTES (t1) != NULL_TREE)
t1 = build_type_attribute_variant (t1, NULL_TREE);
{
tree attrs = affects_type_identity_attributes (TYPE_ATTRIBUTES (t1));
t1 = build_type_attribute_variant (t1, attrs);
}
if (TYPE_ATTRIBUTES (t2) != NULL_TREE)
t2 = build_type_attribute_variant (t2, NULL_TREE);
{
tree attrs = affects_type_identity_attributes (TYPE_ATTRIBUTES (t2));
t2 = build_type_attribute_variant (t2, attrs);
}
/* Save time if the two types are the same. */

View File

@ -0,0 +1,129 @@
/* { dg-do compile } */
/* { dg-additional-options "-std=c99" } */
#include <arm_neon.h>
typedef __typeof(((int32x4_t *) 0)[0][0]) int32_elt;
typedef __typeof(((uint32x4_t *) 0)[0][0]) uint32_elt;
typedef int32_elt gnu_int32x4_t __attribute__((vector_size(16)));
typedef uint32_elt gnu_uint32x4_t __attribute__((vector_size(16)));
#define X_gnu_int32x4_t 1
#define X_gnu_uint32x4_t 2
#define X_int32x4_t 3
#define X_uint32x4_t 4
#define CHECK(T) T: X_##T
#define CHECK_TYPE(EXPR, TYPE) \
do { \
int x[_Generic (EXPR, \
CHECK (gnu_int32x4_t), \
CHECK (gnu_uint32x4_t), \
CHECK (int32x4_t), \
CHECK (uint32x4_t), \
default : 0) == X_##TYPE ? 1 : -1]; \
} while (0)
void
f (gnu_int32x4_t sg, gnu_uint32x4_t ug, int32x4_t sn, uint32x4_t un, int 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);
/* Traditional behavior for mixed signs is to pick the signedness of the
first operand. We don't have any Arm-specific reason for preferring that
behavior, but including the tests helps to demonstrate the points in the
comments below. */
CHECK_TYPE (sg + ug, gnu_int32x4_t);
CHECK_TYPE (ug + sg, gnu_uint32x4_t);
CHECK_TYPE (sn + un, int32x4_t);
CHECK_TYPE (un + sn, uint32x4_t);
/* Nothing specifies the type of mixed GNU and arm_neon.h operations, but:
- it would be surprising if sg + un had a different signedness from
sg + ug
- it would also be mildly surprising if sg + un had a different type from
both of its operands
So in cases where the operands differ in both signedness and ABI, it seems
more consistent to ignore the ABI difference and apply the usual rules for
differences in sign. */
CHECK_TYPE (sg + un, gnu_int32x4_t);
CHECK_TYPE (ug + sn, gnu_uint32x4_t);
CHECK_TYPE (sn + ug, int32x4_t);
CHECK_TYPE (un + sg, uint32x4_t);
/* And if the first vector wins when operands differ in both signedness
and ABI, it seems more consistent to do the same if the operands differ
only in ABI. */
CHECK_TYPE (sg + sn, gnu_int32x4_t);
CHECK_TYPE (ug + un, gnu_uint32x4_t);
CHECK_TYPE (sn + sg, int32x4_t);
CHECK_TYPE (un + ug, 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);
}