diff mbox series

[10] aarch64: Treat GNU and Advanced SIMD vectors as distinct [PR95726]

Message ID mptimeygks9.fsf@arm.com
State New
Headers show
Series [10] aarch64: Treat GNU and Advanced SIMD vectors as distinct [PR95726] | expand

Commit Message

Richard Sandiford July 8, 2020, 2:10 p.m. UTC 
This is a release branch version of r11-1741-g:31427b974ed7b7dd54e2.
The trunk version of the patch made GNU and Advanced SIMD vectors
distinct (but inter-convertible) in all cases.  However, the
traditional behaviour is that the types are distinct in template
arguments but not otherwise.

Following a suggestion from Jason, this patch puts the check
for different vector types under comparing_specializations.
In order to keep the backport as simple as possible, the patch
hard-codes the name of the attribute in the frontend rather than
adding a new branch-only target hook.  This code will be reused
for AArch32 too.

I didn't find a test that tripped the assert on the branch,
even with the --param in the PR, so instead I tested this by
forcing the hash function to only hash the tree code.  That
made the static assertion in the test fail without the patch
but pass with it.

This means that the test passes for unmodified sources even
without the patch (unless you're very unlucky).

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for branches?

Richard


gcc/
	PR target/95726
	* config/aarch64/aarch64.c (aarch64_attribute_table): Add
	"Advanced SIMD type".
	* config/aarch64/aarch64-builtins.c: Include stringpool.h and
	attribs.h.
	(aarch64_init_simd_builtin_types): Add an "Advanced SIMD type"
	attribute to each Advanced SIMD type.

gcc/cp/
	PR target/95726
	* typeck.c (structural_comptypes): When comparing template
	specializations, differentiate between vectors that have and
	do not have an "Advanced SIMD type" attribute.

gcc/testsuite/
	PR target/95726
	* g++.target/aarch64/pr95726.C: New test.
---
 gcc/config/aarch64/aarch64-builtins.c      | 14 +++++++----
 gcc/config/aarch64/aarch64.c               |  1 +
 gcc/cp/typeck.c                            |  9 +++++++
 gcc/testsuite/g++.target/aarch64/pr95726.C | 28 ++++++++++++++++++++++
 4 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/aarch64/pr95726.C

Comments

Jakub Jelinek July 8, 2020, 2:20 p.m. UTC | #1 
On Wed, Jul 08, 2020 at 03:10:14PM +0100, Richard Sandiford wrote:
> gcc/
> 	PR target/95726
> 	* config/aarch64/aarch64.c (aarch64_attribute_table): Add
> 	"Advanced SIMD type".
> 	* config/aarch64/aarch64-builtins.c: Include stringpool.h and
> 	attribs.h.
> 	(aarch64_init_simd_builtin_types): Add an "Advanced SIMD type"
> 	attribute to each Advanced SIMD type.
> 
> gcc/cp/
> 	PR target/95726
> 	* typeck.c (structural_comptypes): When comparing template
> 	specializations, differentiate between vectors that have and
> 	do not have an "Advanced SIMD type" attribute.
> 
> gcc/testsuite/
> 	PR target/95726
> 	* g++.target/aarch64/pr95726.C: New test.
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -1429,6 +1429,15 @@ structural_comptypes (tree t1, tree t2, int strict)
>  	  || maybe_ne (TYPE_VECTOR_SUBPARTS (t1), TYPE_VECTOR_SUBPARTS (t2))
>  	  || !same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
>  	return false;

I'd at least add an explaining comment that it is a hack for GCC 8-10 only,
for aarch64 and arm targets, why, reference to the PR and that it is solved
differently for GCC 11+.

> +      if (comparing_specializations)
> +	{
> +	  bool asimd1 = lookup_attribute ("Advanced SIMD type",
> +					  TYPE_ATTRIBUTES (t1));
> +	  bool asimd2 = lookup_attribute ("Advanced SIMD type",
> +					  TYPE_ATTRIBUTES (t2));
> +	  if (asimd1 != asimd2)
> +	    return false;
> +	}

Otherwise LGTM for release branches if it is acceptable that way to Jason.

Just note, Richi announced 10.2 RC will be June 15th, so would be nice to
have it in by then.

	Jakub
diff mbox series

Patch

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index 95213cd70c8..8407a34b594 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -43,6 +43,8 @@ 
 #include "gimple-iterator.h"
 #include "case-cfn-macros.h"
 #include "emit-rtl.h"
+#include "stringpool.h"
+#include "attribs.h"
 
 #define v8qi_UP  E_V8QImode
 #define v4hi_UP  E_V4HImode
@@ -802,10 +804,14 @@  aarch64_init_simd_builtin_types (void)
 
       if (aarch64_simd_types[i].itype == NULL)
 	{
-	  aarch64_simd_types[i].itype
-	    = build_distinct_type_copy
-	      (build_vector_type (eltype, GET_MODE_NUNITS (mode)));
-	  SET_TYPE_STRUCTURAL_EQUALITY (aarch64_simd_types[i].itype);
+	  tree type = build_vector_type (eltype, GET_MODE_NUNITS (mode));
+	  type = build_distinct_type_copy (type);
+	  SET_TYPE_STRUCTURAL_EQUALITY (type);
+
+	  TYPE_ATTRIBUTES (type)
+	    = tree_cons (get_identifier ("Advanced SIMD type"),
+			 NULL_TREE, TYPE_ATTRIBUTES (type));
+	  aarch64_simd_types[i].itype = type;
 	}
 
       tdecl = add_builtin_type (aarch64_simd_types[i].name,
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 8aeb78a4793..60173b34b23 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1429,6 +1429,7 @@  static const struct attribute_spec aarch64_attribute_table[] =
   { "arm_sve_vector_bits", 1, 1, false, true,  false, true,
 			  aarch64_sve::handle_arm_sve_vector_bits_attribute,
 			  NULL },
+  { "Advanced SIMD type", 0, 0, false, true,  false, true,  NULL, NULL },
   { "SVE type",		  3, 3, false, true,  false, true,  NULL, NULL },
   { "SVE sizeless type",  0, 0, false, true,  false, true,  NULL, NULL },
   { NULL,                 0, 0, false, false, false, false, NULL, NULL }
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5f8f8290f0f..34a27fe2414 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1429,6 +1429,15 @@  structural_comptypes (tree t1, tree t2, int strict)
 	  || maybe_ne (TYPE_VECTOR_SUBPARTS (t1), TYPE_VECTOR_SUBPARTS (t2))
 	  || !same_type_p (TREE_TYPE (t1), TREE_TYPE (t2)))
 	return false;
+      if (comparing_specializations)
+	{
+	  bool asimd1 = lookup_attribute ("Advanced SIMD type",
+					  TYPE_ATTRIBUTES (t1));
+	  bool asimd2 = lookup_attribute ("Advanced SIMD type",
+					  TYPE_ATTRIBUTES (t2));
+	  if (asimd1 != asimd2)
+	    return false;
+	}
       break;
 
     case TYPE_PACK_EXPANSION:
diff --git a/gcc/testsuite/g++.target/aarch64/pr95726.C b/gcc/testsuite/g++.target/aarch64/pr95726.C
new file mode 100644
index 00000000000..3327b335ff5
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr95726.C
@@ -0,0 +1,28 @@ 
+#include <arm_neon.h>
+
+typedef float vecf __attribute__((vector_size(16)));
+
+// This assertion must hold: vecf and float32x4_t have distinct identities
+// and mangle differently, so they are not interchangeable.
+template<typename T> struct bar;
+template<> struct bar<vecf> { static const int x = 1; };
+template<> struct bar<float32x4_t> { static const int x = 2; };
+static_assert(bar<vecf>::x + bar<float32x4_t>::x == 3, "boo");
+
+// GCC 10 and earlier should continue to accept this, but the behavior
+// changed in GCC 11.
+vecf x;
+float32x4_t y;
+float32x4_t &z = x;
+
+// These assignment must be valid even in the strictest mode: vecf must
+// implicitly convert to float32x4_t and vice versa.
+void foo() { x = y; y = x; }
+
+// GCC 10 and earlier should continue to accept this, but the behavior
+// changed in GCC 11.
+auto sel1(bool c, decltype(c ? x : y) d) { return d; }
+auto sel2(bool c, decltype(c ? y : x) d) { return d; }
+
+/* { dg-final { scan-assembler {_Z4sel1bRDv4_f} } } */
+/* { dg-final { scan-assembler {_Z4sel2bR13__Float32x4_t} } } */