diff mbox

[AArch64] Fix for PR67896 (C++ FE cannot distinguish __Poly{8,16,64,128}_t types)

Message ID CAGeEQ1ivp2gL+pwjcQGBVg0J2mdY6NHTr_4JJ4cPm7WbLvR23g@mail.gmail.com
State New
Headers show

Commit Message

Roger Ferrer Ibáñez Jan. 20, 2016, 8:27 p.m. UTC
Hi James,

> This patch looks technically correct to me, though there is a small
> style issue to correct (in-line below), and your ChangeLogs don't fit
> our usual style.

thank you very much for the useful comments. I'm attaching a new
version of the patch with the style issues (hopefully) ironed out.

Kind regards,

gcc/ChangeLog:

2016-01-19  Roger Ferrer Ibáñez  <rofirrim@gmail.com>

        PR target/67896
        * config/aarch64/aarch64-builtins.c
        (aarch64_init_simd_builtin_types): Do not set structural
        equality to __Poly{8,16,64,128}_t types.

gcc/testsuite/ChangeLog:

2016-01-19  Roger Ferrer Ibáñez  <rofirrim@gmail.com>

        PR target/67896
        * gcc.target/aarch64/simd/pr67896.C: New.

Comments

James Greenhalgh Jan. 25, 2016, 12:15 p.m. UTC | #1
On Wed, Jan 20, 2016 at 09:27:41PM +0100, Roger Ferrer Ibáñez wrote:
> Hi James,
> 
> > This patch looks technically correct to me, though there is a small
> > style issue to correct (in-line below), and your ChangeLogs don't fit
> > our usual style.
> 
> thank you very much for the useful comments. I'm attaching a new
> version of the patch with the style issues (hopefully) ironed out.

Thanks, this version of the patch looks correct to me.

> > > P.S.: I haven't signed the copyright assignment to the FSF. The change
> > > is really small but I can do the paperwork if required.

I can't commit it on your behalf until we've heard back regarding whether
this needs a copyright assignment to the FSF, but once I've heard I'd
be happy to commit this for you. I'll expand the CC list a bit further
to see if we can get an answer on that.

Thanks again for the analysis and patch.

James

> gcc/ChangeLog:
> 
> 2016-01-19  Roger Ferrer Ibáñez  <rofirrim@gmail.com>
> 
>         PR target/67896
>         * config/aarch64/aarch64-builtins.c
>         (aarch64_init_simd_builtin_types): Do not set structural
>         equality to __Poly{8,16,64,128}_t types.
> 
> gcc/testsuite/ChangeLog:
> 
> 2016-01-19  Roger Ferrer Ibáñez  <rofirrim@gmail.com>
> 
>         PR target/67896
>         * gcc.target/aarch64/simd/pr67896.C: New.
> 
> -- 
> Roger Ferrer Ibáñez

> From 72c065f6a3f9d168baf357de1b567faa6042c03b Mon Sep 17 00:00:00 2001
> From: Roger Ferrer Ibanez <roger.ferrer@bsc.es>
> Date: Wed, 20 Jan 2016 21:11:42 +0100
> Subject: [PATCH] Do not set structural equality on polynomial types
> 
> ---
>  gcc/config/aarch64/aarch64-builtins.c           | 10 ++++++----
>  gcc/testsuite/gcc.target/aarch64/simd/pr67896.C |  7 +++++++
>  2 files changed, 13 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/pr67896.C
> 
> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
> index bd7a8dd..40272ed 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -610,14 +610,16 @@ aarch64_init_simd_builtin_types (void)
>        enum machine_mode mode = aarch64_simd_types[i].mode;
>  
>        if (aarch64_simd_types[i].itype == NULL)
> -	aarch64_simd_types[i].itype =
> -	  build_distinct_type_copy
> -	    (build_vector_type (eltype, GET_MODE_NUNITS (mode)));
> +	{
> +	  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);
> +	}
>  
>        tdecl = add_builtin_type (aarch64_simd_types[i].name,
>  				aarch64_simd_types[i].itype);
>        TYPE_NAME (aarch64_simd_types[i].itype) = tdecl;
> -      SET_TYPE_STRUCTURAL_EQUALITY (aarch64_simd_types[i].itype);
>      }
>  
>  #define AARCH64_BUILD_SIGNED_TYPE(mode)  \
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd/pr67896.C b/gcc/testsuite/gcc.target/aarch64/simd/pr67896.C
> new file mode 100644
> index 0000000..1f916e0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/simd/pr67896.C
> @@ -0,0 +1,7 @@
> +typedef __Poly8_t A;
> +typedef __Poly16_t A; /* { dg-error "conflicting declaration" } */
> +typedef __Poly64_t A; /* { dg-error "conflicting declaration" } */
> +typedef __Poly128_t A; /* { dg-error "conflicting declaration" } */
> +
> +typedef __Poly8x8_t B;
> +typedef __Poly16x8_t B; /* { dg-error "conflicting declaration" } */ 
> -- 
> 2.1.4
>
Mike Stump Jan. 25, 2016, 6:25 p.m. UTC | #2
On Jan 25, 2016, at 4:15 AM, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>>>> P.S.: I haven't signed the copyright assignment to the FSF. The change
>>>> is really small but I can do the paperwork if required.
> 
> I can't commit it on your behalf until we've heard back regarding whether
> this needs a copyright assignment to the FSF, but once I've heard I'd
> be happy to commit this for you.

