| Message ID | CAGeEQ1jwdwXt7uPPb8k1n9f187ADVZit80hCmkS=z6oo0G_-MA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 19, 2016 at 06:28:18AM +0100, Roger Ferrer Ibáñez wrote: > Hi, > > aarch64-builtins.c defines several SIMD builtin types. Among these > SIMD types there are the polynomials __Poly{8,16,64,128}_t. These are > built by a call to build_distinct_type_copy > (unsigned_int{Q,H,D,T}I_type_node), respectively, i.e. they are not > VECTOR_TYPEs. A later loop, traverses an array containing all the SIMD > types and skips those types for which a VECTOR_TYPE does not have to > be built: this is, types __Poly{8,16,64,128}_t. That same loop does > SET_TYPE_STRUCTURAL_EQUALITY on the newly created vector type, but it > does this unconditionally, thus setting TYPE_CANONICAL of types > __Poly{8,16,64,128}_t to NULL. > > The net effect of this is that the C++ FE is unable to distinguish > between all __Poly{8,16,64,128}_t and between vector types with the > same number of elements but different polynomial type as element type > (like __Poly8x8_t vs __Poly16x8_t). Note that sizeof (correctly) > returns different values for all these types. This patch simply > protects SET_TYPE_STRUCTURAL_EQUALITY inside the branch that creates > the vector type. > > I have bootstrapped and regression tested this on a small board > aarch64-unknown-linux-gnu host without new regressions. 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. > 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 have no experience with making the call as to which sort of change is "small enough" to include in GCC without copyright assignment, so I've CC'ed the AArch64 maintainers, who may be able to give more advice. > gcc/ChangeLog: > > 2016-01-19 Roger Ferrer Ibáñez <rofirrim@gmail.com> Two spaces between the date and your name, and between your name and your email, as so: 2016-01-19 Roger Ferrer Ibáñez <rofirrim@gmail.com> > > PR aarch64/67896 PR target/67896 > * aarch64-builtins.c (aarch64_init_simd_builtin_types): Do not set This should be the path from the directory in which the ChangeLog resides: * config/aarch64/aarch64-builtins.c And a full-stop at the end of the line. So this whole entry would look like: 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. > structural equality to __Poly{8,16,64,128}_t types > > gcc/testsuite/ChangeLog: > > 2016-01-19 Roger Ferrer Ibáñez <rofirrim@gmail.com> > > PR aarch64/67896 > * pr67896.C: New test Same comments apply here: 2016-01-19 Roger Ferrer Ibáñez <rofirrim@gmail.com> PR target/67896 * gcc.target/aarch64/simd/pr67896.C: New. > diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c > index bd7a8dd..edacf10 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 > + { > + aarch64_simd_types[i].itype = > + build_distinct_type_copy > (build_vector_type (eltype, GET_MODE_NUNITS (mode))); I'd rewrap this as: aarch64_simd_types[i].itype = build_distinct_type_copy (build_vector_type (eltype, GET_MODE_NUNITS (mode))); Thanks for the patch! James > + 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 >
From 22edde54bcc50fc41a8195d84d278bd1a1dfeb13 Mon Sep 17 00:00:00 2001 From: Roger Ferrer Ibanez <roger.ferrer@bsc.es> Date: Thu, 14 Jan 2016 21:54:56 +0100 Subject: [PATCH] Do not mark polynomial types as requiring structural equality --- gcc/config/aarch64/aarch64-builtins.c | 8 +++++--- gcc/testsuite/gcc.target/aarch64/simd/pr67896.C | 7 +++++++ 2 files changed, 12 insertions(+), 3 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..edacf10 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 + { + 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
Hi, aarch64-builtins.c defines several SIMD builtin types. Among these SIMD types there are the polynomials __Poly{8,16,64,128}_t. These are built by a call to build_distinct_type_copy (unsigned_int{Q,H,D,T}I_type_node), respectively, i.e. they are not VECTOR_TYPEs. A later loop, traverses an array containing all the SIMD types and skips those types for which a VECTOR_TYPE does not have to be built: this is, types __Poly{8,16,64,128}_t. That same loop does SET_TYPE_STRUCTURAL_EQUALITY on the newly created vector type, but it does this unconditionally, thus setting TYPE_CANONICAL of types __Poly{8,16,64,128}_t to NULL. The net effect of this is that the C++ FE is unable to distinguish between all __Poly{8,16,64,128}_t and between vector types with the same number of elements but different polynomial type as element type (like __Poly8x8_t vs __Poly16x8_t). Note that sizeof (correctly) returns different values for all these types. This patch simply protects SET_TYPE_STRUCTURAL_EQUALITY inside the branch that creates the vector type. I have bootstrapped and regression tested this on a small board aarch64-unknown-linux-gnu host without new regressions. 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. Kind regards, gcc/ChangeLog: 2016-01-19 Roger Ferrer Ibáñez <rofirrim@gmail.com> PR aarch64/67896 * 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 aarch64/67896 * pr67896.C: New test