This is fine for the tree without paper work.  Though, if you work on gcc on a regular basis and are likely to contribute more work in the future, it is nice to get the paper work out of the way for next time.
James Greenhalgh April 1, 2016, 9:48 a.m. UTC | #3
On Mon, Jan 25, 2016 at 12:15:48PM +0000, James Greenhalgh wrote:
> On Wed, Jan 20, 2016 at 09:27:41PM +0100, Roger Ferrer Ibáñez wrote:
> > Hi James,
> > 
> > > This patch looks technically correct to me, though there is a small
> > > style issue to correct (in-line below), and your ChangeLogs don't fit
> > > our usual style.
> > 
> > thank you very much for the useful comments. I'm attaching a new
> > version of the patch with the style issues (hopefully) ironed out.
> 
> Thanks, this version of the patch looks correct to me.
> 
> > > > P.S.: I haven't signed the copyright assignment to the FSF. The change
> > > > is really small but I can do the paperwork if required.
> 
> I can't commit it on your behalf until we've heard back regarding whether
> this needs a copyright assignment to the FSF, but once I've heard I'd
> be happy to commit this for you. I'll expand the CC list a bit further
> to see if we can get an answer on that.
> 
> Thanks again for the analysis and patch.

This patch should also be backported to GCC 5, which has the same bug.

I've done that as r234665 as the merge was clean after a bootstrap and
test cycle on aarch64-none-linux-gnu.

Thanks,
James
diff mbox

Patch

From 72c065f6a3f9d168baf357de1b567faa6042c03b Mon Sep 17 00:00:00 2001
From: Roger Ferrer Ibanez <roger.ferrer@bsc.es>
Date: Wed, 20 Jan 2016 21:11:42 +0100
Subject: [PATCH] Do not set structural equality on polynomial types

---
 gcc/config/aarch64/aarch64-builtins.c           | 10 ++++++----
 gcc/testsuite/gcc.target/aarch64/simd/pr67896.C |  7 +++++++
 2 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/pr67896.C

diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index bd7a8dd..40272ed 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -610,14 +610,16 @@  aarch64_init_simd_builtin_types (void)
       enum machine_mode mode = aarch64_simd_types[i].mode;
 
       if (aarch64_simd_types[i].itype == NULL)
-	aarch64_simd_types[i].itype =
-	  build_distinct_type_copy
-	    (build_vector_type (eltype, GET_MODE_NUNITS (mode)));
+	{
+	  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);
+	}
 
       tdecl = add_builtin_type (aarch64_simd_types[i].name,
 				aarch64_simd_types[i].itype);
       TYPE_NAME (aarch64_simd_types[i].itype) = tdecl;
-      SET_TYPE_STRUCTURAL_EQUALITY (aarch64_simd_types[i].itype);
     }
 
 #define AARCH64_BUILD_SIGNED_TYPE(mode)  \
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/pr67896.C b/gcc/testsuite/gcc.target/aarch64/simd/pr67896.C
new file mode 100644
index 0000000..1f916e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/simd/pr67896.C
@@ -0,0 +1,7 @@ 
+typedef __Poly8_t A;
+typedef __Poly16_t A; /* { dg-error "conflicting declaration" } */
+typedef __Poly64_t A; /* { dg-error "conflicting declaration" } */
+typedef __Poly128_t A; /* { dg-error "conflicting declaration" } */
+
+typedef __Poly8x8_t B;
+typedef __Poly16x8_t B; /* { dg-error "conflicting declaration" } */ 
-- 
2.1.4